rsmith added a comment.

I think that the bugs that this was aiming to fix were fixed by 
rG1e43349e321694d7fee3d77cb691887ad67fb5d7 
<https://reviews.llvm.org/rG1e43349e321694d7fee3d77cb691887ad67fb5d7>, which 
means that we should not ask whether the constraints of an inherited 
constructor are satisfied any more. Instead, overload resolution will look at 
whether the constraints of the original constructors are satisfied. (More 
broadly, I wonder whether `DiagnoseUseOfDecl` should be considering constraints 
at all -- the model in the standard is that any time you name a function, you 
go through overload resolution, which considers constraints, so 
`DiagnoseUseOfDecl` should only ever be duplicating work that's already been 
done by overload resolution, but it's possible we're not entirely following 
that model. Still, we should be able to avoid redoing the constraint 
satisfaction checks in the common case where overload resolution already did 
them.)

That said, I think this change would still be correct if for whatever reason we 
started reaching this codepath for inheriting constructors again.



================
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)
----------------
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.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:217-219
+  if (Ctor->isImplicit() && Ctor->isInheritingConstructor() &&
+      Ctor->getInheritedConstructor().getConstructor())
+    Ctor = Ctor->getInheritedConstructor().getConstructor();
----------------
These checks can be simplified.


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