[PATCH] D123479: [clang-tidy] Support parenthesized literals in modernize-macro-to-enum

Richard via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 11 11:43:01 PDT 2022


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



More information about the cfe-commits mailing list