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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D55125#1314741 <https://reviews.llvm.org/D55125#1314741>, @Szelethus wrote:

> @JonasToth this is the `Lexer` based expression equality check I talked about in D54757#1311516 <https://reviews.llvm.org/D54757#1311516>.  The point of this patch is that the definition is a macro sure could be build dependent, but the macro name is not, so it wouldn't warn on the case I showed. What do you think?


Yes, this approach is possible.
IMHO it is still a bug/redudant if you do the same thing on both paths and warning on it makes the programmer aware of the fact. E.g. the macros might have been something different before, but a refactoring made them equal and resulted in this situation.



================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:601
 
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM){
+  if (T1.getLength()!=T2.getLength())
----------------
Please run clang-format


================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:630
+  const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second;
+  clang::Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+                       MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
----------------
`clang::` should not be necessary here, is it? `SourceLocation` and other types are access without namespace, too.


================
Comment at: test/clang-tidy/misc-redundant-expression.cpp:109
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {
----------------
Could you please add a test where the macro is `undef`ed and redefined to something else?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55125