[Clang] Normalize constraints before checking for satisfaction

clang/include/clang/Sema/Sema.h

@@ -12776,6 +12776,18 @@ class Sema final : public SemaBase {
   void MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
                                   unsigned Depth, llvm::SmallBitVector &Used);
 
+  /// Mark which template parameters are named in a given expression.
+  ///
+  /// Unlike MarkUsedTemplateParameters, this excludes parameter that
+  /// are used but not directly named by an expression - ie it excludes
+  /// any template parameter that denotes the type of a referenced NTTP.
+  ///
+  /// \param Used a bit vector whose elements will be set to \c true
+  /// to indicate when the corresponding template parameter will be
+  /// deduced.
+  void MarkUsedTemplateParametersForSubsumptionParameterMapping(
+      const Expr *E, unsigned Depth, llvm::SmallBitVector &Used);
+
   /// Mark which template parameters can be deduced from a given
   /// template argument list.
   ///

clang/include/clang/Sema/SemaConcept.h

@@ -61,9 +61,15 @@ struct NormalizedConstraint {
     unsigned Kind : 5;
     unsigned Placeholder : 1;
     unsigned PackSubstitutionIndex : 26;
-    // Indexes and Args are part of the common initial sequences
-    // of constraints that do have a mapping.
+    // Indexes, IndexesForSubsumption, and Args are part of the common initial
+    // sequences of constraints that do have a mapping.
+
+    // Indexes of the parameters used in a constraint expression.
     OccurenceList Indexes;
+    // Indexes of the parameters named directly in a constraint expression.
+    // FIXME: we should try to reduce the size of this struct?
+    OccurenceList IndexesForSubsumption;
+
     TemplateArgumentLoc *Args;
     TemplateParameterList *ParamList;
     ExprOrConcept ConstraintExpr;
@@ -77,6 +83,7 @@ struct NormalizedConstraint {
     unsigned FoldOperator : 1;
     unsigned Placeholder : 26;
     OccurenceList Indexes;
+    OccurenceList IndexesForSubsumption;
     TemplateArgumentLoc *Args;
     TemplateParameterList *ParamList;
     const Expr *Pattern;
@@ -119,6 +126,7 @@ struct NormalizedConstraint {
                /*Placeholder=*/0,
                PackIndex.toInternalRepresentation(),
                /*Indexes=*/{},
+               /*IndexesForSubsumption=*/{},
                /*Args=*/nullptr,
                /*ParamList=*/nullptr,
                ConstraintExpr,
@@ -131,6 +139,7 @@ struct NormalizedConstraint {
                      llvm::to_underlying(OpKind),
                      /*Placeholder=*/0,
                      /*Indexes=*/{},
+                     /*IndexesForSubsumption=*/{},
                      /*Args=*/nullptr,
                      /*ParamList=*/nullptr,
                      Pattern,
@@ -145,6 +154,7 @@ struct NormalizedConstraint {
       : ConceptId{{llvm::to_underlying(ConstraintKind::ConceptId),
                    /*Placeholder=*/0, PackIndex.toInternalRepresentation(),
                    /*Indexes=*/{},
+                   /*IndexesForSubsumption=*/{},
                    /*Args=*/nullptr, /*ParamList=*/nullptr, ConceptId,
                    ConstraintDecl},
                   SubConstraint,
@@ -166,6 +176,11 @@ struct NormalizedConstraint {
     return Atomic.Indexes;
   }
 
+  const OccurenceList &mappingOccurenceListForSubsumption() const {
+    assert(hasParameterMapping() && "This constraint has no parameter mapping");
+    return Atomic.IndexesForSubsumption;
+  }
+
   llvm::MutableArrayRef<TemplateArgumentLoc> getParameterMapping() const {
     return {Atomic.Args, Atomic.Indexes.count()};
   }
@@ -175,11 +190,16 @@ struct NormalizedConstraint {
   }
 
   void updateParameterMapping(OccurenceList Indexes,
+                              OccurenceList IndexesForSubsumption,
                               llvm::MutableArrayRef<TemplateArgumentLoc> Args,
                               TemplateParameterList *ParamList) {
     assert(getKind() != ConstraintKind::Compound);
     assert(Indexes.count() == Args.size());
-    Atomic.Indexes = Indexes;
+    assert(IndexesForSubsumption.size() == Indexes.size());
+    assert((Indexes & IndexesForSubsumption) == Indexes);
+
+    Atomic.IndexesForSubsumption = std::move(IndexesForSubsumption);
+    Atomic.Indexes = std::move(Indexes);
     Atomic.Args = Args.data();
     Atomic.ParamList = ParamList;
   }
@@ -198,10 +218,17 @@ struct NormalizedConstraint {
     llvm::ArrayRef<TemplateArgumentLoc> OtherParameterMapping =
         Other.getParameterMapping();
 
+    const OccurenceList &Indexes = mappingOccurenceListForSubsumption();
+    const OccurenceList &OtherIndexes =
+        Other.mappingOccurenceListForSubsumption();
+
     if (ParameterMapping.size() != OtherParameterMapping.size())
       return false;
-
     for (unsigned I = 0, S = ParameterMapping.size(); I < S; ++I) {
+      if (Indexes[I] != OtherIndexes[I])
+        return false;
+      if (!Indexes[I])
+        continue;
       llvm::FoldingSetNodeID IDA, IDB;
       C.getCanonicalTemplateArgument(ParameterMapping[I].getArgument())
           .Profile(IDA, C);
@@ -296,6 +323,7 @@ class NormalizedConstraintWithParamMapping : public NormalizedConstraint {
   using NormalizedConstraint::hasMatchingParameterMapping;
   using NormalizedConstraint::hasParameterMapping;
   using NormalizedConstraint::mappingOccurenceList;
+  using NormalizedConstraint::mappingOccurenceListForSubsumption;
   using NormalizedConstraint::updateParameterMapping;
 
   const NamedDecl *getConstraintDecl() const { return Atomic.ConstraintDecl; }

clang/lib/Sema/SemaConcept.cpp

@@ -1919,25 +1919,28 @@ void SubstituteParameterMappings::buildParameterMapping(
       cast<TemplateDecl>(N.getConstraintDecl())->getTemplateParameters();
 
   llvm::SmallBitVector OccurringIndices(TemplateParams->size());
+  llvm::SmallBitVector OccurringIndicesForSubsumption(TemplateParams->size());
 
   if (N.getKind() == NormalizedConstraint::ConstraintKind::Atomic) {
     SemaRef.MarkUsedTemplateParameters(
         static_cast<AtomicConstraint &>(N).getConstraintExpr(),
         /*OnlyDeduced=*/false,
         /*Depth=*/0, OccurringIndices);
+
+    SemaRef.MarkUsedTemplateParametersForSubsumptionParameterMapping(
+        static_cast<AtomicConstraint &>(N).getConstraintExpr(),
+        /*Depth=*/0, OccurringIndicesForSubsumption);
+
   } else if (N.getKind() ==
              NormalizedConstraint::ConstraintKind::FoldExpanded) {
     SemaRef.MarkUsedTemplateParameters(
         static_cast<FoldExpandedConstraint &>(N).getPattern(),
         /*OnlyDeduced=*/false,
         /*Depth=*/0, OccurringIndices);
-  } else if (N.getKind() == NormalizedConstraint::ConstraintKind::ConceptId) {
-    auto *Args = static_cast<ConceptIdConstraint &>(N)
-                     .getConceptId()
-                     ->getTemplateArgsAsWritten();
-    if (Args)
-      SemaRef.MarkUsedTemplateParameters(Args->arguments(),
-                                         /*Depth=*/0, OccurringIndices);
+
+    SemaRef.MarkUsedTemplateParametersForSubsumptionParameterMapping(
+        static_cast<FoldExpandedConstraint &>(N).getPattern(),
+        /*Depth=*/0, OccurringIndicesForSubsumption);
   }
   TemplateArgumentLoc *TempArgs =
       new (SemaRef.Context) TemplateArgumentLoc[OccurringIndices.count()];
@@ -1962,7 +1965,7 @@ void SubstituteParameterMappings::buildParameterMapping(
       /*RAngleLoc=*/SourceLocation(),
       /*RequiresClause=*/nullptr);
   N.updateParameterMapping(
-      OccurringIndices,
+      std::move(OccurringIndices), std::move(OccurringIndicesForSubsumption),
       MutableArrayRef<TemplateArgumentLoc>{TempArgs, OccurringIndices.count()},
       UsedList);
 }
@@ -2023,7 +2026,8 @@ bool SubstituteParameterMappings::substitute(
 
   MutableArrayRef<TemplateArgumentLoc> Mapping(TempArgs,
                                                CTAI.SugaredConverted.size());
-  N.updateParameterMapping(N.mappingOccurenceList(), Mapping,
+  N.updateParameterMapping(N.mappingOccurenceList(),
+                           N.mappingOccurenceListForSubsumption(), Mapping,
                            N.getUsedTemplateParamList());
   return false;
 }
@@ -2481,10 +2485,13 @@ auto SubsumptionChecker::find(const AtomicConstraint *Ori) -> Literal {
   ID.AddBoolean(Ori->hasParameterMapping());
   if (Ori->hasParameterMapping()) {
     const auto &Mapping = Ori->getParameterMapping();
-    for (const TemplateArgumentLoc &TAL : Mapping) {
-      SemaRef.getASTContext()
-          .getCanonicalTemplateArgument(TAL.getArgument())
-          .Profile(ID, SemaRef.getASTContext());
+    const NormalizedConstraint::OccurenceList &Indexes =
+        Ori->mappingOccurenceListForSubsumption();
+    for (auto [Idx, TAL] : llvm::enumerate(Mapping)) {
+      if (Indexes[Idx])
+        SemaRef.getASTContext()
+            .getCanonicalTemplateArgument(TAL.getArgument())
+            .Profile(ID, SemaRef.getASTContext());
     }
   }
   auto It = Elems.find(ID);

clang/lib/Sema/SemaTemplateDeduction.cpp

@@ -6702,10 +6702,11 @@ namespace {
 struct MarkUsedTemplateParameterVisitor : DynamicRecursiveASTVisitor {
   llvm::SmallBitVector &Used;
   unsigned Depth;
+  bool VisitDeclRefTypes = true;
 
-  MarkUsedTemplateParameterVisitor(llvm::SmallBitVector &Used,
-                                   unsigned Depth)
-      : Used(Used), Depth(Depth) { }
+  MarkUsedTemplateParameterVisitor(llvm::SmallBitVector &Used, unsigned Depth,
+                                   bool VisitDeclRefTypes = true)
+      : Used(Used), Depth(Depth), VisitDeclRefTypes(VisitDeclRefTypes) {}
 
   bool VisitTemplateTypeParmType(TemplateTypeParmType *T) override {
     if (T->getDepth() == Depth)
@@ -6726,7 +6727,8 @@ struct MarkUsedTemplateParameterVisitor : DynamicRecursiveASTVisitor {
     if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(E->getDecl()))
       if (NTTP->getDepth() == Depth)
         Used[NTTP->getIndex()] = true;
-    DynamicRecursiveASTVisitor::TraverseType(E->getType());
+    if (VisitDeclRefTypes)
+      DynamicRecursiveASTVisitor::TraverseType(E->getType());
     return true;
   }
 
@@ -7176,6 +7178,13 @@ Sema::MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
   ::MarkUsedTemplateParameters(Context, E, OnlyDeduced, Depth, Used);
 }
 
+void Sema::MarkUsedTemplateParametersForSubsumptionParameterMapping(
+    const Expr *E, unsigned Depth, llvm::SmallBitVector &Used) {
+  MarkUsedTemplateParameterVisitor(Used, Depth, /*VisitDeclRefTypes=*/false)
+      .TraverseStmt(const_cast<Expr *>(E));
+  return;
+}
+
 void
 Sema::MarkUsedTemplateParameters(const TemplateArgumentList &TemplateArgs,
                                  bool OnlyDeduced, unsigned Depth,

clang/test/SemaTemplate/instantiate-template-argument.cpp

@@ -13,20 +13,17 @@ constexpr int foo() requires C2<int, W> { return 1; } // #cand1
 // sizeof(U) >= 4 [U = W (decltype(int{}))]
 
 template<char X>
-// expected-note@+1{{candidate function}}
 constexpr int foo() requires C1<1, X> && true { return 2; } // #cand2
 // sizeof(U) >= 4 [U = X (decltype(1))]
 
 static_assert(foo<'a'>() == 2);
-// expected-error@-1 {{call to 'foo' is ambiguous}}
-// expected-note@#cand1 {{candidate function}}
-// expected-note@#cand2 {{candidate function}}
+
 
 template<char Z>
 constexpr int foo() requires C2<long long, Z> && true { return 3; } // #cand3
 // sizeof(U) >= 4 [U = Z (decltype(long long{}))]
 
 static_assert(foo<'a'>() == 3);
 // expected-error@-1{{call to 'foo' is ambiguous}}
-// expected-note@#cand1 {{candidate function}}
+// expected-note@#cand2 {{candidate function}}
 // expected-note@#cand3 {{candidate function}}