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

jnewbery · 2020-05-28T17:38:39Z

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:

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

(#19090 (comment))

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).

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

src/.clang-format

maflcko · 2020-06-17T14:01:51Z

ACK cc29d1e fine with me

practicalswift · 2020-06-17T23:48:26Z

ACK cc29d1e

troygiorshev · 2020-06-30T15:41:49Z

Posthumous ACK cc29d1e.

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 · 2020-06-30T15:58:59Z

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.

maflcko · 2020-06-30T17:08:29Z

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

maflcko · 2020-06-30T17:10:03Z

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

jnewbery · 2020-06-30T18:41:48Z

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 · 2020-07-06T01:59:25Z

Thanks! Done in #19454.

…e function declarations and calls cc29d1e [tools] Update clang-format config (John Newbery) Pull request description: 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: ``` - size_t getQueueInfo(std::chrono::system_clock::time_point &first, - std::chrono::system_clock::time_point &last) const; + size_t getQueueInfo(std::chrono::system_clock::time_point& first, std::chrono::system_clock::time_point& last) const; ``` (bitcoin#19090 (comment)) 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`). ACKs for top commit: MarcoFalke: ACK cc29d1e fine with me practicalswift: ACK cc29d1e Tree-SHA512: a62474925e71aaff41bdce7960fd5ffd64317da810f694d8084080b054708cf71c2ab2ce3111db5a9260d1c1f9e02d59a2ecb5543b1b6172ce085cb42432160a

b9253c7 tools: clang-format 6 compatibility (Jon Atack) Pull request description: Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in #19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from #19095 (comment). ACKs for top commit: MarcoFalke: Approach ACK b9253c7 , haven't tested Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6

…s < 9 b9253c7 tools: clang-format 6 compatibility (Jon Atack) Pull request description: Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in bitcoin#19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from bitcoin#19095 (comment). ACKs for top commit: MarcoFalke: Approach ACK b9253c7 , haven't tested Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6