erichkeane added a comment.

Confirmed this is all fixed by your patch.  Thanks!



================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:138-144
+                        bool SkipForSpecialization,
+                        bool ForConstraintInstantiation) {
   if (!ClassTemplSpec->isClassScopeExplicitSpecialization()) {
     // We're done when we hit an explicit specialization.
     if (ClassTemplSpec->getSpecializationKind() == TSK_ExplicitSpecialization 
&&
-        !isa<ClassTemplatePartialSpecializationDecl>(ClassTemplSpec))
+        !isa<ClassTemplatePartialSpecializationDecl>(ClassTemplSpec) &&
+        !ForConstraintInstantiation)
----------------
rsmith wrote:
> rsmith wrote:
> > It seems to me that a namespace-scope explicit specialization shouldn't 
> > pick up template arguments, even during constraint checking:
> > 
> > ```
> > template<typename T> struct A {};
> > template<> struct A<int> {
> >   template<typename U> void f() requires U::value;
> > };
> > ```
> > Constraint checking for `A<X>::f<Y>` should produce a single level of 
> > template arguments, `<Y>`, not two layers `<X>, <Y>`, because `U` in 
> > `U::value` is a depth-0 index-0 parameter.
> Complete testcase:
> ```
> struct X {}; struct Y { static constexpr bool value = true; };
> template<typename T> struct A {};
> template<> struct A<X> {                          
>   template<typename U> void f() requires U::value;
> };                                                                   
> void g(A<X> a) { a.f<Y>(); }
> ```
Ah! Thanks for the reproducer.  I was having a really hard time determining a 
case where this would break, everything I came up with was blocked by 
https://reviews.llvm.org/D146178

The difficulty was that the parent of the inherited constructor was an inline 
definition of the explicit specialization, so it DID have depth, but it wasn't 
clear how to differentiate the two. I think your patch as listed fixes all the 
issues I was trying to fix as you said, so I'll abandon this, though I'll 
include the tests as an NFC/RAC patch for completeness.



================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:217-219
+  if (Ctor->isImplicit() && Ctor->isInheritingConstructor() &&
+      Ctor->getInheritedConstructor().getConstructor())
+    Ctor = Ctor->getInheritedConstructor().getConstructor();
----------------
rsmith wrote:
> These checks can be simplified.
Ah, thanks!  I must have been looking at the wrong thing, I wasn't convinced 
that the `getInheritedConstructor` call was always legal, but looking again, I 
see that should have been obvious.  


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

https://reviews.llvm.org/D149264

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

Reply via email to