================
@@ -5525,6 +5525,8 @@ static TemplateDeductionResult CheckDeductionConsistency(
   // FIXME: A substitution can be incomplete on a non-structural part of the
   // type. Use the canonical type for now, until the TemplateInstantiator can
   // deal with that.
+  if (auto *Injected = P->getAs<InjectedClassNameType>())
+    P = Injected->getInjectedSpecializationType();
----------------
zyn0217 wrote:

I agree with you that fixing the deduction guide generation looks more optimal 
than current way, but that seemed to going to mess up more things, see the 
comment in `transformFunctionProtoType`:

```cpp
    // We use the injected-class-name type of the primary template instead.
    // This has the convenient property that it is different from any type that
    // the user can write in a deduction-guide (because they cannot enter the
    // context of the template), so implicit deduction guides can never collide
    // with explicit ones.
```

So staying with injected types looks like the only option to me. (For now, we 
just want to restore the 19's behavior)

The bug happens when the generated CTAD guide (which is generated from a copy 
constructor) comes along with another deduction guide that is generated from a 
user-defined constructor; since the first guide uses the injected class types, 
the default substitution would fail, hence the failure of type comparision in 
consistency checking, resulting in wrong partial ordering.

Besides CTAD guides, we don't appear to suffer from the issue because we don't 
model user-written function parameters with InjectedClassNameType - which is 
also what the above comment reflects.

So to me, they can't be nested - we promised it when forming the the deduction 
guide.

I tried to fix it at the substitution level, but unfortunately it didn't work 
very well. Some regressions surfaced, and all of them are due to the 
inconsistencies between the types in A, DeducedArgs. That's why I tweaked types 
at here in the end.

I'm happy to add some comments, as long as the approach makes sense to you.

https://github.com/llvm/llvm-project/pull/149782
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to