[PATCH] D149264: GH62362: Handle constraints on "Using" inherited constructors

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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