Author: Nathan James Date: 2022-08-27T19:55:08+01:00 New Revision: 6bd98b4f2df02955b72343038ecf20cdfd23e01e
URL: https://github.com/llvm/llvm-project/commit/6bd98b4f2df02955b72343038ecf20cdfd23e01e DIFF: https://github.com/llvm/llvm-project/commit/6bd98b4f2df02955b72343038ecf20cdfd23e01e.diff LOG: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition Fixed a false positive where a lambda expression in the condition which contained an assignement would trigger a warning. Fixes #56729 Reviewed By: gribozavr2 Differential Revision: https://reviews.llvm.org/D132786 Added: Modified: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp index e4f52191e0b00..590505c1f47ba 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp @@ -17,23 +17,50 @@ namespace tidy { namespace bugprone { void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(ifStmt(hasCondition(forEachDescendant( - binaryOperator(isAssignmentOperator()) - .bind("assignment_in_if_statement")))), - this); - Finder->addMatcher(ifStmt(hasCondition(forEachDescendant( - cxxOperatorCallExpr(isAssignmentOperator()) - .bind("assignment_in_if_statement")))), - this); + Finder->addMatcher(translationUnitDecl(), this); } void AssignmentInIfConditionCheck::check( - const MatchFinder::MatchResult &Result) { - const auto *MatchedDecl = - Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement"); - if (!MatchedDecl) { - return; - } + const ast_matchers::MatchFinder::MatchResult &Result) { + class Visitor : public RecursiveASTVisitor<Visitor> { + AssignmentInIfConditionCheck &Check; + + public: + explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {} + bool VisitIfStmt(IfStmt *If) { + class ConditionVisitor : public RecursiveASTVisitor<ConditionVisitor> { + AssignmentInIfConditionCheck &Check; + + public: + explicit ConditionVisitor(AssignmentInIfConditionCheck &Check) + : Check(Check) {} + + // Dont traverse into any lambda expressions. + bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) { + return true; + } + + bool VisitBinaryOperator(BinaryOperator *BO) { + if (BO->isAssignmentOp()) + Check.report(BO); + return true; + } + + bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *OCE) { + if (OCE->isAssignmentOp()) + Check.report(OCE); + return true; + } + }; + + ConditionVisitor(Check).TraverseStmt(If->getCond()); + return true; + } + }; + Visitor(*this).TraverseAST(*Result.Context); +} + +void AssignmentInIfConditionCheck::report(const Expr *MatchedDecl) { diag(MatchedDecl->getBeginLoc(), "an assignment within an 'if' condition is bug-prone"); diag(MatchedDecl->getBeginLoc(), diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h index 8e57f1a7ca80d..f84ae93ed2ebf 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h @@ -25,6 +25,7 @@ class AssignmentInIfConditionCheck : public ClangTidyCheck { : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void report(const Expr *E); }; } // namespace bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1656debda1368..1734f4acf6865 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -116,6 +116,10 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Fixed a false positive in :doc:`bugprone-assignment-in-if-condition + <clang-tidy/checks/bugprone/assignment-in-if-condition>` check when there + was an assignement in a lambda found in the condition of an ``if``. + - Improved :doc:`bugprone-signal-handler <clang-tidy/checks/bugprone/signal-handler>` check. Partial support for C++14 signal handler rules was added. Bug report generation was diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp index 49e52edb3f3d7..20c30d73516b3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp @@ -101,3 +101,20 @@ void awesome_f4(int arg) { f = 5; } } + +template <typename Func> bool exec(Func F) { return F(); } + +void lambda_if() { + int X; + if ([&X] { + X = 5; + return true; + }()) { + } + + if (exec([&] { + X = 5; + return true; + })) { + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
