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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to