clang-format: make formatting deterministic for different formatter versions #32813

l0rinc · 2025-06-25T15:40:19Z

Updates .clang-format file to reflect latest supported Clang-Format standards while preserving most of the existing formatting behavior.

Note that AfterStruct brace placement was originally aligned here with AfterClass, but was reverted by reviewer demand.

DrahtBot · 2025-06-25T15:40:24Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32813.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hodlinator, maflcko

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko

last commit seems fine, but the others i am not sure. at a minimum you'll have to drop the scripted diffs from executing, because they can't be reproduced anyway with different clang-format versions

src/.clang-format

DrahtBot · 2025-06-25T17:06:36Z

🚧 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.}

Hints

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.

src/.clang-format

maflcko

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 94364a7 🥙

Show signature

Signature:

untrusted 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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 94364a7d5403d9d480ebc065f8709c6dd21e543f 🥙
xsnBRKa1+85+2JOjTRQ1kZgSlBHhVZxj7LlQpMQNkd5vPsoJ/6a84fiaWp9pAnna9VGz0/DA4J3jNoByA1TuCg==

Feeds the file name string directly into the hasher instead of pre-hashing it first. The resuoting hash changes of course, but that doesn't affect us since it's only stored in-memory. Fixes: bitcoin#32604 (comment) A few other related nits were also addressed here: * Comment was redundant, `CSipHasher` is used in other hashers all over the codebase * `static_cast<size_t>` skewed the whole line, changed to shorter functional-style cast * struct formatting is aligned with class formatting - see bitcoin#32813

Feeds the file name string directly into the hasher instead of pre-hashing it first. The resulting hash changes of course, but that doesn't affect us since it's only stored in-memory. Fixes: bitcoin#32604 (comment) A few other related nits were also addressed here: * Comment was redundant, `CSipHasher` is used in other hashers all over the codebase * `static_cast<size_t>` skewed the whole line, changed to shorter functional-style cast * struct formatting is aligned with class formatting - see bitcoin#32813

Feeds the file name string directly into the hasher instead of pre-hashing it first. The resulting hash changes of course, but that doesn't affect us since it's only stored in-memory. Fixes: bitcoin#32604 (comment) A few other related nits were also addressed here: * Comment was redundant, `CSipHasher` is used in other hashers all over the codebase * `static_cast<size_t>` skewed the whole method, changed to shorter functional-style cast * struct formatting is aligned with class formatting - see bitcoin#32813

maflcko · 2025-07-21T07:55:59Z

src/.clang-format

+IncludeBlocks:   Preserve
+IncludeCategories:
+  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
+    Priority:        2
+    SortPriority:    0
+    CaseSensitive:   false
+  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
+    Priority:        3
+    SortPriority:    0
+    CaseSensitive:   false
+  - Regex:           '.*'
+    Priority:        1
+    SortPriority:    0
+    CaseSensitive:   false
+IncludeIsMainRegex: '(Test)?$'
+IncludeIsMainSourceRegex: ''


unrelated note: I understand the setting 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.

doc/developer-notes.md

hodlinator

ACK 7c9b3e1 (first commit)

Ran clang-format -dump-config -style=file:src/.clang-format > foo on that commit and diffed foo against src/.clang-format. Only differences where added/elaborated settings from me running Clang-format 19. Makes sense to keep the format at the minimum version we support.

doc/developer-notes.md

l0rinc · 2025-09-01T21:21:19Z

Kept the formatter config only without the class/struct brace unification, it's ready for re-review, thanks for the comments.

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> Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

hodlinator

re-ACK 13f36c0

struct brace style was made to explicitly differ from class in developer-notes.md after some back & forth (#32813 (comment)).

hodlinator · 2025-09-02T07:18:40Z

src/.clang-format

+PPIndentWidth:   -1
+QualifierAlignment: Leave
+ReferenceAlignment: Pointer
+ReflowComments:  true


nit:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#reflowcomments

We might want to set IndentOnly, so we don't unnecessarily touch lines. But probably better to follow up on this in another PR, similar to initializer lists (#32740 (comment)).

similar to initializer lists (#32740 (comment)).

When formatting initializer lists, one can manually put the : on the next line. This should give a stable clang-format output. (With the current line limit, clang-format won't split or merge lines by itself)

doc/developer-notes.md

maflcko · 2025-09-02T08:36:35Z

re-ACK 13f36c0 🖼

Show signature

Signature:

untrusted 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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 13f36c020f0329b5e975282b45292fdf2a495e31 🖼
gVHv+GHN3tcL9TjuYLc/42PdtTtoJTNhIIFWKWk6RVQ/h7A3PizxMW48bbhzO4bbFkEQlpnI+nyNIkwIbsKHCg==

maflcko reviewed Jun 25, 2025

View reviewed changes

src/.clang-format Show resolved Hide resolved

src/.clang-format Show resolved Hide resolved

DrahtBot added the CI failed label Jun 25, 2025

l0rinc force-pushed the l0rinc/clang-format branch 2 times, most recently from c3ef744 to 5a48369 Compare June 25, 2025 22:03

DrahtBot removed the CI failed label Jun 26, 2025

l0rinc force-pushed the l0rinc/clang-format branch from 5a48369 to 94364a7 Compare June 27, 2025 09:33

l0rinc marked this pull request as ready for review June 29, 2025 08:11

l0rinc changed the title ~~clang-format: modernize and realign clang-format configuration~~ clang-format: align brace-after-struct and *-class formatting Jun 29, 2025

maflcko reviewed Jul 10, 2025

View reviewed changes

src/.clang-format Show resolved Hide resolved

maflcko approved these changes Jul 10, 2025

View reviewed changes

l0rinc mentioned this pull request Jul 10, 2025

refactor: avoid double hashing in SourceLocationHasher #32939

Closed

maflcko requested a review from hebasto July 11, 2025 09:12

maflcko reviewed Jul 21, 2025

View reviewed changes

l0rinc mentioned this pull request Aug 22, 2025

refactor: Header sync optimisations & simplifications #32740

Open

hodlinator reviewed Aug 27, 2025

View reviewed changes

doc/developer-notes.md Outdated Show resolved Hide resolved

hodlinator reviewed Aug 27, 2025

View reviewed changes

doc/developer-notes.md Outdated Show resolved Hide resolved

DrahtBot requested a review from hodlinator August 27, 2025 19:47

l0rinc changed the title ~~clang-format: align brace-after-struct and *-class formatting~~ clang-format: make formatting deterministic for different formatter versions Sep 1, 2025

l0rinc force-pushed the l0rinc/clang-format branch from 94364a7 to 55aba02 Compare September 1, 2025 21:17

l0rinc force-pushed the l0rinc/clang-format branch from 55aba02 to 13f36c0 Compare September 1, 2025 22:10

hodlinator approved these changes Sep 2, 2025

View reviewed changes

DrahtBot requested a review from maflcko September 2, 2025 08:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

clang-format: make formatting deterministic for different formatter versions #32813

clang-format: make formatting deterministic for different formatter versions #32813

Uh oh!

l0rinc commented Jun 25, 2025 •

edited

Loading

Uh oh!

DrahtBot commented Jun 25, 2025 •

edited

Loading

Uh oh!

maflcko left a comment

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Jun 25, 2025

Uh oh!

Uh oh!

maflcko left a comment

Uh oh!

maflcko Jul 21, 2025

Uh oh!

Uh oh!

hodlinator left a comment

Uh oh!

Uh oh!

l0rinc commented Sep 1, 2025

Uh oh!

hodlinator left a comment

Uh oh!

hodlinator Sep 2, 2025

Uh oh!

maflcko Sep 2, 2025

Uh oh!

Uh oh!

maflcko commented Sep 2, 2025

Uh oh!

Uh oh!

clang-format: make formatting deterministic for different formatter versions #32813

Are you sure you want to change the base?

clang-format: make formatting deterministic for different formatter versions #32813

Uh oh!

Conversation

l0rinc commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Jun 25, 2025

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hodlinator left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

l0rinc commented Sep 1, 2025

Uh oh!

hodlinator left a comment

Choose a reason for hiding this comment

Uh oh!

hodlinator Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

maflcko Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Sep 2, 2025

Uh oh!

Uh oh!

l0rinc commented Jun 25, 2025 •

edited

Loading

DrahtBot commented Jun 25, 2025 •

edited

Loading