Updates .clang-format file to reflect latest supported Clang-Format standards while preserving most of the existing formatting behavior, except for the very last commit which aligns AfterStruct brace placement with AfterClass for code formatting.
The first commit reformats a file containing a C++ struct, the second one modifies the formatting and reformats the same file to show the difference.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32813.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
44 SpacesInCStyleCastParentheses: false
45 SpacesInParentheses: false
46-BreakBeforeConceptDeclarations: Always
47-RequiresExpressionIndentation: OuterScope
48-Standard: c++20
49-UseTab: Never
Again, as said in my previous comment this doesn’t work, because older clang-formats will just error out:
0$ clang-format-16 -i src/bench/prevector.cpp
1/b-c/src/.clang-format:12:3: error: unknown key 'AlignFunctionPointers'
2 AlignFunctionPointers: false
3 ^~~~~~~~~~~~~~~~~~~~~
4Error reading /b-c/src/.clang-format: Invalid argument
0@@ -1,26 +1,40 @@
1 AccessModifierOffset: -4
2-AlignAfterOpenBracket: true
3+AlignAfterOpenBracket: Align
🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/runs/44780552247
LLM reason (✨ experimental): The CI failure is caused by errors in the lint check, related to code style or formatting issues.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
Regenerated `.clang-format` from current configs to replace deprecated keys with up-to-date equivalents.
Also added all current formatter default values to guard against version differences.
The configs were updated with the following command (using v16 for maximal compatibility):
$(brew --prefix llvm@16)/bin/clang-format -dump-config -style=file:src/.clang-format
The new config was tested with:
$(brew --prefix llvm@16)/bin/clang-format -i src/deploymentinfo.h
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
The new config was tested with the same file as the previous commit to showcase the changes:
$(brew --prefix llvm@16)/bin/clang-format -i src/deploymentinfo.h
0@@ -1,49 +1,222 @@
1 Language: Cpp
2 AccessModifierOffset: -4
3-AlignAfterOpenBracket: true
4-AlignEscapedNewlinesLeft: true
5-AlignTrailingComments: true
6+AlignAfterOpenBracket: Align
in 7c9b3e1eae8f206753457149f1b1c837f6627d6d: How to reproduce the commit on non-macos? I don’t have an apple, so I tried:
0# clang-format-16 --version
1Ubuntu clang-format version 16.0.6 (23ubuntu4)
However, the command:
0clang-format-16 -dump-config > .clang-format
produces a different result:
0# git diff -U0
1diff --git a/src/.clang-format b/src/.clang-format
2index 2f3f96ae2e..0822db5d11 100644
3--- a/src/.clang-format
4+++ b/src/.clang-format
5@@ -0,0 +1 @@
6+---
7@@ -2 +3,2 @@ Language: Cpp
8-AccessModifierOffset: -4
9+# BasedOnStyle: LLVM
10+AccessModifierOffset: -2
11@@ -29 +31 @@ AlignConsecutiveMacros:
12-AlignEscapedNewlines: Left
13+AlignEscapedNewlines: Right
14@@ -37 +39 @@ AllowShortBlocksOnASingleLine: Never
15-AllowShortCaseLabelsOnASingleLine: true
16+AllowShortCaseLabelsOnASingleLine: false
17@@ -40 +42 @@ AllowShortFunctionsOnASingleLine: All
18-AllowShortIfStatementsOnASingleLine: WithoutElse
19+AllowShortIfStatementsOnASingleLine: Never
20@@ -46 +48 @@ AlwaysBreakBeforeMultilineStrings: false
21-AlwaysBreakTemplateDeclarations: Yes
22+AlwaysBreakTemplateDeclarations: MultiLine
23@@ -54 +56 @@ BraceWrapping:
24- AfterClass: true
25+ AfterClass: false
26@@ -58 +60 @@ BraceWrapping:
27- AfterFunction: true
28+ AfterFunction: false
29@@ -61 +63 @@ BraceWrapping:
30- AfterStruct: true
31+ AfterStruct: false
32@@ -76 +78 @@ BreakBeforeConceptDeclarations: Always
33-BreakBeforeBraces: Custom
34+BreakBeforeBraces: Attach
35@@ -78 +80 @@ BreakBeforeInlineASMColon: OnlyMultiline
36-BreakBeforeTernaryOperators: false
37+BreakBeforeTernaryOperators: true
38@@ -82 +84 @@ BreakStringLiterals: true
39-ColumnLimit: 0
40+ColumnLimit: 80
41@@ -123 +125 @@ IndentRequiresClause: true
42-IndentWidth: 4
43+IndentWidth: 2
44@@ -137 +139 @@ JavaScriptWrapImports: true
45-KeepEmptyLinesAtTheStartOfBlocks: false
46+KeepEmptyLinesAtTheStartOfBlocks: true
47@@ -142 +144 @@ MacroBlockEnd: ''
48-MaxEmptyLinesToKeep: 2
49+MaxEmptyLinesToKeep: 1
50@@ -160 +162 @@ PenaltyReturnTypeOnItsOwnLine: 60
51-PointerAlignment: Left
52+PointerAlignment: Right
53@@ -208 +210 @@ SpacesInSquareBrackets: false
54-Standard: c++20
55+Standard: Latest
56@@ -222,0 +225 @@ WhitespaceSensitiveMacros:
57+
Many of these keys aren’t even listed in the documentation and the IDE shows that many are invalid since it doesn’t match the allowed schema.
How many are those? Maybe just fixing them up manually one-by-one, if they are relevant is enough?
How to reproduce the commit on non-macos
Please see the commit messages - should I move the instructions to the PR description as well?
produces a different result
I think you have dumped the defaults that don’t load our config first, which are indeed different from what we have (hence our own config).
0$ clang-format-16 --version
1Ubuntu clang-format version 16.0.6 (23ubuntu4)
2$ clang-format-16 -dump-config | grep AfterClass
3 AfterClass: false
4$ clang-format-16 -dump-config -style=file:src/.clang-format | grep AfterClass
5 AfterClass: true
and
0$ clang-format-16 -dump-config -style=file:src/.clang-format | egrep 'AccessModifierOffset|AlignEscapedNewlines|AllowShortCaseLabelsOnASingleLine|AllowShortIfStatementsOnASingleLine|AlwaysBreakTemplateDeclarations|AfterClass|AfterFunction|AfterStruct|BreakBeforeBraces|BreakBeforeTernaryOperators|ColumnLimit|IndentWidth|KeepEmptyLinesAtTheStartOfBlocks|MaxEmptyLinesToKeep|PointerAlignment|Standard'
1AccessModifierOffset: -4
2AlignEscapedNewlines: Left
3AllowShortCaseLabelsOnASingleLine: true
4AllowShortIfStatementsOnASingleLine: WithoutElse
5AlwaysBreakTemplateDeclarations: Yes
6 AfterClass: true
7 AfterFunction: true
8 AfterStruct: false
9BreakBeforeBraces: Custom
10BreakBeforeTernaryOperators: false
11ColumnLimit: 0
12ConstructorInitializerIndentWidth: 4
13ContinuationIndentWidth: 4
14DerivePointerAlignment: false
15IndentWidth: 4
16KeepEmptyLinesAtTheStartOfBlocks: false
17MaxEmptyLinesToKeep: 2
18ObjCBlockIndentWidth: 2
19PointerAlignment: Left
20PPIndentWidth: -1
21 AfterFunctionDefinitionName: false
22 AfterFunctionDeclarationName: false
23Standard: c++20
clang-format-16 -dump-config > .clang-format
to answer my own question, I guess redirecting > is the problem here. It works fine when redirecting to a tmp file first: ( clang-format-16 -dump-config > tmp ) && mv tmp ./.clang-format
I haven’t checked if this fixes the issues with your IDE, but I guess it makes sense either way to get the clang-format config up to date. clang-16 is the minimum required for about a year now, so every developer should have access to clang-format-16 in some way or another.
I’ve recreated the commits on Ubuntu and reviewed the diff:
review ACK 94364a7d5403d9d480ebc065f8709c6dd21e543f 🥙
Signature:
0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 94364a7d5403d9d480ebc065f8709c6dd21e543f 🥙
3xsnBRKa1+85+2JOjTRQ1kZgSlBHhVZxj7LlQpMQNkd5vPsoJ/6a84fiaWp9pAnna9VGz0/DA4J3jNoByA1TuCg==
122+ - Regex: '.*'
123+ Priority: 1
124+ SortPriority: 0
125+ CaseSensitive: false
126+IncludeIsMainRegex: '(Test)?$'
127+IncludeIsMainSourceRegex: ''
IncludeBlocks is set to Preserve (and it should probably stay this way by default, but it would be nice to update the other Include* settings (in another pull request) to properly sort the includes: (1) the main include, (2) everything else (Bitcoin Core includes) (3) third-party library includes (boost, Qt), (4) stdlib includes (those that do not end in .h). This way, one could toggle the IncludeBlocks to get the desired behavior.
