[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 added inline comments.


================
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:
> njames93 wrote:
> > 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. 
> Yeah, be people are free to write `if constexpr` outside of templates right? So instantiations are not the only places where branches can be folded away.
The issue is that only template instantiations cause the false branch to get folded away, observe [[ https://godbolt.org/z/u48xzy | here ]] to see how its handled in templated code that isn't yet instantiated, instantiated template code and normal untemplated code


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

https://reviews.llvm.org/D71980