[clang-tools-extra] [clang-tidy] Fix nested macro false positives in misc-redundant-expression (PR #209385)
Zeyi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 15 02:27:46 PDT 2026
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/209385
>From b416121697edb8aec89068d9657a01824b2109c1 Mon Sep 17 00:00:00 2001
From: Zeyi Xu <mitchell.xu2 at gmail.com>
Date: Tue, 14 Jul 2026 14:22:31 +0800
Subject: [PATCH] [clang-tidy] Fix nested macro false positives in
misc-redundant-expression
---
.../misc/RedundantExpressionCheck.cpp | 157 +++++++++---------
clang-tools-extra/docs/ReleaseNotes.rst | 5 +-
.../checkers/misc/redundant-expression.cpp | 14 ++
3 files changed, 99 insertions(+), 77 deletions(-)
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 different 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 different 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 cd568efb274ff..608b72d3138fd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -605,9 +605,12 @@ Changes in existing checks
- Fixed a crash when evaluating bitwise comparisons against integer constants
wider than 64 bits.
- - Avoided false positives when comparing expressions that are structurally
+ - Fixed false positives when comparing expressions that are structurally
identical but use different type aliases.
+ - Fixed false positives in nested expressions involving different macros or
+ a mix of macro and non-macro operands.
+
- Improved :doc:`misc-throw-by-value-catch-by-reference
<clang-tidy/checks/misc/throw-by-value-catch-by-reference>` check:
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