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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 440071.

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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return fal

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

@LegalizeAdulthood Thanks! I don't have commit rights to the repository, can 
you commit it on my behalf?


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

Does anybody on this thread have land permissions? If not, would anyone know 
who to tag?


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Remove `-fix-errors`


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 %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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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 %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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->contains

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

I remember now, @njames93 :

Without the `-fix-errors`, the test fails as follows:

  Command Output (stdout):
  
  --
  
  Running ['clang-tidy', 
'/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp',
 '-fix', '--checks=-*,bugprone-branch-clone', '-config={}', '--', '-std=c++11', 
'-nostdinc++']...
  
  clang-tidy 
/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp
 -fix --checks=-*,bugprone-branch-clone -config={} -- -std=c++11 -nostdinc++ 
failed:
  
  3 errors generated.
  
  Error while processing 
/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp.
  
  
/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp:4:7:
 error: use of undeclared identifier 'unknown_expression_1' 
[clang-diagnostic-error]
  
if (unknown_expression_1) {//
  
^
  
  
/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp:5:15:
 error: use of undeclared identifier 'unknown_expression_2' 
[clang-diagnostic-error]
  
  function1(unknown_expression_2); //
  
^
  
  
/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/branch-clone-unknown-expr.cpp.tmp.cpp:7:15:
 error: use of undeclared identifier 'unknown_expression_3' 
[clang-diagnostic-error]
  
  function2(unknown_expression_3); //
  
^
  
  Found compiler errors, but -fix-errors was not specified.

I won't update the diff, since I assume you will want to see the buildkite 
failure for yourself.

Once you see it, can you either confirm `-fix-errors` was correct originally, 
or instruct me on how to fix this test failure?


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 441509.

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 -expect-clang-tidy-error %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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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 -expect-clang-tidy-error %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
@@ -158,6 +158,10 @@
 - Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErr

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

Sure!

Ishaan Gandhi
ishaangandhi AT gmail DOT com


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi created this revision.
ishaangandhi added reviewers: nridge, sammccall.
Herald added subscribers: usaxena95, kadircet.
Herald added a project: All.
ishaangandhi requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

This leads to ".C" files being rewritten as ".c" files and being inferred to be 
"c" files as opposed to "c++" files.

See https://github.com/clangd/clangd/issues/1108 for the original bug report.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp


Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

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

- Added a test case
- Removed redundant "StringRef" constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi marked an inline comment as done.
ishaangandhi added a comment.

Test case added, re-diffed with `-U`, and removed redundant constructor. 
Thanks for the quick feedback, @sammccall!

(I didn't wait for the tests to run locally, I am hoping to use your CI systems 
to do that. It seems like a simple enough of a test that I hope I can get it 
right on the first try.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 424906.

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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:852
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");

sammccall wrote:
> Is this really what you intend to test?
> 
> I think our goal here is to treat `.H` and `.C` as C++ rather than C, not to 
> link them together in some way. If the current file is `foo.H`, then 
> `foo.cpp` and `foo.C` should be equally compelling candidates, but both beat 
> `foo.c`.
Bleh brain fart. You're right. "exact.C" should win over "exact.c", not 
necessarily "exact.cpp".


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

https://reviews.llvm.org/D124262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Made test case reflect that proxies for ".H" files are ".C" files, and not ".c 
files".


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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

In D124262#3472079 , @sammccall wrote:

> Thanks!
> I can land this for you if you don't have commit access - can you provide the 
> name/email to use for the commit?

Thank you!

Ishaan Gandhi
ishaangan...@gmail.com


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

https://reviews.llvm.org/D124262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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
+  ` 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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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
+  ` 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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Rebased


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.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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Move test to new file with `-fix-errors` flag


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,11 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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,11 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) {
+function1(unknown_expression_1);
+  }
+  else {
+function2(unknown_expression_2);
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Check for error messages in test case


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,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: use of 
undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: use of 
undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: 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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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,10 @@
+// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t
+
+int test_unknown_expression() {
+  if (unknown_expression_1) { // CHECK-MESSAGES: :[[@LINE]]:7: use of undeclared identifier 'unknown_expression_1' [clang-diagnostic-error]
+function1(unknown_expression_2); // CHECK-MESSAGES: :[[@LINE]]:15: use of undeclared identifier 'unknown_expression_2' [clang-diagnostic-error]
+  }
+  else {
+function2(unknown_expression_3); // CHECK-MESSAGES: :[[@LINE]]:15: 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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Include `error: ` in error line


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,10 @@
+// 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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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,10 @@
+// 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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439240.

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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);
___
cfe-commit

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:28
+const Expr *RHSExpr = llvm::dyn_cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;

njames93 wrote:
> Surely we can bail if only one of the sides contains an errors.
Makes no difference, since when only one of the side has errors, the two 
profile hashes will be different.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

Eugene.Zelenko wrote:
> Documentation path was changed recently. Please also keep alphabetical order 
> inside section.
Hm? Its still at this path on mastesr: 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

ishaangandhi wrote:
> Eugene.Zelenko wrote:
> > Documentation path was changed recently. Please also keep alphabetical 
> > order inside section.
> Hm? Its still at this path on mastesr: 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/ReleaseNotes.rst
Also, it doesn't look alphabetized to me? Two points up is 
"performance-unnecessary-..." then its "bugprone-use-after...".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Use `auto` instead of `Expr` and `llvm::cast` instead of `llvm::dyn_cast`.


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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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
@@ -233,6 +233,10 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed a false positive in :doc:`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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->P

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

Eugene.Zelenko wrote:
> Eugene.Zelenko wrote:
> > ishaangandhi wrote:
> > > ishaangandhi wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Documentation path was changed recently. Please also keep 
> > > > > alphabetical order inside section.
> > > > Hm? Its still at this path on mastesr: 
> > > > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/ReleaseNotes.rst
> > > Also, it doesn't look alphabetized to me? Two points up is 
> > > "performance-unnecessary-..." then its "bugprone-use-after...".
> > Sorry, my mistake.
> It'll be good idea to fix order of other entries too.
Happy to! Would you like that in this diff or a new one?


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Alphabetize release notes


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
@@ -150,24 +150,29 @@
 Changes in existing checks
 ^^
 
-- Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
-  ` check for empty structs.
-
-- Fixed some false positives in :doc:`bugprone-infinite-loop
-  ` involving dependent expressions.
+- Expanded :doc:`readability-simplify-boolean-expr
+  ` to simplify expressions
+  using DeMorgan's Theorem.
 
 - Fixed a crash in :doc:`bugprone-sizeof-expression
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
-- Made :doc:`cert-oop57-cpp ` more sensitive
-  by checking for an arbitrary expression in the second argument of ``memset``.
+- Fixed a crash in :doc:`llvmlibc-callee-namespace
+  ` when executing for C++ code
+  that contain calls to advanced constructs, e.g. overloaded operators.
 
-- Improved :doc:`cppcoreguidelines-prefer-member-initializer
-  ` check.
+- Fixed a crash in :doc:`performance-unnecessary-value-param
+  ` when the specialization
+  template has an unnecessary value parameter. Removed the fix for a template.
 
-  Fixed an issue when there was already an initializer in the constructor and
-  the check would try to create another initializer for the same member.
+- Fixed a crash in :doc:`readability-const-return-type
+  ` when a pure virtual function
+  overrided has a const return type. Removed the fix for a virtual function.
+
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
   ` involving
@@ -175,18 +180,6 @@
   those cannot be used as base classes, consequently, they can not violate the
   rule.
 
-- Fixed a crash in :doc:`llvmlibc-callee-namespace
-  ` when executing for C++ code
-  that contain calls to advanced constructs, e.g. overloaded operators.
-
-- Fixed false positives in :doc:`misc-redundant-expression
-  `:
-
-  - Fixed a false positive involving overloaded comparison operators.
-
-  - Fixed a false positive involving assignments in
-conditions. This fixes `Issue 35853 `_.
-
 - Fixed a false positive in :doc:`misc-unused-parameters
   `
   where invalid parameters were implicitly being treated as being unused. 
@@ -201,28 +194,12 @@
   the ``CheckHeaderFile=true`` option if header files of the project only
   included by C++ source files.
 
-- Improved :doc:`performance-inefficient-vector-operation
-  ` to work when
-  the vector is a member of a structure.
-
-- Fixed a crash in :doc:`readability-const-return-type
-  ` when a pure virtual function
-  overrided has a const return type. Removed the fix for a virtual function.
-
-- Fixed incorrect suggestions for :doc:`readability-container-size-empty
-  ` when smart pointers are involved.
-
 - Fixed a false positive in :doc:`readability-non-const-parameter
   ` when the parameter is
   referenced by an lvalue.
 
-- Expanded :doc:`readability-simplify-boolean-expr
-  ` to simplify expressions
-  using DeMorgan's Theorem.
-
-- Fixed a crash in :doc:`performance-unnecessary-value-param
-  ` when the specialization
-  template has an unnecessary value parameter. Removed the fix for a template.
+- Fixed an issue when there was already an initializer in the constructor and
+  the check would try to create another initializer for the same member.
 
 - Fixed bugs in :doc:`bugprone-use-after-move
   `:
@@ -233,6 +210,33 @@
 
   - Don't emit an erroneous warning on self-moves.
 
+- Fixed false positiv

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi marked an inline comment as done.
ishaangandhi added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

Eugene.Zelenko wrote:
> ishaangandhi wrote:
> > Eugene.Zelenko wrote:
> > > Eugene.Zelenko wrote:
> > > > ishaangandhi wrote:
> > > > > ishaangandhi wrote:
> > > > > > Eugene.Zelenko wrote:
> > > > > > > Documentation path was changed recently. Please also keep 
> > > > > > > alphabetical order inside section.
> > > > > > Hm? Its still at this path on mastesr: 
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/ReleaseNotes.rst
> > > > > Also, it doesn't look alphabetized to me? Two points up is 
> > > > > "performance-unnecessary-..." then its "bugprone-use-after...".
> > > > Sorry, my mistake.
> > > It'll be good idea to fix order of other entries too.
> > Happy to! Would you like that in this diff or a new one?
> This is trivial change and could be made in same commit.
Done


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 439501.
ishaangandhi marked an inline comment as done.
ishaangandhi added a comment.

Change doc path


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
@@ -150,24 +150,29 @@
 Changes in existing checks
 ^^
 
-- Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
-  ` check for empty structs.
-
-- Fixed some false positives in :doc:`bugprone-infinite-loop
-  ` involving dependent expressions.
+- Expanded :doc:`readability-simplify-boolean-expr
+  ` to simplify expressions
+  using DeMorgan's Theorem.
 
 - Fixed a crash in :doc:`bugprone-sizeof-expression
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
-- Made :doc:`cert-oop57-cpp ` more sensitive
-  by checking for an arbitrary expression in the second argument of ``memset``.
+- Fixed a crash in :doc:`llvmlibc-callee-namespace
+  ` when executing for C++ code
+  that contain calls to advanced constructs, e.g. overloaded operators.
 
-- Improved :doc:`cppcoreguidelines-prefer-member-initializer
-  ` check.
+- Fixed a crash in :doc:`performance-unnecessary-value-param
+  ` when the specialization
+  template has an unnecessary value parameter. Removed the fix for a template.
 
-  Fixed an issue when there was already an initializer in the constructor and
-  the check would try to create another initializer for the same member.
+- Fixed a crash in :doc:`readability-const-return-type
+  ` when a pure virtual function
+  overrided has a const return type. Removed the fix for a virtual function.
+
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
   ` involving
@@ -175,18 +180,6 @@
   those cannot be used as base classes, consequently, they can not violate the
   rule.
 
-- Fixed a crash in :doc:`llvmlibc-callee-namespace
-  ` when executing for C++ code
-  that contain calls to advanced constructs, e.g. overloaded operators.
-
-- Fixed false positives in :doc:`misc-redundant-expression
-  `:
-
-  - Fixed a false positive involving overloaded comparison operators.
-
-  - Fixed a false positive involving assignments in
-conditions. This fixes `Issue 35853 `_.
-
 - Fixed a false positive in :doc:`misc-unused-parameters
   `
   where invalid parameters were implicitly being treated as being unused. 
@@ -201,28 +194,12 @@
   the ``CheckHeaderFile=true`` option if header files of the project only
   included by C++ source files.
 
-- Improved :doc:`performance-inefficient-vector-operation
-  ` to work when
-  the vector is a member of a structure.
-
-- Fixed a crash in :doc:`readability-const-return-type
-  ` when a pure virtual function
-  overrided has a const return type. Removed the fix for a virtual function.
-
-- Fixed incorrect suggestions for :doc:`readability-container-size-empty
-  ` when smart pointers are involved.
-
 - Fixed a false positive in :doc:`readability-non-const-parameter
   ` when the parameter is
   referenced by an lvalue.
 
-- Expanded :doc:`readability-simplify-boolean-expr
-  ` to simplify expressions
-  using DeMorgan's Theorem.
-
-- Fixed a crash in :doc:`performance-unnecessary-value-param
-  ` when the specialization
-  template has an unnecessary value parameter. Removed the fix for a template.
+- Fixed an issue when there was already an initializer in the constructor and
+  the check would try to create another initializer for the same member.
 
 - Fixed bugs in :doc:`bugprone-use-after-move
   `:
@@ -233,6 +210,33 @@
 
   - Don't emit an erroneous warning on

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

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:236
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches

Eugene.Zelenko wrote:
> ishaangandhi wrote:
> > Eugene.Zelenko wrote:
> > > ishaangandhi wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Eugene.Zelenko wrote:
> > > > > > ishaangandhi wrote:
> > > > > > > ishaangandhi wrote:
> > > > > > > > Eugene.Zelenko wrote:
> > > > > > > > > Documentation path was changed recently. Please also keep 
> > > > > > > > > alphabetical order inside section.
> > > > > > > > Hm? Its still at this path on mastesr: 
> > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/docs/ReleaseNotes.rst
> > > > > > > Also, it doesn't look alphabetized to me? Two points up is 
> > > > > > > "performance-unnecessary-..." then its "bugprone-use-after...".
> > > > > > Sorry, my mistake.
> > > > > It'll be good idea to fix order of other entries too.
> > > > Happy to! Would you like that in this diff or a new one?
> > > This is trivial change and could be made in same commit.
> > Done
> Sorry for being unclear, but I meant alphabetical order for check names, not 
> beginning of sentences.
This is then unclear to me. How do you alphabetize by check name?


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

https://reviews.llvm.org/D128402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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
   ` check for empty structs.
 
+- Fixed a false positive in :doc:`bugprone-branch-clone
+  ` when the branches
+  involve unknown expressions.
+
 - Fixed some false positives in :doc:`bugprone-infinite-loop
   ` involving dependent expressions.
 
@@ -160,6 +164,15 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Fixed bugs in :doc:`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 ` more sensitive
   by checking for an arbitrary expression in the second argument of ``memset``.
 
@@ -205,6 +218,10 @@
   ` to work when
   the vector is a member of a structure.
 
+- Fixed a crash in :doc:`performance-unnecessary-value-param
+  ` when the 
specialization
+  template has an unnecessary value parameter. Removed the fix for a template.
+
 - Fixed a crash in :doc:`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 @@
   ` to simplify 
expressions
   using DeMorgan's Theorem.
 
-- Fixed a crash in :doc:`performance-unnecessary-value-param
-  ` when the 
specialization
-  template has an unnecessary value parameter. Removed the fix for a template.
-
-- Fixed bugs in :doc:`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(LHS) && isa(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(LHS);
+const auto *RHSExpr = llvm::cast(RHS);
+if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
+  return false;
+}
+  }
+
   llvm::FoldingSetNodeID DataLHS, DataRHS;
   LHS->Profile(DataLHS, Context, false);
   RHS->Profile(DataRHS, Context, false);


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-erro