dkrupp updated this revision to Diff 198041.
dkrupp marked 6 inline comments as done.
dkrupp added a comment.
I have fixed all your comments and rebased the patch to the latest master.
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
@@ -106,6 +106,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;
@@ -114,16 +115,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 UnaryExprOrTypeTraitExpr *LeftUnaryExpr =
+ cast<UnaryExprOrTypeTraitExpr>(Left);
+ const UnaryExprOrTypeTraitExpr *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;
}
}
@@ -513,7 +527,8 @@
Value = APSInt(32, false);
} else if (const auto *OverloadedOperatorExpr =
Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
- const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
+ const auto *OverloadedFunctionDecl =
+ dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
if (!OverloadedFunctionDecl)
return false;
@@ -529,7 +544,8 @@
Symbol = OverloadedOperatorExpr->getArg(0);
OperandExpr = OverloadedOperatorExpr;
- Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+ Opcode = BinaryOperator::getOverloadedOpcode(
+ OverloadedOperatorExpr->getOperator());
return BinaryOperator::isComparisonOp(Opcode);
} else {
@@ -547,7 +563,8 @@
}
// Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+ const ASTContext *AstCtx) {
const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
@@ -597,23 +614,60 @@
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,
@@ -829,7 +883,6 @@
((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
}
-
void RedundantExpressionCheck::checkBitwiseExpr(
const MatchFinder::MatchResult &Result) {
if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
@@ -874,8 +927,8 @@
ConstExpr))
return;
- if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
- return;
+ if ((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
+ return;
SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
@@ -992,7 +1045,6 @@
Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
const Expr *TrueExpr = CondOp->getTrueExpr();
const Expr *FalseExpr = CondOp->getFalseExpr();
-
if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
return;
@@ -1001,7 +1053,8 @@
}
if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
- const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
+ const auto *OverloadedFunctionDecl =
+ dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
if (!OverloadedFunctionDecl)
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
