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

[Nathan James via Phabricator via cfe-commits]

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

In D71980#1799437 <https://reviews.llvm.org/D71980#1799437>, @xazax.hun wrote:

> Thanks for working on this, please see one of my concerns inline.
>
> If you are trying to fix this problem, alternatively, you could also fix it in Clang.
>
> Richard outlined the best possible way to solve this class of errors by introducing a new type of AST node in: https://reviews.llvm.org/D46234


I'm going to look into adding a DiscardedStmt, see how that goes. would probably be best just creating a new Stmt rather than trying to inherit from NullStmt.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:62
   Finder->addMatcher(
-      ifStmt(stmt().bind("if"),
+      ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())),
+             stmt().bind("if"),
----------------
xazax.hun wrote:
> Why do we care if we are inside a template instantiation? 
> 
> Couldn't we trigger the bug with something like:
> ```
> void shouldPass() {
>   if constexpr (constexprFun(1) == 0) {
>     handle(0);
>   } else if constexpr (constexprFun(1)  == 1) {
>     handle(1);
>   } else {
>     handle(2);
>   }
> }
> ```
> 
> ?
We are disabling the bug check when we are in a template instantiation. Reason being the template instantiation folds away the false branches to null statements which is the cause of the false positives. Also if there is a bug in a templated if constexpr, it will be caught when the template is defined. 


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

https://reviews.llvm.org/D71980