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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
