diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h @@ -25,6 +25,7 @@ : 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/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp @@ -17,23 +17,50 @@ 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("assignment_in_if_statement"); - if (!MatchedDecl) { - return; - } + const ast_matchers::MatchFinder::MatchResult &Result) { + class Visitor : public RecursiveASTVisitor { + AssignmentInIfConditionCheck &Check; + + public: + explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {} + bool VisitIfStmt(IfStmt *If) { + class ConditionVisitor : public RecursiveASTVisitor { + 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -116,6 +116,10 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Fixed a false positive in :doc:`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 ` 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 --- 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 @@ f = 5; } } + +template bool exec(Func F) { return F(); } + +void lambda_if() { + int X; + if ([&X] { + X = 5; + return true; + }()) { + } + + if (exec([&] { + X = 5; + return true; + })) { + } +}