[clang-tools-extra] c4b8985 - [clang-tidy] Fix nested macro false positives in misc-redundant-expression (#209385)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 26 05:37:53 PDT 2026
Author: Zeyi Xu
Date: 2026-07-26T12:37:48Z
New Revision: c4b89858f8f2d52d18007bc66a3a827c6bfa8151
URL: https://github.com/llvm/llvm-project/commit/c4b89858f8f2d52d18007bc66a3a827c6bfa8151
DIFF: https://github.com/llvm/llvm-project/commit/c4b89858f8f2d52d18007bc66a3a827c6bfa8151.diff
LOG: [clang-tidy] Fix nested macro false positives in misc-redundant-expression (#209385)
Apply the existing macro-origin filtering when comparing flattened
operands in nested expressions.
Closes https://github.com/llvm/llvm-project/issues/209373
Added:
Modified:
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
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 0475186862913..89dab5aa0ea9b 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -404,6 +404,76 @@ static bool hasSameOperatorParent(const Expr *TheExpr,
return false;
}
+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());
+}
+
+static bool isTokAtEndOfExpr(SourceRange ExprSR, Token T,
+ const SourceManager &SM) {
+ return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
+}
+
+/// Returns true if both LhsExpr 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;
+ const SourceRange Lsr = LhsExpr->getSourceRange();
+ const SourceRange Rsr = RhsExpr->getSourceRange();
+ if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
+ return false;
+
+ const SourceManager &SM = AstCtx->getSourceManager();
+ const LangOptions &LO = AstCtx->getLangOpts();
+
+ const std::pair<FileID, unsigned> LsrLocInfo =
+ SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
+ const std::pair<FileID, unsigned> RsrLocInfo =
+ SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
+ const llvm::MemoryBufferRef MB = SM.getBufferOrFake(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,
+ const Expr *&RhsExpr) {
+ if (!LhsExpr || !RhsExpr)
+ return false;
+
+ const SourceLocation LhsLoc = LhsExpr->getExprLoc();
+ const SourceLocation RhsLoc = RhsExpr->getExprLoc();
+
+ return LhsLoc.isMacroID() != RhsLoc.isMacroID();
+}
+
template <typename TExpr>
static bool
markDuplicateOperands(const TExpr *TheExpr,
@@ -440,12 +510,17 @@ markDuplicateOperands(const TExpr *TheExpr,
if (AllOperands[J]->HasSideEffects(Context))
break;
- if (areEquivalentExpr(AllOperands[I], AllOperands[J])) {
- FoundDuplicates = true;
- Duplicates.set(J);
- Builder->setBinding(SmallString<11>(llvm::formatv("duplicate{0}", J)),
- DynTypedNode::create(*AllOperands[J]));
- }
+ const Expr *Lhs = AllOperands[I];
+ const Expr *Rhs = AllOperands[J];
+ if (!areEquivalentExpr(Lhs, Rhs) ||
+ areExprsFromDifferentMacros(Lhs, Rhs, &Context) ||
+ areExprsMacroAndNonMacro(Lhs, Rhs))
+ continue;
+
+ FoundDuplicates = true;
+ Duplicates.set(J);
+ Builder->setBinding(SmallString<11>(llvm::formatv("duplicate{0}", J)),
+ DynTypedNode::create(*Rhs));
}
if (FoundDuplicates)
@@ -835,76 +910,6 @@ 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());
-}
-
-static bool isTokAtEndOfExpr(SourceRange ExprSR, Token T,
- const SourceManager &SM) {
- return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
-}
-
-/// Returns true if both LhsExpr 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;
- const SourceRange Lsr = LhsExpr->getSourceRange();
- const SourceRange Rsr = RhsExpr->getSourceRange();
- if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
- return false;
-
- const SourceManager &SM = AstCtx->getSourceManager();
- const LangOptions &LO = AstCtx->getLangOpts();
-
- const std::pair<FileID, unsigned> LsrLocInfo =
- SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
- const std::pair<FileID, unsigned> RsrLocInfo =
- SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
- const llvm::MemoryBufferRef MB = SM.getBufferOrFake(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,
- const Expr *&RhsExpr) {
- if (!LhsExpr || !RhsExpr)
- return false;
-
- const SourceLocation LhsLoc = LhsExpr->getExprLoc();
- const SourceLocation RhsLoc = RhsExpr->getExprLoc();
-
- return LhsLoc.isMacroID() != RhsLoc.isMacroID();
-}
-
static bool areStringsSameIgnoreSpaces(const StringRef Left,
const StringRef Right) {
if (Left == Right)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index dd397b574b6f4..9a5a23f3d8542 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,11 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`misc-redundant-expression
+ <clang-tidy/checks/misc/redundant-expression>` by fixing false positives in
+ nested expressions involving
diff erent macros or a mix of macro and
+ non-macro operands.
+
- Improved :doc:`readability-named-parameter
<clang-tidy/checks/readability/named-parameter>` check by ignoring
standard tag types (e.g. ``std::in_place_t``, ``std::allocator_arg_t``,
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 327f7e81e1f3a..0678208ad9017 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
@@ -92,6 +92,20 @@ int TestSimpleEquivalent(int X, int Y) {
return 0;
}
+#define NESTED_MACRO_A 0x0100
+#define NESTED_MACRO_B 0x0200
+#define NESTED_MACRO_AB (NESTED_MACRO_A | NESTED_MACRO_B)
+
+int TestNestedMacroOperands() {
+ int Result = NESTED_MACRO_A | NESTED_MACRO_AB | NESTED_MACRO_B;
+ Result |= NESTED_MACRO_A | NESTED_MACRO_B | NESTED_MACRO_A;
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: operator has equivalent nested operands
+ Result |= NESTED_MACRO_AB | 0x0400 | NESTED_MACRO_AB;
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: operator has equivalent nested operands
+ Result |= NESTED_MACRO_A | NESTED_MACRO_B | 0x0100;
+ return Result;
+}
+
#ifndef TEST_MACRO
#define VAL_1 2
#define VAL_3 3
More information about the cfe-commits
mailing list