[clang-tools-extra] 1caa66d - Fix a false positive in misc-redundant-expression check
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 30 10:38:38 PDT 2019
Author: Aaron Ballman
Date: 2019-10-30T13:38:25-04:00
New Revision: 1caa66d0759f6bd0851a40645afac8e8a7f84341
URL: https://github.com/llvm/llvm-project/commit/1caa66d0759f6bd0851a40645afac8e8a7f84341
DIFF: https://github.com/llvm/llvm-project/commit/1caa66d0759f6bd0851a40645afac8e8a7f84341.diff
LOG: Fix a false positive in misc-redundant-expression check
Do not warn for redundant conditional expressions when the true and false
branches are expanded from different macros even when they are defined by
one another.
Patch by Daniel Krupp.
Added:
Modified:
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index e646ee91ac52..e3e84b71601b 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -131,6 +131,20 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
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,62 @@ static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
return true;
}
+static bool isSameRawIdentifierToken(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 StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
+ StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
+}
+
+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
diff erent 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) &&
+ isSameRawIdentifierToken(LTok, RTok, SM) &&
+ !isTokAtEndOfExpr(Lsr, LTok, SM) &&
+ !isTokAtEndOfExpr(Rsr, RTok, SM));
+ return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
+ !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
+ !isSameRawIdentifierToken(LTok, RTok, SM);
}
static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
index f6b47eb79fb9..500478159a42 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -114,6 +114,7 @@ int Valid(int X, int Y) {
#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,11 +123,27 @@ int TestConditional(int x, int y) {
// 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
diff erent 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
diff erent.
+ k += (y < 0) ? sizeof(TestConditional(k,y)) : sizeof(Valid(k,y));
+
return k;
}
#undef COND_OP_MACRO
@@ -134,7 +151,7 @@ int TestConditional(int x, int y) {
// Overloaded operators that compare two instances of a struct.
struct MyStruct {
- int x;
+ int x;
bool operator==(const MyStruct& rhs) const {return this->x == rhs.x; } // not modifing
bool operator>=(const MyStruct& rhs) const { return this->x >= rhs.x; } // not modifing
bool operator<=(MyStruct& rhs) const { return this->x <= rhs.x; }
More information about the cfe-commits
mailing list