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

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950