[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439596.
ishaangandhi added a comment.

Alphabetize some more


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128402/new/

https://reviews.llvm.org/D128402

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {        // CHECK-MESSAGES: :[[@LINE]]:7: error: use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+    function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  } else {
+    function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: error: use of undeclared identifier 'unknown_expression_3' [clang-diagnostic-error]
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,6 +153,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   <clang-tidy/checks/altera/struct-pack-align>` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  <clang-tidy/checks/bugprone/branch-clone>` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   <clang-tidy/checks/bugprone/infinite-loop>` involving dependent expressions.
 
@@ -160,6 +164,15 @@
   <clang-tidy/checks/bugprone/sizeof-expression>` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Fixed bugs in :doc:`bugprone-use-after-move
+  <clang-tidy/checks/bugprone/use-after-move>`:
+
+  - Treat a move in a lambda capture as happening in the function that defines
+    the lambda, not within the body of the lambda (as we were previously doing
+    erroneously).
+
+  - Don't emit an erroneous warning on self-moves.
+
 - Made :doc:`cert-oop57-cpp <clang-tidy/checks/cert/oop57-cpp>` more sensitive
   by checking for an arbitrary expression in the second argument of ``memset``.
 
@@ -205,6 +218,10 @@
   <clang-tidy/checks/performance/inefficient-vector-operation>` to work when
   the vector is a member of a structure.
 
+- Fixed a crash in :doc:`performance-unnecessary-value-param
+  <clang-tidy/checks/readability/suspicious-call-argument>` when the specialization
+  template has an unnecessary value parameter. Removed the fix for a template.
+
 - Fixed a crash in :doc:`readability-const-return-type
   <clang-tidy/checks/readability/const-return-type>` when a pure virtual function
   overrided has a const return type. Removed the fix for a virtual function.
@@ -220,19 +237,6 @@
   <clang-tidy/checks/readability/simplify-boolean-expr>` to simplify expressions
   using DeMorgan's Theorem.
 
-- Fixed a crash in :doc:`performance-unnecessary-value-param
-  <clang-tidy/checks/readability/suspicious-call-argument>` when the specialization
-  template has an unnecessary value parameter. Removed the fix for a template.
-
-- Fixed bugs in :doc:`bugprone-use-after-move
-  <clang-tidy/checks/bugprone/use-after-move>`:
-
-  - Treat a move in a lambda capture as happening in the function that defines
-    the lambda, not within the body of the lambda (as we were previously doing
-    erroneously).
-
-  - Don't emit an erroneous warning on self-moves.
-
 Removed checks
 ^^^^^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -19,6 +19,17 @@
 /// Returns true when the statements are Type I clones of each other.
 static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
                                    const ASTContext &Context) {
+  if (isa<Expr>(LHS) && isa<Expr>(RHS)) {
+    // If we have errors in expressions, we will be unable
+    // to accurately profile and compute hashes for each
+    // of the left and right statements.
+    const auto *LHSExpr = llvm::cast<Expr>(LHS);
+    const auto *RHSExpr = llvm::cast<Expr>(RHS);
+    if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+      return false;
+    }
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128402.439596.patch
Type: text/x-patch
Size: 4492 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220624/a5f65c15/attachment.bin>