[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