[Clang] make canonical AutoType constraints-free

[ychen]

Summary

As @mizvekov suggested in D134772. This works great for D128750 when
dealing with AutoType's.

[erichkeane]

This looks right enough to me with the 1 nit, but let Matheus take a look before committing.

clang/lib/AST/ASTContext.cpp

699

As a nit, I'd probably just call this TC or TypeConstr.

[ychen]

• address @erichkeane's comment.

[ychen]

In D135088#3831183, @erichkeane wrote:

This looks right enough to me with the 1 nit, but let Matheus take a look before committing.

Thanks for the quick review.

[mizvekov]

clang/lib/AST/ASTContext.cpp

5730–5732

Pre-existing problem: This was not good to begin with, since we would not capture the correct declaration used, but now you can just remove it, no further changes required.

5736–5737

FYI (not asking you to fix this on this patch) there is a pre-existing bug here where we are not profiling the node on the isPack flag.

5745–5746

Also adds back the isDependent flag. Any reason to have removed it?

clang/lib/Sema/SemaDeclCXX.cpp

8664–8669

I think this might crash if the three-way comparison is defaulted with a deduced template specialization.

Something like:

template <class> struct A {};

struct P {
  friend A operator<=>(const P&, const P&) = default;
};

The problem is that RT->getContainedDeducedType() will return non-null, because DTSTs are also deduced types, but RT->getContainedAutoType() will return null, because DTSTs are not AutoTypes.

[ychen]

clang/lib/AST/ASTContext.cpp

5736–5737

FYI (not asking you to fix this on this patch) there is a pre-existing bug here where we are not profiling the node on the isPack flag.

Thanks. Great to know.

5745–5746

I was thinking the replacement type is null, so there is no need to depend on instantiation?

clang/lib/Sema/SemaDeclCXX.cpp

8664–8669

I think this might crash if the three-way comparison is defaulted with a deduced template specialization.
The problem is that RT->getContainedDeducedType() will return non-null, because DTSTs are also deduced types, but RT->getContainedAutoType() will return null, because DTSTs are not AutoTypes.

RT->getContainedAutoType() will be called only when RT is AutoType?

[ychen]

• remove dead code

[mizvekov]

clang/lib/Sema/SemaDeclCXX.cpp

8664–8669

Oh right, I see it now, there is a lot going on in that condition :)

[ychen]

clang/lib/AST/ASTContext.cpp

5745–5746

I was thinking the replacement type is null, so there is no need to depend on instantiation?

5745–5746

Sorry I was not precise. I meant the constrained non-canonical AutoType would always be used for template instantiation. But I think it could happen that the user may query dependencies of the canonical AutoType of a constrained AutoType, maybe it just doesn't show up in the codebase yet. I'll put it back.

[ychen]

• add back IsDependent parameter.

[mizvekov]

clang/lib/AST/ASTContext.cpp

5745–5746

FYI my memory is fuzzy right now, but we do have the distinction between undeduced but non-dependent, and undeduced but dependent. It's suspicious that no tests fail though. It could be either it doesn't matter for the canonical type, or that the test would be a bit convoluted.

[ychen]

clang/lib/AST/ASTContext.cpp

5745–5746

Yeah, I'm not confident to remove it right now. Already put it back.

[mizvekov]

clang/lib/AST/ASTContext.cpp

5743–5744

Last minor nit, but I think this looks more readable if you invert the isNull check and do the TypeConstraintConcept on the else of that, less nesting.

6310–6313

isSameConstraintExpr already handles that.

[ychen]

• Address comments.

[ychen]

@mizvekov Thanks for the review!

clang/lib/AST/ASTContext.cpp

5743–5744

Yep. That's better.

6310–6313

Ack.

clang/lib/AST/ASTContext.cpp

if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(*P)) {
       } else
         ID.AddBoolean(false);
       continue;
     }
 
     if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(*P)) {
       ID.AddInteger(1);
       ID.AddBoolean(NTTP->isParameterPack());
+      const Expr *TC = NTTP->getPlaceholderTypeConstraint();
+      ID.AddBoolean(TC != nullptr);
       ID.AddPointer(NTTP->getType().getCanonicalType().getAsOpaquePtr());
+      if (TC)
+        TC->Profile(ID, C, /*Canonical=*/true);
       if (NTTP->isExpandedParameterPack()) {
         ID.AddBoolean(true);
         ID.AddInteger(NTTP->getNumExpansionTypes());
         for (unsigned I = 0, N = NTTP->getNumExpansionTypes(); I != N; ++I) {
           QualType T = NTTP->getExpansionType(I);
           ID.AddPointer(T.getCanonicalType().getAsOpaquePtr());
         }
       } else

 QualType ASTContext::getAutoTypeInternal(
     QualType DeducedType, AutoTypeKeyword Keyword, bool IsDependent,
     bool IsPack, ConceptDecl *TypeConstraintConcept,
     ArrayRef<TemplateArgument> TypeConstraintArgs, bool IsCanon) const {
   if (DeducedType.isNull() && Keyword == AutoTypeKeyword::Auto &&
       !TypeConstraintConcept && !IsDependent)
     return getAutoDeductType();
 
   if (TypeConstraintConcept)
     TypeConstraintConcept = TypeConstraintConcept->getCanonicalDecl();
 
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
   AutoType::Profile(ID, *this, DeducedType, Keyword, IsDependent,
                     TypeConstraintConcept, TypeConstraintArgs);
   if (AutoType *AT = AutoTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(AT, 0);
 
   QualType Canon;
   if (!IsCanon) {
     if (DeducedType.isNull()) {
-      SmallVector<TemplateArgument, 4> CanonArgs;
-      bool AnyNonCanonArgs =
-          ::getCanonicalTemplateArguments(*this, TypeConstraintArgs, CanonArgs);
-      if (AnyNonCanonArgs) {
-        Canon = getAutoTypeInternal(QualType(), Keyword, IsDependent, IsPack,
-                                    TypeConstraintConcept, CanonArgs, true);
+      if (TypeConstraintConcept) {
+        Canon = getAutoTypeInternal(QualType(), Keyword, false, IsPack, nullptr,
+                                    {}, true);
         // Find the insert position again.
         [[maybe_unused]] auto *Nothing =
             AutoTypes.FindNodeOrInsertPos(ID, InsertPos);
         assert(!Nothing && "canonical type broken");
       }
     } else {
       Canon = DeducedType.getCanonicalType();
     }
if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
       return false;
     return isSameTypeConstraint(TX->getTypeConstraint(),
                                 TY->getTypeConstraint());
   }
 
   if (auto *TX = dyn_cast<NonTypeTemplateParmDecl>(X)) {
     auto *TY = cast<NonTypeTemplateParmDecl>(Y);
     return TX->isParameterPack() == TY->isParameterPack() &&
-           TX->getASTContext().hasSameType(TX->getType(), TY->getType());
+           TX->getASTContext().hasSameType(TX->getType(), TY->getType()) &&
+           TX->hasPlaceholderTypeConstraint() ==
+               TY->hasPlaceholderTypeConstraint() &&
+           isSameConstraintExpr(TX->getPlaceholderTypeConstraint(),
+                                TY->getPlaceholderTypeConstraint());
   }
 
   auto *TX = cast<TemplateTemplateParmDecl>(X);
   auto *TY = cast<TemplateTemplateParmDecl>(Y);
   return TX->isParameterPack() == TY->isParameterPack() &&
          isSameTemplateParameterList(TX->getTemplateParameters(),
                                      TY->getTemplateParameters());
 }

clang/lib/AST/DeclTemplate.cpp

static void ProfileTemplateParameterList(ASTContext &C,
   if (RC)
     RC->Profile(ID, C, /*Canonical=*/true);
   ID.AddInteger(TPL->size());
   for (NamedDecl *D : *TPL) {
     if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(D)) {
       ID.AddInteger(0);
       ID.AddBoolean(NTTP->isParameterPack());
       NTTP->getType().getCanonicalType().Profile(ID);
+      ID.AddBoolean(NTTP->hasPlaceholderTypeConstraint());
+      if (const Expr *E = NTTP->getPlaceholderTypeConstraint())
+        E->Profile(ID, C, /*Canonical=*/true);
       continue;
     }
     if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(D)) {
       ID.AddInteger(1);
       ID.AddBoolean(TTP->isParameterPack());
       ID.AddBoolean(TTP->hasTypeConstraint());
       if (const TypeConstraint *TC = TTP->getTypeConstraint())
         TC->getImmediatelyDeclaredConstraint()->Profile(ID, C,

clang/lib/Sema/SemaDeclCXX.cpp

if (DCK != DefaultedComparisonKind::ThreeWay &&
     Diag(FD->getLocation(), diag::err_defaulted_comparison_return_type_not_bool)
         << (int)DCK << FD->getDeclaredReturnType() << Context.BoolTy
         << FD->getReturnTypeSourceRange();
     return true;
   }
   // C++2a [class.spaceship]p2 [P2002R0]:
   //   Let R be the declared return type [...]. If R is auto, [...]. Otherwise,
   //   R shall not contain a placeholder type.
-  if (DCK == DefaultedComparisonKind::ThreeWay &&
-      FD->getDeclaredReturnType()->getContainedDeducedType() &&
-      !Context.hasSameType(FD->getDeclaredReturnType(),
-                           Context.getAutoDeductType())) {
+  if (QualType RT = FD->getDeclaredReturnType();
+      DCK == DefaultedComparisonKind::ThreeWay &&
+      RT->getContainedDeducedType() &&
+      (!Context.hasSameType(RT, Context.getAutoDeductType()) ||
+       RT->getContainedAutoType()->isConstrained())) {
     Diag(FD->getLocation(),
          diag::err_defaulted_comparison_deduced_return_type_not_auto)
         << (int)DCK << FD->getDeclaredReturnType() << Context.AutoDeductTy
         << FD->getReturnTypeSourceRange();
     return true;
   }
 
   // For a defaulted function in a dependent class, defer all remaining checks
   // until instantiation.

clang/lib/Sema/SemaTemplate.cpp

else if (TemplateTemplateParmDecl *OldTTP
     if (!S.TemplateParameterListsAreEqual(
             NewInstFrom, NewTTP->getTemplateParameters(), OldInstFrom,
             OldTTP->getTemplateParameters(), Complain,
             (Kind == Sema::TPL_TemplateMatch
                  ? Sema::TPL_TemplateTemplateParmMatch
                  : Kind),
             TemplateArgLoc))
       return false;
-  } else if (Kind != Sema::TPL_TemplateTemplateArgumentMatch) {
+  }
+
+  if (Kind != Sema::TPL_TemplateTemplateArgumentMatch &&
+      !isa<TemplateTemplateParmDecl>(Old)) {
     const Expr *NewC = nullptr, *OldC = nullptr;
+
+    if (isa<TemplateTypeParmDecl>(New)) {
       if (const auto *TC = cast<TemplateTypeParmDecl>(New)->getTypeConstraint())
         NewC = TC->getImmediatelyDeclaredConstraint();
       if (const auto *TC = cast<TemplateTypeParmDecl>(Old)->getTypeConstraint())
         OldC = TC->getImmediatelyDeclaredConstraint();
+    } else if (isa<NonTypeTemplateParmDecl>(New)) {
+      if (const Expr *E = cast<NonTypeTemplateParmDecl>(New)
+                              ->getPlaceholderTypeConstraint())
+        NewC = E;
+      if (const Expr *E = cast<NonTypeTemplateParmDecl>(Old)
+                              ->getPlaceholderTypeConstraint())
+        OldC = E;
+    } else
+      llvm_unreachable("unexpected template parameter type");
 
     auto Diagnose = [&] {
       S.Diag(NewC ? NewC->getBeginLoc() : New->getBeginLoc(),
            diag::err_template_different_type_constraint);
       S.Diag(OldC ? OldC->getBeginLoc() : Old->getBeginLoc(),
            diag::note_template_prev_declaration) << /*declaration*/0;
     };
 

clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp

namespace PR48593 {
   d<,> auto e = 0; // expected-error{{expected expression}}
 }
 
 namespace PR48617 {
   template <typename...> concept C = true;
   template <typename...> class A {};
 
   template <typename... Ts> C<Ts...> auto e(A<Ts...>) { return 0; }
-
-  // FIXME: The error here does not make sense.
   template auto e<>(A<>);
-  // expected-error@-1 {{explicit instantiation of 'e' does not refer to a function template}}
-  // expected-note@-5  {{candidate template ignored: failed template argument deduction}}
-
-  // FIXME: Should be able to instantiate this with no errors.
-  template C<int> auto e<int>(A<int>);
-  // expected-error@-1 {{explicit instantiation of 'e' does not refer to a function template}}
-  // expected-note@-10 {{candidate template ignored: could not match 'C<int, Ts...> auto' against 'C<int> auto'}}
-  
-  template C<> auto e<>(A<>);
+  template auto e<int>(A<int>);
+  template <typename... Ts> C<Ts...> auto d(A<Ts...>) { return 0; }
+  template C<> auto d<>(A<>);
+  template C<int> auto d<int>(A<int>);
 
   template <typename... Ts> A<Ts...> c(Ts...);
   int f = e(c(1, 2));
 }