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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
