Author: Nathan Date: 2020-01-17T14:21:38Z New Revision: f9c46229e4ac29053747c96e08c574c6c48d544b
URL: https://github.com/llvm/llvm-project/commit/f9c46229e4ac29053747c96e08c574c6c48d544b DIFF: https://github.com/llvm/llvm-project/commit/f9c46229e4ac29053747c96e08c574c6c48d544b.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 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>(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
