[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 18:36:40 PST 2023


owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.

In D155529#4509686 <https://reviews.llvm.org/D155529#4509686>, @MyDeveloperDay wrote:

> We should never make assumptions about what people do/don't use



> If you have this you have to honour it.. if 'SpacesInParentheses ' was true then 'InAttributeSpecifiers' needs to be true by default, shouldn't there be something in the expanding out of SpacesInParentheses

Because `SpacesInParentheses` was added 10+ years ago in rGb55acad91c37 <https://reviews.llvm.org/rGb55acad91c37bf70455e8ff1803fc1a0b10ad859> and `__attribute__((foo))` was not in the unit tests, it's probably a bug that the double parens were not excluded. Not sure whether the users who didn't complain about it really wanted it or simply didn't bother. The only way to find out would be to fix the bug, assuming it indeed was a bug.

When fixing bugs, especially the very old ones, we often have to choose between just fixing the bugs and adding an option to avoid "regressions", and I usually prefer the former. Nevertheless, I would have no problem if this new option is extended to handle all double parens, e.g. `if (( i = j ))`, `decltype(( x ))`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155529



More information about the cfe-commits mailing list