Skip to content

[clang] CWG2398: improve overload resolution backwards compat #107350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

mizvekov
Copy link
Contributor

@mizvekov mizvekov commented Sep 5, 2024

With this change, we discriminate if the primary template and which partial specializations would have participated in overload resolution prior to P0522 changes.

We collect those in an initial set. If this set is not empty, or the primary template would have matched, we proceed with this set as the candidates for overload resolution.

Otherwise, we build a new overload set with everything else, and proceed as usual.

@mizvekov mizvekov requested a review from zygoloid September 5, 2024 04:24
@mizvekov mizvekov self-assigned this Sep 5, 2024
@mizvekov mizvekov force-pushed the users/mizvekov/clang-p0522-complete-implementation branch from 958703d to b29e3d3 Compare September 7, 2024 18:52
Base automatically changed from users/mizvekov/clang-p0522-complete-implementation to main October 1, 2024 23:50
@mizvekov mizvekov force-pushed the users/mizvekov/clang-cwg2398-improve-overload-resolution branch from 7de7d24 to 9b12c0b Compare October 5, 2024 21:51
@mizvekov mizvekov marked this pull request as ready for review October 5, 2024 22:25
@mizvekov mizvekov requested a review from Endilll as a code owner October 5, 2024 22:25
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 5, 2024
@mizvekov mizvekov requested a review from cor3ntin October 5, 2024 22:26
@llvmbot
Copy link
Member

llvmbot commented Oct 5, 2024

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

With this change, we discriminate if the primary template and which partial specializations would have participated in overload resolution prior to P0522 changes.

We collect those in an initial set. If this set is not empty, or the primary template would have matched, we proceed with this set as the candidates for overload resolution.

Otherwise, we build a new overload set with everything else, and proceed as usual.


Patch is 21.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/107350.diff

8 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang/include/clang/Sema/Sema.h (+9-5)
  • (modified) clang/include/clang/Sema/TemplateDeduction.h (+13)
  • (modified) clang/lib/Sema/SemaLookup.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+26-18)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+30-13)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+14-10)
  • (modified) clang/test/SemaTemplate/cwg2398.cpp (+1-5)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 44d5f348ed2d54..48784f2adcf38b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -204,7 +204,8 @@ Resolutions to C++ Defect Reports
   (`CWG2351: void{} <https://cplusplus.github.io/CWG/issues/2351.html>`_).
 
 - Clang now has improved resolution to CWG2398, allowing class templates to have
-  default arguments deduced when partial ordering.
+  default arguments deduced when partial ordering, and better backwards compatibility
+  in overload resolution.
 
 - Clang now allows comparing unequal object pointers that have been cast to ``void *``
   in constant expressions. These comparisons always worked in non-constant expressions.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bede971ce0191b..5d38862ce59f0c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11636,7 +11636,8 @@ class Sema final : public SemaBase {
                         SourceLocation RAngleLoc, unsigned ArgumentPackIndex,
                         SmallVectorImpl<TemplateArgument> &SugaredConverted,
                         SmallVectorImpl<TemplateArgument> &CanonicalConverted,
-                        CheckTemplateArgumentKind CTAK);
+                        CheckTemplateArgumentKind CTAK,
+                        bool *MatchedPackOnParmToNonPackOnArg);
 
   /// Check that the given template arguments can be provided to
   /// the given template, converting the arguments along the way.
@@ -11683,7 +11684,8 @@ class Sema final : public SemaBase {
       SmallVectorImpl<TemplateArgument> &SugaredConverted,
       SmallVectorImpl<TemplateArgument> &CanonicalConverted,
       bool UpdateArgsWithConversions = true,
-      bool *ConstraintsNotSatisfied = nullptr, bool PartialOrderingTTP = false);
+      bool *ConstraintsNotSatisfied = nullptr, bool PartialOrderingTTP = false,
+      bool *MatchedPackOnParmToNonPackOnArg = nullptr);
 
   bool CheckTemplateTypeArgument(
       TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
@@ -11717,7 +11719,8 @@ class Sema final : public SemaBase {
   /// It returns true if an error occurred, and false otherwise.
   bool CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
                                      TemplateParameterList *Params,
-                                     TemplateArgumentLoc &Arg, bool IsDeduced);
+                                     TemplateArgumentLoc &Arg, bool IsDeduced,
+                                     bool *MatchedPackOnParmToNonPackOnArg);
 
   void NoteTemplateLocation(const NamedDecl &Decl,
                             std::optional<SourceRange> ParamRange = {});
@@ -12418,7 +12421,7 @@ class Sema final : public SemaBase {
   bool isTemplateTemplateParameterAtLeastAsSpecializedAs(
       TemplateParameterList *PParam, TemplateDecl *PArg, TemplateDecl *AArg,
       const DefaultArguments &DefaultArgs, SourceLocation ArgLoc,
-      bool IsDeduced);
+      bool IsDeduced, bool *MatchedPackOnParmToNonPackOnArg);
 
   /// Mark which template parameters are used in a given expression.
   ///
@@ -13417,7 +13420,8 @@ class Sema final : public SemaBase {
   bool InstantiateClassTemplateSpecialization(
       SourceLocation PointOfInstantiation,
       ClassTemplateSpecializationDecl *ClassTemplateSpec,
-      TemplateSpecializationKind TSK, bool Complain = true);
+      TemplateSpecializationKind TSK, bool Complain = true,
+      bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false);
 
   /// Instantiates the definitions of all of the member
   /// of the given class, which is an instantiation of a class template
diff --git a/clang/include/clang/Sema/TemplateDeduction.h b/clang/include/clang/Sema/TemplateDeduction.h
index 28b014fd84e4b3..9c12eef5c42a06 100644
--- a/clang/include/clang/Sema/TemplateDeduction.h
+++ b/clang/include/clang/Sema/TemplateDeduction.h
@@ -51,6 +51,11 @@ class TemplateDeductionInfo {
   /// Have we suppressed an error during deduction?
   bool HasSFINAEDiagnostic = false;
 
+  /// Have we matched any packs on the parameter side, versus any non-packs on
+  /// the argument side, in a context where the opposite matching is also
+  /// allowed?
+  bool MatchedPackOnParmToNonPackOnArg = false;
+
   /// The template parameter depth for which we're performing deduction.
   unsigned DeducedDepth;
 
@@ -87,6 +92,14 @@ class TemplateDeductionInfo {
     return DeducedDepth;
   }
 
+  bool hasMatchedPackOnParmToNonPackOnArg() const {
+    return MatchedPackOnParmToNonPackOnArg;
+  }
+
+  void setMatchedPackOnParmToNonPackOnArg() {
+    MatchedPackOnParmToNonPackOnArg = true;
+  }
+
   /// Get the number of explicitly-specified arguments.
   unsigned getNumExplicitArgs() const {
     return ExplicitArgs;
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index f3f62474d06441..31422c213ac249 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3666,7 +3666,8 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
           TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
           if (CheckTemplateArgument(
                   Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
-                  0, SugaredChecked, CanonicalChecked, CTAK_Specified) ||
+                  0, SugaredChecked, CanonicalChecked, CTAK_Specified,
+                  /*MatchedPackOnParmToNonPackOnArg=*/nullptr) ||
               Trap.hasErrorOccurred())
             IsTemplate = false;
         }
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index eeaa1ebd7ba83a..82fd4789956622 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5179,7 +5179,7 @@ bool Sema::CheckTemplateArgument(
     unsigned ArgumentPackIndex,
     SmallVectorImpl<TemplateArgument> &SugaredConverted,
     SmallVectorImpl<TemplateArgument> &CanonicalConverted,
-    CheckTemplateArgumentKind CTAK) {
+    CheckTemplateArgumentKind CTAK, bool *MatchedPackOnParmToNonPackOnArg) {
   // Check template type parameters.
   if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param))
     return CheckTemplateTypeArgument(TTP, Arg, SugaredConverted,
@@ -5395,7 +5395,8 @@ bool Sema::CheckTemplateArgument(
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion:
     if (CheckTemplateTemplateArgument(TempParm, Params, Arg,
-                                      /*IsDeduced=*/CTAK != CTAK_Specified))
+                                      /*IsDeduced=*/CTAK != CTAK_Specified,
+                                      MatchedPackOnParmToNonPackOnArg))
       return true;
 
     SugaredConverted.push_back(Arg.getArgument());
@@ -5469,7 +5470,7 @@ bool Sema::CheckTemplateArgumentList(
     SmallVectorImpl<TemplateArgument> &SugaredConverted,
     SmallVectorImpl<TemplateArgument> &CanonicalConverted,
     bool UpdateArgsWithConversions, bool *ConstraintsNotSatisfied,
-    bool PartialOrderingTTP) {
+    bool PartialOrderingTTP, bool *MatchedPackOnParmToNonPackOnArg) {
 
   if (ConstraintsNotSatisfied)
     *ConstraintsNotSatisfied = false;
@@ -5545,10 +5546,10 @@ bool Sema::CheckTemplateArgumentList(
 
     if (ArgIdx < NumArgs) {
       // Check the template argument we were given.
-      if (CheckTemplateArgument(*Param, NewArgs[ArgIdx], Template, TemplateLoc,
-                                RAngleLoc, SugaredArgumentPack.size(),
-                                SugaredConverted, CanonicalConverted,
-                                CTAK_Specified))
+      if (CheckTemplateArgument(
+              *Param, NewArgs[ArgIdx], Template, TemplateLoc, RAngleLoc,
+              SugaredArgumentPack.size(), SugaredConverted, CanonicalConverted,
+              CTAK_Specified, MatchedPackOnParmToNonPackOnArg))
         return true;
 
       CanonicalConverted.back().setIsDefaulted(
@@ -5714,7 +5715,8 @@ bool Sema::CheckTemplateArgumentList(
     // Check the default template argument.
     if (CheckTemplateArgument(*Param, Arg, Template, TemplateLoc, RAngleLoc, 0,
                               SugaredConverted, CanonicalConverted,
-                              CTAK_Specified))
+                              CTAK_Specified,
+                              /*MatchedPackOnParmToNonPackOnArg=*/nullptr))
       return true;
 
     SugaredConverted.back().setIsDefaulted(true);
@@ -7292,10 +7294,10 @@ static void DiagnoseTemplateParameterListArityMismatch(
     Sema &S, TemplateParameterList *New, TemplateParameterList *Old,
     Sema::TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc);
 
-bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
-                                         TemplateParameterList *Params,
-                                         TemplateArgumentLoc &Arg,
-                                         bool IsDeduced) {
+bool Sema::CheckTemplateTemplateArgument(
+    TemplateTemplateParmDecl *Param, TemplateParameterList *Params,
+    TemplateArgumentLoc &Arg, bool IsDeduced,
+    bool *MatchedPackOnParmToNonPackOnArg) {
   TemplateName Name = Arg.getArgument().getAsTemplateOrTemplatePattern();
   auto [Template, DefaultArgs] = Name.getTemplateDeclAndDefaultArgs();
   if (!Template) {
@@ -7339,7 +7341,8 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
   //   A template-argument matches a template template-parameter P when P
   //   is at least as specialized as the template-argument A.
   if (!isTemplateTemplateParameterAtLeastAsSpecializedAs(
-          Params, Param, Template, DefaultArgs, Arg.getLocation(), IsDeduced))
+          Params, Param, Template, DefaultArgs, Arg.getLocation(), IsDeduced,
+          MatchedPackOnParmToNonPackOnArg))
     return true;
   // P2113
   // C++20[temp.func.order]p2
@@ -9757,11 +9760,14 @@ DeclResult Sema::ActOnExplicitInstantiation(
 
   // Check that the template argument list is well-formed for this
   // template.
+  bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false;
   SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
-  if (CheckTemplateArgumentList(ClassTemplate, TemplateNameLoc, TemplateArgs,
-                                /*DefaultArgs=*/{}, false, SugaredConverted,
-                                CanonicalConverted,
-                                /*UpdateArgsWithConversions=*/true))
+  if (CheckTemplateArgumentList(
+          ClassTemplate, TemplateNameLoc, TemplateArgs,
+          /*DefaultArgs=*/{}, false, SugaredConverted, CanonicalConverted,
+          /*UpdateArgsWithConversions=*/true,
+          /*ConstraintsNotSatisfied=*/nullptr, /*PartialOrderingTTP=*/false,
+          &PrimaryHasMatchedPackOnParmToNonPackOnArg))
     return true;
 
   // Find the class template specialization declaration that
@@ -9882,7 +9888,9 @@ DeclResult Sema::ActOnExplicitInstantiation(
     = cast_or_null<ClassTemplateSpecializationDecl>(
                                               Specialization->getDefinition());
   if (!Def)
-    InstantiateClassTemplateSpecialization(TemplateNameLoc, Specialization, TSK);
+    InstantiateClassTemplateSpecialization(
+        TemplateNameLoc, Specialization, TSK,
+        /*Complain=*/true, PrimaryHasMatchedPackOnParmToNonPackOnArg);
   else if (TSK == TSK_ExplicitInstantiationDefinition) {
     MarkVTableUsed(TemplateNameLoc, Specialization, true);
     Specialization->setPointOfInstantiation(Def->getPointOfInstantiation());
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index ce3317db5a8202..925d3cbf9310f7 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2767,8 +2767,12 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
     for (; hasTemplateArgumentForDeduction(As, ArgIdx) &&
            PackScope.hasNextElement();
          ++ArgIdx) {
-      if (!FoldPackParameter && !As[ArgIdx].isPackExpansion())
-        return TemplateDeductionResult::MiscellaneousDeductionFailure;
+      if (!As[ArgIdx].isPackExpansion()) {
+        if (!FoldPackParameter)
+          return TemplateDeductionResult::MiscellaneousDeductionFailure;
+        if (FoldPackArgument)
+          Info.setMatchedPackOnParmToNonPackOnArg();
+      }
       // Deduce template arguments from the pattern.
       if (auto Result = DeduceTemplateArguments(
               S, TemplateParams, Pattern, As[ArgIdx], Info, PartialOrdering,
@@ -2962,15 +2966,20 @@ static bool ConvertDeducedTemplateArgument(
     TemplateArgumentLoc ArgLoc = S.getTrivialTemplateArgumentLoc(
         Arg, QualType(), Info.getLocation(), Param);
 
+    bool MatchedPackOnParmToNonPackOnArg = false;
     // Check the template argument, converting it as necessary.
-    return S.CheckTemplateArgument(
+    auto Res = S.CheckTemplateArgument(
         Param, ArgLoc, Template, Template->getLocation(),
         Template->getSourceRange().getEnd(), ArgumentPackIndex, SugaredOutput,
         CanonicalOutput,
         IsDeduced
             ? (Arg.wasDeducedFromArrayBound() ? Sema::CTAK_DeducedFromArrayBound
                                               : Sema::CTAK_Deduced)
-            : Sema::CTAK_Specified);
+            : Sema::CTAK_Specified,
+        &MatchedPackOnParmToNonPackOnArg);
+    if (MatchedPackOnParmToNonPackOnArg)
+      Info.setMatchedPackOnParmToNonPackOnArg();
+    return Res;
   };
 
   if (Arg.getKind() == TemplateArgument::Pack) {
@@ -3165,7 +3174,8 @@ static TemplateDeductionResult ConvertDeducedTemplateArguments(
     // Check whether we can actually use the default argument.
     if (S.CheckTemplateArgument(
             Param, DefArg, TD, TD->getLocation(), TD->getSourceRange().getEnd(),
-            0, SugaredBuilder, CanonicalBuilder, Sema::CTAK_Specified)) {
+            0, SugaredBuilder, CanonicalBuilder, Sema::CTAK_Specified,
+            /*MatchedPackOnParmToNonPackOnArg=*/nullptr)) {
       Info.Param = makeTemplateParameter(
                          const_cast<NamedDecl *>(TemplateParams->getParam(I)));
       // FIXME: These template arguments are temporary. Free them!
@@ -3314,16 +3324,20 @@ FinishTemplateArgumentDeduction(
     return TemplateDeductionResult::SubstitutionFailure;
   }
 
+  bool MatchedPackOnParmToNonPackOnArg = false;
   bool ConstraintsNotSatisfied;
   SmallVector<TemplateArgument, 4> SugaredConvertedInstArgs,
       CanonicalConvertedInstArgs;
   if (S.CheckTemplateArgumentList(
           Template, Partial->getLocation(), InstArgs, /*DefaultArgs=*/{}, false,
           SugaredConvertedInstArgs, CanonicalConvertedInstArgs,
-          /*UpdateArgsWithConversions=*/true, &ConstraintsNotSatisfied))
+          /*UpdateArgsWithConversions=*/true, &ConstraintsNotSatisfied,
+          /*PartialOrderingTTP=*/false, &MatchedPackOnParmToNonPackOnArg))
     return ConstraintsNotSatisfied
                ? TemplateDeductionResult::ConstraintsNotSatisfied
                : TemplateDeductionResult::SubstitutionFailure;
+  if (MatchedPackOnParmToNonPackOnArg)
+    Info.setMatchedPackOnParmToNonPackOnArg();
 
   TemplateParameterList *TemplateParams = Template->getTemplateParameters();
   for (unsigned I = 0, E = TemplateParams->size(); I != E; ++I) {
@@ -6470,8 +6484,8 @@ bool Sema::isMoreSpecializedThanPrimary(
 
 bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
     TemplateParameterList *P, TemplateDecl *PArg, TemplateDecl *AArg,
-    const DefaultArguments &DefaultArgs, SourceLocation ArgLoc,
-    bool IsDeduced) {
+    const DefaultArguments &DefaultArgs, SourceLocation ArgLoc, bool IsDeduced,
+    bool *MatchedPackOnParmToNonPackOnArg) {
   // C++1z [temp.arg.template]p4: (DR 150)
   //   A template template-parameter P is at least as specialized as a
   //   template template-argument A if, given the following rewrite to two
@@ -6523,11 +6537,11 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
     //   If the rewrite produces an invalid type, then P is not at least as
     //   specialized as A.
     SmallVector<TemplateArgument, 4> CanonicalPArgs;
-    if (CheckTemplateArgumentList(AArg, ArgLoc, PArgList, DefaultArgs, false,
-                                  PArgs, CanonicalPArgs,
-                                  /*UpdateArgsWithConversions=*/true,
-                                  /*ConstraintsNotSatisfied=*/nullptr,
-                                  /*PartialOrderingTTP=*/true))
+    if (CheckTemplateArgumentList(
+            AArg, ArgLoc, PArgList, DefaultArgs, false, PArgs, CanonicalPArgs,
+            /*UpdateArgsWithConversions=*/true,
+            /*ConstraintsNotSatisfied=*/nullptr,
+            /*PartialOrderingTTP=*/true, MatchedPackOnParmToNonPackOnArg))
       return false;
   }
 
@@ -6553,6 +6567,9 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
       IsDeduced ? PackFold::ArgumentToParameter : PackFold::Both,
       /*HasDeducedAnyParam=*/nullptr)) {
   case clang::TemplateDeductionResult::Success:
+    if (MatchedPackOnParmToNonPackOnArg &&
+        Info.hasMatchedPackOnParmToNonPackOnArg())
+      *MatchedPackOnParmToNonPackOnArg = true;
     break;
 
   case TemplateDeductionResult::MiscellaneousDeductionFailure:
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 898255ff7c6a32..9e6023a95ec190 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4004,11 +4004,11 @@ bool Sema::usesPartialOrExplicitSpecialization(
 /// Get the instantiation pattern to use to instantiate the definition of a
 /// given ClassTemplateSpecializationDecl (either the pattern of the primary
 /// template or of a partial specialization).
-static ActionResult<CXXRecordDecl *>
-getPatternForClassTemplateSpecialization(
+static ActionResult<CXXRecordDecl *> getPatternForClassTemplateSpecialization(
     Sema &S, SourceLocation PointOfInstantiation,
     ClassTemplateSpecializationDecl *ClassTemplateSpec,
-    TemplateSpecializationKind TSK) {
+    TemplateSpecializationKind TSK,
+    bool PrimaryHasMatchedPackOnParmToNonPackOnArg) {
   Sema::InstantiatingTemplate Inst(S, PointOfInstantiation, ClassTemplateSpec);
   if (Inst.isInvalid())
     return {/*Invalid=*/true};
@@ -4031,7 +4031,7 @@ getPatternForClassTemplateSpecialization(
     //   specialization with the template argument lists of the partial
     //   specializations.
     typedef PartialSpecMatchResult MatchResult;
-    SmallVector<MatchResult, 4> Matched;
+    SmallVector<MatchResult, 4> Matched, ExtraMatched;
     SmallVector<ClassTemplatePartialSpecializationDecl *, 4> PartialSpecs;
     Template->getPartialSpecializations(PartialSpecs);
     TemplateSpecCandidateSet FailedCandidates(PointOfInstantiation);
@@ -4048,11 +4048,13 @@ getPatternForClassTemplateSpecialization(
             MakeDeductionFailureInfo(S.Context, Result, Info));
         (void)Result;
       } else {
-        Matched.push_back(PartialSpecMatchResult());
-        Matched.back().Partial = Partial;
-        Matched.back().Args = Info.takeCanonical();
+        auto &List =
+            Info.hasMatchedPackOnParmToNonPackOnArg() ? ExtraMatched : Matched;
+        List.push_back(MatchResult{Partial, Info.takeCanonical()});
       }
     }
+    if (Matched.empty() && PrimaryHasMatchedPackOnParmToNonPackOnArg)
+      Matched = std::move(ExtraMatched);
 
     // If we're dealing with a member template where the template parameters
     // have been instantiated, this provides the original template parameters
@@ -4155,7 +4157,8 @@ getPatternForClassTemplateSpecialization(
 bool Sema::InstantiateClassTemplateSpecialization(
     SourceLocation PointOfInstantiation,
     ClassTemplateSpecializationDecl *ClassTemplateSpec,
-    TemplateSpecializationKind TSK, bool Complain) {
+    TemplateSpecializationKind TSK, bool Complain,
+    bool PrimaryHasMatchedPackOnParmToNonPackOnArg) {
   // Perform the actual instantiation on the canonical declaration.
   ClassTemplateSpec = cast<ClassTemplateSpecializationDecl>(
       ClassTemplateSpec->getCanonicalDecl());
@@ -4163,8 +4166,9 @@ bool Sema::InstantiateClassTemplateSpecialization(
     return true;
 
   ActionResult<CXXRecordDecl *> Pattern =
-      getPatternForClassTemplateSpecialization(*this, PointOfInstantiation,
-                                        ...
[truncated]

@cor3ntin
Copy link
Contributor

cor3ntin commented Oct 6, 2024

@mizvekov
Copy link
Contributor Author

mizvekov commented Oct 6, 2024

This implements the wording I presented here: https://lists.isocpp.org/core/2024/09/16266.php
This will be part of the next revision of P3310.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a reasonable fallback, and, IMO, needs implementation experience before WG21 sees it, so I'm in favor.

@mizvekov mizvekov changed the base branch from main to users/mizvekov/clang-p0522-complete-implementation October 9, 2024 16:50
@mizvekov mizvekov force-pushed the users/mizvekov/clang-cwg2398-improve-overload-resolution branch from 9b12c0b to aa01604 Compare October 10, 2024 04:46
Base automatically changed from users/mizvekov/clang-p0522-complete-implementation to main October 10, 2024 07:40
With this change, we discriminate if the primary template and which partial
specializations would have participated in overload resolution prior to
P0522 changes.

We collect those in an initial set. If this set is not empty, or the
primary template would have matched, we proceed with this set as the
candidates for overload resolution.

Otherwise, we build a new overload set with everything else, and proceed
as usual.
@mizvekov mizvekov force-pushed the users/mizvekov/clang-cwg2398-improve-overload-resolution branch from aa01604 to e348747 Compare October 10, 2024 07:44
@mizvekov mizvekov merged commit 224519b into main Oct 10, 2024
6 of 9 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-cwg2398-improve-overload-resolution branch October 10, 2024 07:50
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 10, 2024

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building clang at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/8122

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: Process/Utility/./ProcessUtilityTests/34/39 (2677 of 2686)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/convenience_variables.test (2678 of 2686)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/independent_state.test (2679 of 2686)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/interactive_crashlog_legacy.test (2680 of 2686)
PASS: lldb-api :: api/multithreaded/TestMultithreaded.py (2681 of 2686)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteFork.py (2682 of 2686)
PASS: lldb-api :: tools/lldb-server/TestLldbGdbServer.py (2683 of 2686)
PASS: lldb-api :: commands/process/attach/TestProcessAttach.py (2684 of 2686)
PASS: lldb-api :: repl/clang/TestClangREPL.py (2685 of 2686)
UNRESOLVED: lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py (2686 of 2686)
******************** TEST 'lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/commands/gui/spawn-threads -p TestGuiSpawnThreads.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 224519b08945637a85e9798c78286643288f7b77)
  clang revision 224519b08945637a85e9798c78286643288f7b77
  llvm revision 224519b08945637a85e9798c78286643288f7b77
(lldb) settings clear -all
(lldb) settings set symbols.enable-external-lookup false
(lldb) settings set target.inherit-tcc true
(lldb) settings set target.disable-aslr false
(lldb) settings set target.detach-on-error false
(lldb) settings set target.auto-apply-fixits false
(lldb) settings set plugin.process.gdb-remote.packet-timeout 60
(lldb) settings set symbols.clang-modules-cache-path "/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api"
(lldb) settings set use-color false
(lldb) target create "/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/commands/gui/spawn-threads/TestGuiSpawnThreads.test_gui/a.out"

Parsing symbol table: a.out...�[K

�[2K
Locating external symbol file: a.out...�[K

�[2K
Locating external symbol file: a.out...�[K

�[2K
Locating external symbol file: a.out...�[K

�[2K
[0/10] Manually indexing DWARF: a.out...�[K

[1/10] Manually indexing DWARF: a.out...�[K


metaflow added a commit that referenced this pull request Oct 11, 2024
ichaer added a commit to splunk/ichaer-llvm-project that referenced this pull request Oct 11, 2024
…ent-indentonly

* llvm-trunk/main: (6379 commits)
  [gn build] Port 1c94388
  [RISCV] Introduce VLOptimizer pass (llvm#108640)
  [mlir][vector] Add more tests for ConvertVectorToLLVM (7/n) (llvm#111895)
  [libc++] Add output groups to run-buildbot (llvm#111739)
  [libc++abi] Remove unused LIBCXXABI_LIBCXX_INCLUDES CMake option (llvm#111824)
  [clang] Ignore inline namespace for `hasName` (llvm#109147)
  [AArch64] Disable consecutive store merging when Neon is unavailable (llvm#111519)
  [lldb] Fix finding make tool for tests (llvm#111980)
  Turn `-Wdeprecated-literal-operator` on by default (llvm#111027)
  [AMDGPU] Rewrite RegSeqNames using !foreach. NFC. (llvm#111994)
  Revert "Reland: [clang] Finish implementation of P0522 (llvm#111711)"
  Revert "[clang] CWG2398: improve overload resolution backwards compat (llvm#107350)"
  Revert "[clang] Implement TTP P0522 pack matching for deduced function template calls. (llvm#111457)"
  [Clang] Replace Intrinsic::getDeclaration with getOrInsertDeclaration (llvm#111990)
  Revert "[NVPTX] Prefer prmt.b32 over bfi.b32 (llvm#110766)"
  [RISCV] Add DAG combine to turn (sub (shl X, 8-Y), (shr X, Y)) into orc.b (llvm#111828)
  [libc] Fix compilation of new trig functions (llvm#111987)
  [NFC] Rename `Intrinsic::getDeclaration` to `getOrInsertDeclaration` (llvm#111752)
  [NFC][CodingStandard] Add additional example for if-else brace rule (llvm#111733)
  CodeGen: Remove redundant REQUIRES registered-target from tests (llvm#111982)
  ...
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…07350)

With this change, we discriminate if the primary template and which
partial specializations would have participated in overload resolution
prior to P0522 changes.

We collect those in an initial set. If this set is not empty, or the
primary template would have matched, we proceed with this set as the
candidates for overload resolution.

Otherwise, we build a new overload set with everything else, and proceed
as usual.
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
mizvekov added a commit that referenced this pull request Jan 23, 2025
…ation of P3310 (#124137)

This patch relands the following PRs:
* #111711
* #107350
* #111457

All of these patches were reverted due to an issue reported in
#111711 (comment),
due to interdependencies.

---
[clang] Finish implementation of P0522

This finishes the clang implementation of P0522, getting rid
of the fallback to the old, pre-P0522 rules.

Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
  generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
  generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.

This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.

With this patch, we apply the P0522 rules throughout.

This needed to extend template argument deduction in order
to accept the historial rule for TTP matching pack parameter to non-pack
arguments.
This change also makes us accept some combinations of historical and P0522
allowances we wouldn't before.

It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.

This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.

The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics,
and to keep them consistent with each other.

The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.

The approach taken here is to mostly rely on existing diagnostics,
and create a new instantiation context that keeps track of this context.

So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining
that it occurred in the context of partial ordering this template
argument against that template parameter.

This diverges from the old diagnostics, which would lead with an
error pointing to the template argument, explain the problem
in subsequent notes, and produce a final note pointing to the parameter.

---
[clang] CWG2398: improve overload resolution backwards compat

With this change, we discriminate if the primary template and which partial
specializations would have participated in overload resolution prior to
P0522 changes.

We collect those in an initial set. If this set is not empty, or the
primary template would have matched, we proceed with this set as the
candidates for overload resolution.

Otherwise, we build a new overload set with everything else, and proceed
as usual.

---
[clang] Implement TTP 'reversed' pack matching for deduced function template calls.

Clang previously missed implementing P0522 pack matching
for deduced function template calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants