Nikita Popov via llvm-dev
2020-Dec-03 21:00 UTC
[llvm-dev] Change coding style for argument alignment?
Hi,
LLVM currently uses a coding style where arguments and parameters need to
be aligned to the opening parenthesis "(".
someFunctionCall(Arg1, Arg2,
Arg3, Arg4);
This style guideline is unfortunate for a couple of reasons:
1. If a function is renamed, it is necessary to also reindent the arguments
at all call-sites. For functions with many or complex arguments, this may
require significant reflow.
2. It causes significant right-ward drift. Especially for declarations,
it's somewhat common for code ending up like this...
Foo SomeClassName::someMethodName(Bar &Arg1,
Bar &Arg2,
Bar &Arg3,
Bar &Arg4) {
... because there is just enough space to fit each argument individually,
but still a need to break each one out on a separate line. Closure
arguments are another common case of very awkward formatting.
3. There are some arcane rules about when this is not the preferred style
and you're supposed to indent arguments on the following line instead.
Is there any chance that the style could be changed towards indenting (but
not aligning) the following line instead?
someFunctionCall(
Arg1, Arg2, Arg3, Arg4);
This is unaffected by renames, does not cause rightward drift and results
in very predictable formatting.
Based on past discussions, LLVM seems to be open to improving coding style
for the better, so I thought I'd give this a shot, as this is a continuous
source of friction for me.
Regards,
Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/a1d28c4c/attachment.html>
Krzysztof Parzyszek via llvm-dev
2020-Dec-03 21:32 UTC
[llvm-dev] Change coding style for argument alignment?
I support this proposal. I very much dislike the current function parameter
formatting (in function declarations/definitions). It just adds a bunch of
whitespace without improvement in readability.
In practice there are already various mixtures of formatting being used. For
example:
bool tailDuplicate(bool IsSimple,
MachineBasicBlock *TailBB,
MachineBasicBlock *ForcedLayoutPred,
SmallVectorImpl<MachineBasicBlock *> &TDBBs,
SmallVectorImpl<MachineInstr *> &Copies,
SmallVectorImpl<MachineBasicBlock *> *CandidatePtr);
void appendCopies(MachineBasicBlock *MBB,
SmallVectorImpl<std::pair<Register, RegSubRegPair>>
&CopyInfos,
SmallVectorImpl<MachineInstr *> &Copies);
void removeDeadBlock(
MachineBasicBlock *MBB,
function_ref<void(MachineBasicBlock *)> *RemovalCallback = nullptr);
These are 3 adjacent declarations in the same file
(llvm/include/llvm/CodeGen/TailDuplicator.h).
I don’t think that the alternative format would make call sites immune to
renaming, since it can still cause fewer or more function arguments to fit in a
given line of code. I still think it’s a worthwhile change in style.
--
Krzysztof Parzyszek kparzysz at quicinc.com<mailto:kparzysz at
quicinc.com> AI tools development
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Nikita
Popov via llvm-dev
Sent: Thursday, December 3, 2020 3:01 PM
To: llvm-dev <llvm-dev at lists.llvm.org>
Subject: [EXT] [llvm-dev] Change coding style for argument alignment?
Hi,
LLVM currently uses a coding style where arguments and parameters need to be
aligned to the opening parenthesis "(".
someFunctionCall(Arg1, Arg2,
Arg3, Arg4);
This style guideline is unfortunate for a couple of reasons:
1. If a function is renamed, it is necessary to also reindent the arguments at
all call-sites. For functions with many or complex arguments, this may require
significant reflow.
2. It causes significant right-ward drift. Especially for declarations, it's
somewhat common for code ending up like this...
Foo SomeClassName::someMethodName(Bar &Arg1,
Bar &Arg2,
Bar &Arg3,
Bar &Arg4) {
... because there is just enough space to fit each argument individually, but
still a need to break each one out on a separate line. Closure arguments are
another common case of very awkward formatting.
3. There are some arcane rules about when this is not the preferred style and
you're supposed to indent arguments on the following line instead.
Is there any chance that the style could be changed towards indenting (but not
aligning) the following line instead?
someFunctionCall(
Arg1, Arg2, Arg3, Arg4);
This is unaffected by renames, does not cause rightward drift and results in
very predictable formatting.
Based on past discussions, LLVM seems to be open to improving coding style for
the better, so I thought I'd give this a shot, as this is a continuous
source of friction for me.
Regards,
Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/2119b054/attachment.html>
Mehdi AMINI via llvm-dev
2020-Dec-03 21:47 UTC
[llvm-dev] Change coding style for argument alignment?
On Thu, Dec 3, 2020 at 1:01 PM Nikita Popov via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hi, > > LLVM currently uses a coding style where arguments and parameters need to > be aligned to the opening parenthesis "(". > > someFunctionCall(Arg1, Arg2, > Arg3, Arg4); > > This style guideline is unfortunate for a couple of reasons: > > 1. If a function is renamed, it is necessary to also reindent the > arguments at all call-sites. For functions with many or complex arguments, > this may require significant reflow. >Why is this a problem though? This is all automated by clang-format in practice.> > 2. It causes significant right-ward drift. Especially for declarations, > it's somewhat common for code ending up like this... > > Foo SomeClassName::someMethodName(Bar &Arg1, > Bar &Arg2, > Bar &Arg3, > Bar &Arg4) { > > ... because there is just enough space to fit each argument individually, > but still a need to break each one out on a separate line. Closure > arguments are another common case of very awkward formatting. >Seems like clang-format has some heuristics for this, I wrote this in a file: Foo SomeClassName::someveryveryveryveryverylongMethodName(Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {} Foo SomeClassName::someveryveryveryveryveryverylongMethodName(Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {} Foo SomeClassName::someMethodName(Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {} and clang-format print it this way: Foo SomeClassName::someveryveryveryveryverylongMethodName(Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {} Foo SomeClassName::someveryveryveryveryveryverylongMethodName( Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {} Foo SomeClassName::someMethodName(Bar &Arg1, Bar &Arg2, Bar &Arg3, Bar &Arg4, Bar &Arg5, Bar &Arg6, Bar &Arg7, Bar &Arg8) {}> > 3. There are some arcane rules about when this is not the preferred style > and you're supposed to indent arguments on the following line instead. > > Is there any chance that the style could be changed towards indenting (but > not aligning) the following line instead? > > someFunctionCall( > Arg1, Arg2, Arg3, Arg4); >Is there an existing clang-format setting to hook up to already?> > This is unaffected by renames, does not cause rightward drift and results > in very predictable formatting. > > Based on past discussions, LLVM seems to be open to improving coding style > for the better, so I thought I'd give this a shot, as this is a continuous > source of friction for me. > > Regards, > Nikita > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/be772886/attachment.html>
David Greene via llvm-dev
2020-Dec-15 15:03 UTC
[llvm-dev] Change coding style for argument alignment?
Nikita Popov via llvm-dev <llvm-dev at lists.llvm.org> writes:> 2. It causes significant right-ward drift. Especially for declarations, > it's somewhat common for code ending up like this... > > Foo SomeClassName::someMethodName(Bar &Arg1, > Bar &Arg2, > Bar &Arg3, > Bar &Arg4) { > > ... because there is just enough space to fit each argument individually, > but still a need to break each one out on a separate line. Closure > arguments are another common case of very awkward formatting.FWIW, I have actually drifted toward this style in my own non-LLVM code. That is, if there are more arguments than can fit on a single line, each gets its own line. For example: void short_args(double an_argument_name, int another_parameter); void long_args(double first_argument, int second_argument, int *third_argument, int fourth argument); I find this easier to read than: void long_args(double first_argument, int second_argument, int *third_argument, int fourth argument); It's easier for me because I need only scan vertically to find the arguments, not both vertically and horizontally. Long function names can be problematic and you're right about changing names requiring reformatting. I've toyed with this style: void a_really_long_and_descriptive_function_name( double first_argument, int second_argument, int *third_argument, int fourth argument); I haven't yet convinced myself that this is a good idea. It looks too jumbled to me and is more difficult to immediately find the start of the argument list. -David
Chris Lattner via llvm-dev
2020-Dec-19 18:35 UTC
[llvm-dev] Change coding style for argument alignment?
On Dec 3, 2020, at 1:00 PM, Nikita Popov via llvm-dev <llvm-dev at lists.llvm.org> wrote:> Hi, > LLVM currently uses a coding style where arguments and parameters need to be aligned to the opening parenthesis "(". > > someFunctionCall(Arg1, Arg2, > Arg3, Arg4); > > This style guideline is unfortunate for a couple of reasons: > > 1. If a function is renamed, it is necessary to also reindent the arguments at all call-sites. For functions with many or complex arguments, this may require significant reflow. > > 2. It causes significant right-ward drift. Especially for declarations, it's somewhat common for code ending up like this... > > Foo SomeClassName::someMethodName(Bar &Arg1, > Bar &Arg2, > Bar &Arg3, > Bar &Arg4) { > > ... because there is just enough space to fit each argument individually, but still a need to break each one out on a separate line. Closure arguments are another common case of very awkward formatting. > > 3. There are some arcane rules about when this is not the preferred style and you're supposed to indent arguments on the following line instead. > > Is there any chance that the style could be changed towards indenting (but not aligning) the following line instead? > > someFunctionCall( > Arg1, Arg2, Arg3, Arg4); > > This is unaffected by renames, does not cause rightward drift and results in very predictable formatting. > > Based on past discussions, LLVM seems to be open to improving coding style for the better, so I thought I'd give this a shot, as this is a continuous source of friction for me.I support this change. I think you’ve done a good job of laying out a principled reason that this approach is better, and I don’t see any significant advantages to the current approach. We should obviously keep the arguments on the same line if they fit though. -Chris