[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69950#1771462 <https://reviews.llvm.org/D69950#1771462>, @eandrews wrote:

> In D69950#1771340 <https://reviews.llvm.org/D69950#1771340>, @mstorsjo wrote:
>
> > In D69950#1771133 <https://reviews.llvm.org/D69950#1771133>, @eandrews wrote:
> >
> > > In D69950#1770138 <https://reviews.llvm.org/D69950#1770138>, @mstorsjo wrote:
> > >
> > > > This (when reapplied in https://reviews.llvm.org/rG878a24ee244a24c39d1c57e9af2) broke compilation of code that earlier built fine. A reduced example:
> > > >
> > > >   namespace glslang {
> > > >   class TPoolAllocator {
> > > >     void operator=(TPoolAllocator);
> > > >   };
> > > >   template <class> class a {
> > > >     TPoolAllocator *b;
> > > >     void c() { allocator = *b; }
> > > >     TPoolAllocator allocator;
> > > >   };
> > > >   } // namespace glslang
> > > >
> > >
> > >
> > > With this patch, some errors in templates are diagnosed earlier (i.e. does not wait till instantiation). Since 'allocator' and 'b' aren't dependent, I think this is a valid diagnosis. GCC throws an error on this code upon instantiation. https://godbolt.org/z/X9Y-Vy
> >
> >
> > The original non-reduced case does build fine with GCC though (I'm fairly sure). I can try to reduce one later that still builds with GCC but fails with current clang.
>
>
> Yes. The reduced example here builds fine with GCC as well, unless template is instantiated. However I don't think Clang always matches GCC behavior when diagnosing errors in templates (See example from cpplearner here - https://reviews.llvm.org/D61027). Clang diagnoses several errors even if template is not instantiated (unlike GCC).  I am not sure what the 'correct' behavior is / whether we should match GCC here. @erichkeane @rnk could you please take a look?


I'll have to see the reproducer, however Clang DOES diagnose some template errors earlier than GCC in many situations.  A template definition that isn't instantiatible is ill-formed (I think NDR), so if the original reproducer doesn't instantiate the template, then it is likely incorrect code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950