This revision was automatically updated to reflect the committed changes. Closed by commit rG6bd98b4f2df0: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition (authored by njames93).
Changed prior to commit: https://reviews.llvm.org/D132786?vs=456102&id=456139#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132786/new/ https://reviews.llvm.org/D132786 Files: 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
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp +++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp @@ -101,3 +101,20 @@ 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; + })) { + } +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ 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 + <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 Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h =================================================================== --- clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h +++ 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 Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp +++ 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<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(),
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits