[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
Miklos Vajna via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Nov 10 12:23:34 PST 2018
vmiklos added a comment.
> What do you think about code like:
>
> #if FOO == 4
> #if FOO == 4
> #endif
> #endif
>
> #if defined(FOO)
> #if defined(FOO)
> #endif
> #endif
>
> #if !defined(FOO)
> #if !defined(FOO)
> #endif
> #endif
I looked at supporting these, but it's not clear to me how to get e.g. the 'FOO
4' string (the condition) from the `If()` callback. So far I only see how to
----------------------------------------------------------------------------
get the start location of the condition and the whole range till the end of
endif. Do you have a code pointer how to do this, please? (Or OK if I leave
this for a follow-up patch?)
> #if defined(FOO)
> #if !defined(FOO)
> #endif
> #endif
>
> #if !defined(FOO)
> #if defined(FOO)
> #endif
> #endif
I expect handling these correctly is more complex -- I would prefer focusing on
matching conditons in this patch, and get back to inverted conditions in a
follow-up patch. Is that OK? If we handle inverted conditions, then handling `a
&& b` and `!a || !b` would make sense as well for example.
> Please keep this list sorted alphabetically.
Done.
> This name is not particularly descriptive. This seems to be more like
> `CheckMacroRedundancy` or something like that?
Makes sense, done.
> This comment should be re-flowed to fit the column width.
Done.
> What constitutes "redundancy"? A bit more exposition here would be useful.
Hopefully "nested directives with the same condition" makes it easier to
understand the intention and current scope.
https://reviews.llvm.org/D54349
More information about the cfe-commits
mailing list