xazax.hun 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"),
----------------
njames93 wrote:
> 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
Oh, I see. This is really weird. I do not really see why should the `if 
constexpr` behave differently outside of instantiations. Thanks for the 
clarification!


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

https://reviews.llvm.org/D71980



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

Reply via email to