[clang] [clang-tools-extra] [clang] check deduction consistency when partial ordering function templates (PR #100692)

[Matheus Izvekov via cfe-commits]

================
@@ -5399,11 +5434,85 @@ static QualType GetImplicitObjectParameterType(ASTContext &Context,
   return Context.getLValueReferenceType(RawType);
 }
 
+static TemplateDeductionResult FinishTemplateArgumentDeduction(
+    Sema &S, FunctionTemplateDecl *FTD, int ArgIdx, QualType P, QualType A,
+    SmallVectorImpl<DeducedTemplateArgument> &Deduced,
+    TemplateDeductionInfo &Info) {
+  // Unevaluated SFINAE context.
+  EnterExpressionEvaluationContext Unevaluated(
+      S, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::SFINAETrap Trap(S);
+
+  Sema::ContextRAII SavedContext(S, getAsDeclContextOrEnclosing(FTD));
+
+  // C++ [temp.deduct.type]p2:
+  //   [...] or if any template argument remains neither deduced nor
+  //   explicitly specified, template argument deduction fails.
+  bool IsIncomplete = false;
+  SmallVector<TemplateArgument, 4> SugaredBuilder, CanonicalBuilder;
+  if (auto Result = ConvertDeducedTemplateArguments(
+          S, FTD, /*IsDeduced=*/true, Deduced, Info, SugaredBuilder,
+          CanonicalBuilder, /*CurrentInstantiationScope=*/nullptr,
+          /*NumAlreadyConverted=*/0, &IsIncomplete);
+      Result != TemplateDeductionResult::Success)
+    return Result;
+
+  // Form the template argument list from the deduced template arguments.
+  TemplateArgumentList *SugaredDeducedArgumentList =
+      TemplateArgumentList::CreateCopy(S.Context, SugaredBuilder);
+  TemplateArgumentList *CanonicalDeducedArgumentList =
+      TemplateArgumentList::CreateCopy(S.Context, CanonicalBuilder);
+
+  Info.reset(SugaredDeducedArgumentList, CanonicalDeducedArgumentList);
+
+  // Substitute the deduced template arguments into the argument
+  // and verify that the instantiated argument is both valid
+  // and equivalent to the parameter.
+  LocalInstantiationScope InstScope(S);
+
+  QualType InstP;
+  {
+    MultiLevelTemplateArgumentList MLTAL(FTD, SugaredBuilder,
+                                         /*Final=*/true);
+    if (ArgIdx != -1)
+      if (auto *MD = dyn_cast<CXXMethodDecl>(FTD->getTemplatedDecl());
+          MD && MD->isImplicitObjectMemberFunction())
+        ArgIdx -= 1;
+    Sema::ArgumentPackSubstitutionIndexRAII PackIndex(
+        S, ArgIdx != -1 ? ::getPackIndexForParam(S, FTD, MLTAL, ArgIdx) : -1);
+    InstP = S.SubstType(P, MLTAL, FTD->getLocation(), FTD->getDeclName());
+    if (InstP.isNull())
+      return TemplateDeductionResult::SubstitutionFailure;
+  }
+
+  if (auto *PA = dyn_cast<PackExpansionType>(A);
+      PA && !isa<PackExpansionType>(InstP))
+    A = PA->getPattern();
+  if (!S.Context.hasSameType(
+          S.Context.getUnqualifiedArrayType(InstP.getNonReferenceType()),
+          S.Context.getUnqualifiedArrayType(A.getNonReferenceType())))
+    return TemplateDeductionResult::NonDeducedMismatch;
+
+  // C++20 [temp.deduct]p5 - Only check constraints when all parameters have
+  // been deduced.
+  if (!IsIncomplete) {
+    if (auto Result = CheckDeducedArgumentConstraints(S, FTD, SugaredBuilder,
+                                                      CanonicalBuilder, Info);
+        Result != TemplateDeductionResult::Success)
+      return Result;
+  }
----------------
mizvekov wrote:

We seem to consistently follow the order I implemented here.

One reason I can think of, is that it would probably be too confusing to produce constraints check failure diagnostics before checking one thing even implies the other.

But doing it before would certainly simplify the implementation when we change the implementation to check only once.

https://github.com/llvm/llvm-project/pull/100692