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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
