[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a subscriber: donat.nagy.
Szelethus added a comment.

I see your point, but here's why I think it isn't a bug: I like to see macros as `constexpr` variables, and if I used those instead, I personally wouldn't like to get a warning just because they have the same value. In C, silencing such a warning isn't even really possible.

Another interesting thought, @donat.nagy's check works by comparing actual nodes of the AST, while this one would work with `Lexer`, but they almost want to do the same thing, the only difference is that the first checks whether two pieces of code are equivalent, and the second checks whether they are the same. Maybe it'd be worth to extract the logic into a simple `areStmtsEqual(const Stmt *S1, const Stmt *S2, bool ShouldCompareLexically = false)` function, that would do AST based comparison if the last param is set to false, and would use `Lexer` if set to true. After that, we could just add command line options to both of these checks, to leave it up to the user to choose in between them. Maybe folks who suffer from heavily macro-infested code would rather bear the obvious performance hit `Lexer` causes for little more precise warnings, and (for example) users of C++11 (because there are few excuses not to prefer `constexpr` there) could run in AST mode.

But I'm just thinking aloud really.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55125/new/

https://reviews.llvm.org/D55125