LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added a comment.
In D123479#3443401 <https://reviews.llvm.org/D123479#3443401>, @aaron.ballman wrote: > In D123479#3442968 <https://reviews.llvm.org/D123479#3442968>, > @LegalizeAdulthood wrote: > >> In D123479#3442062 <https://reviews.llvm.org/D123479#3442062>, @njames93 >> wrote: >> >>> Would it not be better if these parens were stripped in the fixit as they >>> are unnecessary in the enum value decl? > > To me, that would be ideal, but not strictly necessary (and perhaps doesn't > scale super well across all fix-its) because the code is still correct with > the parens, just a bit ugly. We have other similar situations with fix-its > (like formatting). > >> I prefer checks to do one thing only in a straightforward manner. >> >> It's easier to place things before and after the macro expansion rather than >> modify the contents >> of the expansion. >> >> It doesn't feel like the responsibility of this check is to decide which >> parentheses are necessary or not. > > Hmm, I'm more on the fence. Me too. I am just not a fan of an automated check doing "other things" to my code that aren't advertised in the description. This is a macro-to-enum check not a redundant-parens check. BTW, many IDEs highlight unnecessary parentheses and have "quick fixes" to remove them, ReSharper for C++ is one such IDE add-on that does this. I'd prefer there to be a readability-redundant-parentheses check that removes unnecessary parentheses across the board everywhere, rather than it being a weird side-effect of turning my macros into enums. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123479/new/ https://reviews.llvm.org/D123479 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
