https://github.com/zeyi2 created https://github.com/llvm/llvm-project/pull/209385
None >From b416121697edb8aec89068d9657a01824b2109c1 Mon Sep 17 00:00:00 2001 From: Zeyi Xu <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
