dkrupp updated this revision to Diff 179277.
dkrupp marked an inline comment as done.
dkrupp added a comment.
All comments fixed.
I also added the handling of redundancy comparison of sizeof(..), alignof()
operators.
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
@@ -132,6 +132,18 @@
case Stmt::BinaryOperatorClass:
return cast<BinaryOperator>(Left)->getOpcode() ==
cast<BinaryOperator>(Right)->getOpcode();
+ case Stmt::UnaryExprOrTypeTraitExprClass:
+ if (cast<UnaryExprOrTypeTraitExpr>(Left)->isArgumentType() &&
+ cast<UnaryExprOrTypeTraitExpr>(Right)->isArgumentType())
+ return cast<UnaryExprOrTypeTraitExpr>(Left)->getArgumentType() ==
+ cast<UnaryExprOrTypeTraitExpr>(Right)->getArgumentType();
+ else if (!cast<UnaryExprOrTypeTraitExpr>(Left)->isArgumentType() &&
+ !cast<UnaryExprOrTypeTraitExpr>(Right)->isArgumentType())
+ return areEquivalentExpr(
+ cast<UnaryExprOrTypeTraitExpr>(Left)->getArgumentExpr(),
+ cast<UnaryExprOrTypeTraitExpr>(Right)->getArgumentExpr());
+
+ return false;
}
}
@@ -598,23 +610,66 @@
return true;
}
+static bool isSameToken(Token &T1, Token &T2, const SourceManager &SM) {
+ if (T1.getKind()!=T2.getKind())
+ return false;
+ else 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;
+}
+
+unsigned getCharCountUntilEndOfExpr(SourceRange ExprSR, Token T,
+ const SourceManager &SM) {
+ assert((T.getLocation() < ExprSR.getBegin() ||
+ ExprSR.getEnd() < T.getLocation()) &&
+ "Token is not within the expression range!");
+
+ return SM.getCharacterData(SM.getExpansionLoc(ExprSR.getEnd())) -
+ SM.getCharacterData(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) &&
+ getCharCountUntilEndOfExpr(Lsr, LTok, SM) > 0 &&
+ getCharCountUntilEndOfExpr(Rsr, RTok, SM) > 0);
+ return !((getCharCountUntilEndOfExpr(Lsr, LTok, SM) == 0 &&
+ getCharCountUntilEndOfExpr(Rsr, RTok, SM) == 0) &&
+ isSameToken(LTok, RTok, SM));
}
static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
@@ -993,7 +1048,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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
