dkrupp updated this revision to Diff 214529. dkrupp added a comment. @aaron.ballman 's comments are fixed.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55125/new/ https://reviews.llvm.org/D55125 Files: clang-tidy/misc/RedundantExpressionCheck.cpp test/clang-tidy/misc-redundant-expression.cpp
Index: test/clang-tidy/misc-redundant-expression.cpp =================================================================== --- test/clang-tidy/misc-redundant-expression.cpp +++ test/clang-tidy/misc-redundant-expression.cpp @@ -114,6 +114,7 @@ #define COND_OP_MACRO 9 #define COND_OP_OTHER_MACRO 9 +#define COND_OP_THIRD_MACRO COND_OP_MACRO int TestConditional(int x, int y) { int k = 0; k += (y < 0) ? x : x; @@ -122,16 +123,33 @@ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO; // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent + k += (y < 0) ? COND_OP_MACRO + COND_OP_OTHER_MACRO : COND_OP_MACRO + COND_OP_OTHER_MACRO; + // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'true' and 'false' expressions are equivalent // Do not match for conditional operators with a macro and a const. k += (y < 0) ? COND_OP_MACRO : 9; // Do not match for conditional operators with expressions from different macros. k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO; + // Do not match for conditional operators when a macro is defined to another macro + k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO; +#undef COND_OP_THIRD_MACRO +#define COND_OP_THIRD_MACRO 8 + k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO; +#undef COND_OP_THIRD_MACRO + + k += (y < 0) ? sizeof(I64) : sizeof(I64); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 'true' and 'false' expressions are equivalent + k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(TestConditional(k,y)); + // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'true' and 'false' expressions are equivalent + // No warning if the expression arguments are different. + k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(Valid(k,y)); + return k; } #undef COND_OP_MACRO #undef COND_OP_OTHER_MACRO + // Overloaded operators that compare two instances of a struct. struct MyStruct { int x; Index: clang-tidy/misc/RedundantExpressionCheck.cpp =================================================================== --- clang-tidy/misc/RedundantExpressionCheck.cpp +++ clang-tidy/misc/RedundantExpressionCheck.cpp @@ -131,6 +131,20 @@ case Stmt::BinaryOperatorClass: return cast<BinaryOperator>(Left)->getOpcode() == cast<BinaryOperator>(Right)->getOpcode(); + case Stmt::UnaryExprOrTypeTraitExprClass: + const auto *LeftUnaryExpr = + cast<UnaryExprOrTypeTraitExpr>(Left); + const auto *RightUnaryExpr = + cast<UnaryExprOrTypeTraitExpr>(Right); + if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType()) + return LeftUnaryExpr->getArgumentType() == + RightUnaryExpr->getArgumentType(); + else if (!LeftUnaryExpr->isArgumentType() && + !RightUnaryExpr->isArgumentType()) + return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(), + RightUnaryExpr->getArgumentExpr()); + + return false; } } @@ -604,23 +618,61 @@ return true; } +static bool isSameToken(const Token &T1, const Token &T2, + const SourceManager &SM) { + if (T1.getKind() != T2.getKind()) + return false; + if (T1.isNot(tok::raw_identifier)) + return true; + if (T1.getLength() != T2.getLength()) + return false; + return strncmp(SM.getCharacterData(T1.getLocation()), + SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0; +} + +bool isTokAtEndOfExpr(SourceRange ExprSR, Token T, const SourceManager &SM) { + return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation(); +} + +/// Returns true if both LhsEpxr and RhsExpr are +/// macro expressions and they are expanded +/// from different macros. static bool areExprsFromDifferentMacros(const Expr *LhsExpr, const Expr *RhsExpr, const ASTContext *AstCtx) { if (!LhsExpr || !RhsExpr) return false; - - SourceLocation LhsLoc = LhsExpr->getExprLoc(); - SourceLocation RhsLoc = RhsExpr->getExprLoc(); - - if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID()) + SourceRange Lsr = LhsExpr->getSourceRange(); + SourceRange Rsr = RhsExpr->getSourceRange(); + if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID()) return false; const SourceManager &SM = AstCtx->getSourceManager(); const LangOptions &LO = AstCtx->getLangOpts(); - return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) == - Lexer::getImmediateMacroName(RhsLoc, SM, LO)); + std::pair<FileID, unsigned> LsrLocInfo = + SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin())); + std::pair<FileID, unsigned> RsrLocInfo = + SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin())); + const llvm::MemoryBuffer *MB = SM.getBuffer(LsrLocInfo.first); + + const char *LTokenPos = MB->getBufferStart() + LsrLocInfo.second; + const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second; + Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO, + MB->getBufferStart(), LTokenPos, MB->getBufferEnd()); + Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO, + MB->getBufferStart(), RTokenPos, MB->getBufferEnd()); + + Token LTok, RTok; + do { // Compare the expressions token-by-token. + LRawLex.LexFromRawLexer(LTok); + RRawLex.LexFromRawLexer(RTok); + } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) && + isSameToken(LTok, RTok, SM) && !isTokAtEndOfExpr(Lsr, LTok, SM) && + !isTokAtEndOfExpr(Rsr, RTok, SM)); + return !( + (isTokAtEndOfExpr(Lsr, LTok, SM) && isTokAtEndOfExpr(Rsr, RTok, SM)) && + isSameToken(LTok, RTok, SM)); } static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits