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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
ishaangandhi requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The clang-tidy check `bugprone-branch-clone` has a false positive if some symbols are undefined. This patch silences the warning when the two sides of a branch are invalid.

See //github.com/llvm/llvm-project/issues/56057 for the original issue.


Repository:
  rG LLVM Github Monorepo

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.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone.cpp
@@ -191,6 +191,16 @@
     return a + b * c;
 }
 
+// Unknown expressions are ignored
+int test_basic18() {
+  if (unknown_expression_1) {
+    function1(unknown_expression_1);
+  }
+  else {
+    function2(unknown_expression_2);
+  }
+}
+
 //=========--------------------==========//
 
 #define PASTE_CODE(x) x
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  <clang-tidy/checks/bugprone-branch-clone>` when the branches
+  involve unknown expressions.
+
 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 Expr *LHSExpr = llvm::dyn_cast<Expr>(LHS);
+    const Expr *RHSExpr = llvm::dyn_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.439219.patch
Type: text/x-patch
Size: 2096 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220623/b0c17f50/attachment-0001.bin>