clang-format: Minor tweaks
ClosedPublic
Actions

Authored by markj on Apr 20 2021, 4:27 PM.

Details

Reviewers

arichardson
emaste

Commits

rGd27c35123b92: clang-format: Minor tweaks
rG2c18289b72c8: clang-format: Minor tweaks

Summary

Invert KeepEmptyLinesAtTheStartOfBlocks. We used to require an empty
line at the beginning of functions with no local variables, which I
believe is the reason for this setting. Now it is discouraged in new
code.

Tell clang-format to align consecutive macros, since we tend to do that.
clang-format's output isn't quite what we want here. Typically we have
a tab after a #define for some reason, and clang-format doesn't appear
to have an option for that. clang-format will also use a mix of tabs
and spaces to minimize indentation, which is also against our
convention. However, the result looks better with this setting than
without.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj created this revision.Apr 20 2021, 4:27 PM

Herald added a subscriber: imp. · View Herald TranscriptApr 20 2021, 4:27 PM

markj requested review of this revision.Apr 20 2021, 4:27 PM

Harbormaster completed remote builds in B38731: Diff 87820.Apr 20 2021, 4:27 PM

markj added reviewers: arichardson, emaste.Apr 20 2021, 4:27 PM

markj added inline comments.Apr 20 2021, 4:35 PM

.clang-format
5	Hrm, this setting only works with LLVM 12 and up. I guess it should be commented out for now.

Disable AlignConsecutiveMacros with a comment.
Make sure clang-format doesn't insert a space after FOREACH macros.

Harbormaster completed remote builds in B38734: Diff 87823.Apr 20 2021, 4:40 PM

Now it is discouraged in new code.

We certainly don't require it now, but I'm not sure we went as far as discouraging it (although IMO we should).

In D29870#670323, @emaste wrote:

Now it is discouraged in new code.

We certainly don't require it now, but I'm not sure we went as far as discouraging it (although IMO we should).

That's true. Now style(9) makes no mention of it, so I'm still fairly sure clang-format shouldn't bother preserving it. Outside of that case, we shouldn't have newlines at the start of blocks anyway.

In D29870#670333, @markj wrote:

In D29870#670323, @emaste wrote:

Now it is discouraged in new code.

We certainly don't require it now, but I'm not sure we went as far as discouraging it (although IMO we should).

That's true. Now style(9) makes no mention of it, so I'm still fairly sure clang-format shouldn't bother preserving it. Outside of that case, we shouldn't have newlines at the start of blocks anyway.

I think we've stopped requiring it and the vast majority of new code where it matters choses the new style.

LLVM 12 adds another option that we should enable for consistency:

# We use "void * const *x" and not "void *const *x"
# Note: requires clang-format 12
SpaceAroundPointerQualifiers: Before

I added this setting upstream (https://reviews.llvm.org/D88227) since I got frustrated by clang-format breaking my CheriBSD diffs (we hit this quite a lot with the __capability qualifier in our kernel sources).

arichardson added inline comments.Sep 6 2021, 8:25 AM