[llvm-branch-commits] [clang-tools-extra] 1f98c2b - [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

[Hans Wennborg via llvm-branch-commits]

Author: Nathan
Date: 2020-01-23T00:57:56+01:00
New Revision: 1f98c2b586e4ebce68d75856699059663a052cb7

URL: https://github.com/llvm/llvm-project/commit/1f98c2b586e4ebce68d75856699059663a052cb7
DIFF: https://github.com/llvm/llvm-project/commit/1f98c2b586e4ebce68d75856699059663a052cb7.diff

LOG: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

Summary: fixes [[ https://bugs.llvm.org/show_bug.cgi?id=32203 | readability-braces-around-statements broken for if constexpr]] and [[ https://bugs.llvm.org/show_bug.cgi?id=44229 | bugprone-branch-clone false positive with template functions and constexpr ]] by disabling the relevant checks on if constexpr statements while inside an instantiated template. This is due to how the else branch of an if constexpr statement is folded away to a null statement if the condition evaluates to false

Reviewers: alexfh, hokein, aaron.ballman, xazax.hun

Reviewed By: aaron.ballman, xazax.hun

Subscribers: rnkovacs, JonasToth, Jim, lebedev.ri, xazax.hun, cfe-commits

Tags: #clang-tools-extra, #clang

Differential Revision: https://reviews.llvm.org/D71980

(cherry picked from commit f9c46229e4ac29053747c96e08c574c6c48d544b)

Added: 
    clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
    clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp

Modified: 
    clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
    clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
index eb54aaa99445..e40b27585d3d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -59,7 +59,8 @@ namespace bugprone {
 
 void BranchCloneCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      ifStmt(stmt().bind("if"),
+      ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+             stmt().bind("if"),
              hasParent(stmt(unless(ifStmt(hasElse(equalsBoundNode("if")))))),
              hasElse(stmt().bind("else"))),
       this);

diff  --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
index 117ef36d78fe..da0bef32c091 100644
--- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -123,7 +123,10 @@ void BracesAroundStatementsCheck::storeOptions(
 }
 
 void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(
+      ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())))
+          .bind("if"),
+      this);
   Finder->addMatcher(whileStmt().bind("while"), this);
   Finder->addMatcher(doStmt().bind("do"), this);
   Finder->addMatcher(forStmt().bind("for"), this);

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
new file mode 100644
index 000000000000..382330139c8c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-branch-clone-if-constexpr-template.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- -- -std=c++17
+
+void handle(int);
+
+template <unsigned Index>
+void shouldFail() {
+  if constexpr (Index == 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: repeated branch in conditional chain [bugprone-branch-clone]
+    handle(0);
+  } else if constexpr (Index == 1) {
+    handle(1);
+  } else {
+    handle(0);
+  }
+}
+
+template <unsigned Index>
+void shouldPass() {
+  if constexpr (Index == 0) {
+    handle(0);
+  } else if constexpr (Index == 1) {
+    handle(1);
+  } else {
+    handle(2);
+  }
+}
+
+void shouldFailNonTemplate() {
+  constexpr unsigned Index = 1;
+  if constexpr (Index == 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: repeated branch in conditional chain [bugprone-branch-clone]
+    handle(0);
+  } else if constexpr (Index == 1) {
+    handle(1);
+  } else {
+    handle(0);
+  }
+}
+
+void shouldPassNonTemplate() {
+  constexpr unsigned Index = 1;
+  if constexpr (Index == 0) {
+    handle(0);
+  } else if constexpr (Index == 1) {
+    handle(1);
+  } else {
+    handle(2);
+  }
+}
+
+void run() {
+    shouldFail<0>();
+    shouldFail<1>();
+    shouldFail<2>();
+    shouldPass<0>();
+    shouldPass<1>();
+    shouldPass<2>();
+}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp
new file mode 100644
index 000000000000..988125f9ce2d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements-constexpr-if-templates.cpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s readability-braces-around-statements %t -- -- -std=c++17
+
+void handle(bool);
+
+template <bool branch>
+void shouldFail() {
+  if constexpr (branch)
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: statement should be inside braces [readability-braces-around-statements]
+    handle(true);
+  else
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: statement should be inside braces [readability-braces-around-statements]
+    handle(false);
+}
+
+template <bool branch>
+void shouldPass() {
+  if constexpr (branch) {
+    handle(true);
+  } else {
+    handle(false);
+  }
+}
+
+void shouldFailNonTemplate() {
+  constexpr bool branch = false;
+  if constexpr (branch)
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: statement should be inside braces [readability-braces-around-statements]
+    handle(true);
+  else
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: statement should be inside braces [readability-braces-around-statements]
+    handle(false);
+}
+
+void shouldPass() {
+  constexpr bool branch = false;
+  if constexpr (branch) {
+    handle(true);
+  } else {
+    handle(false);
+  }
+}
+
+void run() {
+    shouldFail<true>();
+    shouldFail<false>();
+    shouldPass<true>();
+    shouldPass<false>();
+}