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
