[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

[Anders Waldenborg via Phabricator via cfe-commits]

wanders marked an inline comment as done.
wanders added a comment.

In D84090#2160411 <https://reviews.llvm.org/D84090#2160411>, @curdeius wrote:

> The changes look good to me in general. I share your doubt though about whether a bool flag is sufficient here. We've seen in the past a few times that at some time a false/true flag is not enough. I'd rather go for a Before/After/Both/None flag (or similar, naming probably should be coherent with other flags). But I'm not really aware of the projects/coding styles that use bit fields. Maybe a small research on this would be good to confirm or infirm a necessity of higher complexity.


Did some basic research in repos I happened to have.

Here I call the different styles:  "Neither" - no spaces around colon. "Both" space on both sides. "Left" space on left side only. "Right" space on right side only. ("Right" is of course subject to AlignConsecutiveBitField, so if alignment requires there to be spaces there will be space).

In summary - in the projects I looked at - the uses doesn't seem to be very consistent, can even use different styles within same struct. Most projects mixes "Both" and "Neither".  "Right" doesn't really seem to be used - it seems to be typos when it is.  "Left" is used in some cases when there are aligment for consecutive lines. (keep in mind that there almost always is consecutive lines,  a single bitfield in a struct seldom makes any sense)..

There was one case in musl that really made me smile.  `include/arpa/nameser.h` struct HEADER has different bitfield members depending on endianness,  for big endian it uses "Right" and little endian it uses "Left".  ( https://git.musl-libc.org/cgit/musl/tree/include/arpa/nameser.h#n324 )

My guess is that no project's coding style document really specifies this detail (except implicitly for projects where the style is defined as "whatever clang-format says").

But I think the conclusion is that it probably should be more that true/false.  Pondering about doing it in steps, so first just `BitFieldColonSpacing` with values `Both` and `Neither` (i.e same functionality as in current patch) and then add `Before` as an additional option.

- qemu
  - mixing "Both" and "Neither"

- emacs
  - mixing "Both" and "Neither"

- git
  - mixing "Both" and "Neither"

- linux kernel
  - mixing "Both" and "Neither"
  - (also has a bunch of "unsigned x:1, y:2, z:3", which is interesting and seems to be missing tests for clang-format)
  - Has consecutive aligned variants

- gnupg
  - mixing "Both" and "Neither"
  - but mostly "Neither"
  - Also has one "Right": `agent/agent.h:    int raw_value: 1;`

- lwip
  - Mostly "Left" combined with aligned consecutive
  - Some "Both"

- intel-pcm
  - "Both"

- syslog-ng
  - "Neither"

- busybox
  - "Left" combined with aligned consecutive

- glibc
  - mixing "Both" and "Neither"

- gcc
  - mixing "Both" and "Neither"  (mostly "Both")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84090/new/

https://reviews.llvm.org/D84090