[tools] Update clang-format config for multi-line function declarations and calls

jnewbery commented at 5:38 pm on May 28, 2020: member

In some cases, running clang-format has made code less readable by joining declarations and calls for functions with many arguments into very long lines. For example:

0-    size_t getQueueInfo(std::chrono::system_clock::time_point &first,
1-                        std::chrono::system_clock::time_point &last) const;
2+    size_t getQueueInfo(std::chrono::system_clock::time_point& first, std::chrono::system_clock::time_point& last) const;

(https://github.com/bitcoin/bitcoin/pull/19090#discussion_r431961148)

This change to clang-format would allow arguments/parameters for func declarations/calls to be split over multiple lines, aligned with the opening parens. It does not force args/params to be on new lines (that setting is BinPackParameters : true).

[tools] Update clang-format config

Allow arguments and parameters to be split over multiple lines
if they don't fit on one line

cc29d1e2c4

in src/.clang-format:3 in cc29d1e2c4

0@@ -1,9 +1,10 @@
1 Language:        Cpp
2 AccessModifierOffset: -4
3-AlignAfterOpenBracket: false
4+AlignAfterOpenBracket: true

MarcoFalke commented at 5:44 pm on May 28, 2020:

Slightly -0 on this:

Wastes whitespace of N*M, where N is the length of the function name and M the number of parameters in their own line.
Changing the function name forces a re-flow of all parameters and/or arguments (if they are in multiple lines). Thus increasing the size of patches.

Also, I am wondering if this change is needed, given that you also set AllowAllArgumentsOnNextLine to true?

jnewbery commented at 3:36 pm on May 29, 2020:

Wastes whitespace

I don’t understand what ‘wastes whitespace’ means. Either the code is easier to read or not.

Without this setting, then clang would format arguments on subsequent lines as follows:

0size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point& first,
1    std::chrono::system_clock::time_point& last) const

which is definitely less clear, since the arguments are now aligned with the function body and there’s no obvious visual split between the function signature and body.

Changing the function name forces a re-flow of all parameters and/or arguments

I think maybe you misunderstand this setting. It only affects that are already on a newline. It doesn’t move parameters onto their own line. So:

function renames are already rare
most functions don’t have parameters split between lines so are unaffected
for those that do, run clang-format and review ignoring whitespace.

MarcoFalke commented at 2:01 pm on June 17, 2020:

Fair enough

fanquake added the label Scripts and tools on May 29, 2020

MarcoFalke commented at 2:01 pm on June 17, 2020: member

ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7 fine with me

practicalswift commented at 11:48 pm on June 17, 2020: contributor

ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7

MarcoFalke merged this on Jun 21, 2020

MarcoFalke closed this on Jun 21, 2020

jnewbery deleted the branch on Jun 21, 2020

troygiorshev commented at 3:41 pm on June 30, 2020: contributor

Posthumous ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7.

For future reference, the addition of AllowAllArgumentsOnNextLine : true in this PR bumps the required version of clang-format up to 9.

Glancing briefly at https://distrowatch.com/ and https://pkgs.org/search/?q=clang it doesn’t look like this will be a problem for most developers.

Recent (19.04+) ubuntu releases are already at version 9. Older ubuntu releases and related distros can easily upgrade by installing clang-format-9. Arch or Manjaro or related look like they’re already there for recent-ish versions. RHEL derivatives look like they’re already there. Debian and related look to be the most problematic, but they can use https://apt.llvm.org/.

jonatack commented at 3:58 pm on June 30, 2020: member

This is a breaking change for me on Debian with Clang 6, and the last time I tried to install llvm from source was an unsuccessful rabbit hole.

MarcoFalke commented at 5:08 pm on June 30, 2020: member

I wasn’t aware that this requires clang-9 and indeed debian stable does not seem to have clang-9:

https://packages.debian.org/search?keywords=clang-format-9

MarcoFalke commented at 5:10 pm on June 30, 2020: member

Maybe the AllowAllArgumentsOnNextLine setting can be removed for now while still keeping the other setting changes?

jnewbery commented at 6:41 pm on June 30, 2020: member

Sorry, this is my fault. Sadly https://clang.llvm.org/docs/ClangFormatStyleOptions.html doesn’t document which configuration options are valid on which versions, so I missed that AllowAllArgumentsOnNextLine requires V9.

Maybe the AllowAllArgumentsOnNextLine setting can be removed for now while still keeping the other setting changes?

Fine by me.

jonatack commented at 1:59 am on July 6, 2020: member

Thanks! Done in #19454.

sidhujag referenced this in commit a9c4279d65 on Jul 7, 2020

fanquake referenced this in commit 6b48c30541 on Jul 9, 2020

sidhujag referenced this in commit 843b646ff2 on Jul 9, 2020

PastaPastaPasta referenced this in commit c45dda6bbb on Jun 27, 2021

PastaPastaPasta referenced this in commit 5a86247acf on Jun 27, 2021

PastaPastaPasta referenced this in commit 089ff0f52e on Jun 28, 2021

PastaPastaPasta referenced this in commit bccbdaabb3 on Jun 28, 2021

PastaPastaPasta referenced this in commit 170818d645 on Jun 29, 2021

PastaPastaPasta referenced this in commit 385afaecf7 on Jun 29, 2021

PastaPastaPasta referenced this in commit 5c297fe8ae on Jul 1, 2021

PastaPastaPasta referenced this in commit 2c1db7bb50 on Jul 1, 2021

PastaPastaPasta referenced this in commit 849a583664 on Jul 1, 2021

PastaPastaPasta referenced this in commit 50bb5c93a5 on Jul 1, 2021

PastaPastaPasta referenced this in commit 3f3bdafd4e on Jul 15, 2021

PastaPastaPasta referenced this in commit 8271531378 on Jul 15, 2021

PastaPastaPasta referenced this in commit ee752790cd on Jul 16, 2021

DrahtBot locked this on Feb 15, 2022

[tools] Update clang-format config for multi-line function declarations and calls #19095