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

Reply via email to