dkrupp created this revision.
dkrupp added reviewers: alexfh, aaron.ballman, gamesh411.
dkrupp added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, Szelethus, rnkovacs.
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.
We used to get a false warning in the following case:
define M1 <https://reviews.llvm.org/M1> 1
#define M2 M1 <https://reviews.llvm.org/M1>
int test(int cond){
return cond ? M1 <https://reviews.llvm.org/M1> : M2;//false warning: 'true'
and 'false' expressions are equivalent
}
The problem was that the Lexer::getImmediateMacroName() call was used for
comparing macros, which returned "M1 <https://reviews.llvm.org/M1>" for the
expansion of M1 <https://reviews.llvm.org/M1> and M2.
Now we are comparing macros from token to token.
Repository:
rCTE Clang Tools Extra
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,11 +115,15 @@
// 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;
return k;
}
#undef COND_OP_MACRO
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===================================================================
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -598,23 +598,55 @@
return true;
}
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM){
+ if (T1.getLength()!=T2.getLength())
+ return false;
+ return
strncmp(SM.getCharacterData(T1.getLocation()),SM.getCharacterData(T2.getLocation()),T1.getLength())==0;
+}
+
+//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;
+ clang::Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+ MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
+ clang::Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
+ MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
+
+ clang::Token LTok, RTok;
+ SourceLocation LLoc, RLoc;
+ int LRem, RRem;
+ do {//we compare the expressions token-by-token
+ LRawLex.LexFromRawLexer(LTok);
+ RRawLex.LexFromRawLexer(RTok);
+ LLoc = LTok.getLocation();
+ RLoc = RTok.getLocation();
+ LRem = SM.getCharacterData(SM.getExpansionLoc(Lsr.getEnd())) -
+ SM.getCharacterData(LLoc);//remaining characters until the end of
expr
+ RRem = SM.getCharacterData(SM.getExpansionLoc(Rsr.getEnd())) -
+ SM.getCharacterData(RLoc);
+ } while (!LTok.is(clang::tok::eof) && !RTok.is(clang::tok::eof) &&
+ compareToks(LTok, RTok, SM) && LRem > 0 && RRem > 0);
+ return !((LRem == 0 && RRem == 0) && compareToks(LTok, RTok, SM));
}
static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
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,11 +115,15 @@
// 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;
return k;
}
#undef COND_OP_MACRO
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===================================================================
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -598,23 +598,55 @@
return true;
}
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM){
+ if (T1.getLength()!=T2.getLength())
+ return false;
+ return strncmp(SM.getCharacterData(T1.getLocation()),SM.getCharacterData(T2.getLocation()),T1.getLength())==0;
+}
+
+//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;
+ clang::Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+ MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
+ clang::Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
+ MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
+
+ clang::Token LTok, RTok;
+ SourceLocation LLoc, RLoc;
+ int LRem, RRem;
+ do {//we compare the expressions token-by-token
+ LRawLex.LexFromRawLexer(LTok);
+ RRawLex.LexFromRawLexer(RTok);
+ LLoc = LTok.getLocation();
+ RLoc = RTok.getLocation();
+ LRem = SM.getCharacterData(SM.getExpansionLoc(Lsr.getEnd())) -
+ SM.getCharacterData(LLoc);//remaining characters until the end of expr
+ RRem = SM.getCharacterData(SM.getExpansionLoc(Rsr.getEnd())) -
+ SM.getCharacterData(RLoc);
+ } while (!LTok.is(clang::tok::eof) && !RTok.is(clang::tok::eof) &&
+ compareToks(LTok, RTok, SM) && LRem > 0 && RRem > 0);
+ return !((LRem == 0 && RRem == 0) && compareToks(LTok, RTok, SM));
}
static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
