Page MenuHomePhabricator

[clang-format] [PR45816] Add AlignConsecutiveBitFields
AbandonedPublic

Authored by MyDeveloperDay on May 18 2020, 3:20 AM.

Details

Summary

https://bugs.llvm.org/show_bug.cgi?id=45816

The following revision adds the ability to align consecutive bitfield similar to AlignConsecutiveAssignments and AlignConsecutiveDeclarations

This will format the following code from

struct Options {
  bool some_field : 1;
  bool some_another_field : 1;
  int yet_another_field : 1;
};

With the following consiguration change

...
AlignConsecutiveBitFields: 'true'
...

To be

struct Options {
  bool some_field         : 1;
  bool some_another_field : 1;
  int  yet_another_field  : 1;
};

Diff Detail

Event Timeline

MyDeveloperDay created this revision.May 18 2020, 3:20 AM
MyDeveloperDay edited the summary of this revision. (Show Details)May 18 2020, 3:39 AM
MyDeveloperDay edited the summary of this revision. (Show Details)

Ironically, I independently made a near-identical change because my codebase needs this, and was going to upload it today. I won't post it now, but I stuck a copy on github if you're interested: https://github.com/JakeMerdichAMD/llvm-project/commit/6eec9ce0bb7732a519b765659422b72a9fa8aa67. I also have some more tests there, and will cross-check with this once the merge conflicts are fixed.

clang/lib/Format/WhitespaceManager.cpp
98

This needs to be before assignments, because 'int foo : 3 = 1;' is valid in c++2a.

588–590

TT_BitFieldColon should match this criteria exactly.

MyDeveloperDay planned changes to this revision.May 18 2020, 7:46 AM
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added inline comments.
clang/lib/Format/WhitespaceManager.cpp
588–590

I think using TT_BitFieldColon is a better idea

How about we combine the two revisions

Good idea, feel free to incorporate anything about the change (eg, tests and TT_BitFieldColon) or delegate to me if you like.

How about I abandon and we use yours (but would you please consider changing the name to be AlignConsecutiveBitFields ) (capital F) as that was in the orignal request

MyDeveloperDay abandoned this revision.May 19 2020, 12:44 AM