mizvekov added inline comments.

================
Comment at: clang/lib/AST/ASTImporter.cpp:3285
   }
-  if (const TypedefType *TypedefT =
-          dyn_cast<TypedefType>(FromFPT->getReturnType())) {
-    TypedefNameDecl *TD = TypedefT->getDecl();
+  if (const auto *TypedefT = dyn_cast<TypedefType>(FromFPT->getReturnType())) {
+    const TypedefNameDecl *TD = TypedefT->getDecl();
----------------
rsmith wrote:
> (Preparing for D112374)
Good catch, thanks :)


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1385-1388
+    P = P.getUnqualifiedType();
     //     - If A is a cv-qualified type, A is replaced by the cv-unqualified
     //       version of A.
+    A = A.getUnqualifiedType();
----------------
rsmith wrote:
> Do we need to use `Context.getUnqualifiedArrayType` here, for cases like:
> ```
> typedef const int CInt;
> // Partial ordering should deduce `T = int` from both parameters here,
> // even though `T = const int` might make more sense for the first one.
> template<int N> void f(CInt (&)[N], int*);
> template<int N, typename T> void f(T (&)[N], T*);
> CInt arr[5];
> void g() { f(arr, arr); }
> ```
> ? I think `getUnqualifiedType()` on `CInt[N]` will not remove the indirect 
> `const` qualifier.
I see, that is true, if P and A could be sugared here, we would need 
`getUnqualifiedArrayType`.

But this issue had been sidestepped because we were not handling sugar in the 
PartialOrdering case, because that was only used in the isAtLeastAsSpecialized 
path where we were still using the canonical type, as we don't care about what 
types where actually deduced there.

I just changed it to not use the canonical type anymore and took this change.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1546-1568
+  QualType CanP = P.getCanonicalType();
+  // If the parameter type is not dependent, there is nothing to deduce.
+  if (!P->isDependentType()) {
+    if (TDF & TDF_SkipNonDependent)
       return Sema::TDK_Success;
+
+    QualType CanA = A.getCanonicalType();
----------------
rsmith wrote:
> I think we can excise `CanA` and `CanP` entirely here so people aren't 
> tempted to use them and lose sugar.
Nice suggestion!

Though we must keep the last return as conditional, as I did there, otherwise we
break a test case such as the one from PR12132:
```
template<typename S> void fun(const int* const S::* member) {}
struct A { int* x; };
void foo() {
    fun(&A::x);
}
```
When ignoring qualifiers, hasSameUnqualifiedType being false is not enough to 
deduce a mismatch, so this is the one corner case where we analyze with a 
non-dependent parameter type.

That is what I tried to explain in that comment, but I have just reworded it to 
be clearer on this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

Reply via email to