[libcxx-commits] [clang] [libcxx] [Clang] Implement CWG2369 "Ordering between constraints and substitution" (PR #102857)

[Younan Zhang via libcxx-commits]

================
@@ -1122,6 +1122,154 @@ bool Sema::CheckInstantiatedFunctionTemplateConstraints(
                                      PointOfInstantiation, Satisfaction);
 }
 
+namespace {
+
+// We employ a TreeTransform because RAV couldn't recurse into a bunch of
+// Exprs e.g. SizeOfPackExpr, CXXFoldExpr, etc.
+// FIXME: Could we do the Decl instantiation as we substitute into
+// the constraint expressions?
+class InstantiateReferencedParameter
+    : public TreeTransform<InstantiateReferencedParameter> {
+  const MultiLevelTemplateArgumentList &TemplateArgs;
+
+  llvm::SmallPtrSet<ParmVarDecl *, 4> InstantiatedDecls;
+
+  FunctionDecl *PrimaryTemplatedFunction;
+
+  using inherited = TreeTransform<InstantiateReferencedParameter>;
+
+  bool instantiateParameterToScope(ParmVarDecl *OldParm,
+                                   LocalInstantiationScope &Scope) {
+    // The current context might have been changed by lambda expressions. So
+    // resume it before we substitute into parameters.
+    Sema::ContextRAII Context(SemaRef, PrimaryTemplatedFunction);
+    std::optional<unsigned> NumExpansions;
+    ParmVarDecl *NewParm = nullptr;
+    unsigned IndexAdjustment = 0;
+    if (OldParm->isParameterPack()) {
+      SmallVector<UnexpandedParameterPack, 2> Unexpanded;
+      TypeLoc TL = OldParm->getTypeSourceInfo()->getTypeLoc();
+      PackExpansionTypeLoc ExpansionTL = TL.castAs<PackExpansionTypeLoc>();
+      TypeLoc Pattern = ExpansionTL.getPatternLoc();
+      SemaRef.collectUnexpandedParameterPacks(Pattern, Unexpanded);
+
+      assert(!Unexpanded.empty() &&
+             "A pack Decl doesn't contain anything unexpanded?");
+
+      bool ShouldExpand = false;
+      bool RetainExpansion = false;
+      std::optional<unsigned> OrigNumExpansions =
+          ExpansionTL.getTypePtr()->getNumExpansions();
+      NumExpansions = OrigNumExpansions;
+      if (SemaRef.CheckParameterPacksForExpansion(
+              ExpansionTL.getEllipsisLoc(), Pattern.getSourceRange(),
+              Unexpanded, TemplateArgs, ShouldExpand, RetainExpansion,
+              NumExpansions))
+        return true;
+
+      assert(ShouldExpand && !RetainExpansion &&
+             "Shouldn't retain an expansion here!");
+      Scope.MakeInstantiatedLocalArgPack(OldParm);
+
+      for (unsigned I = 0; I != *NumExpansions; ++I) {
+        Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(SemaRef, I);
+        ParmVarDecl *NewParm = SemaRef.SubstParmVarDecl(
+            OldParm, TemplateArgs, /*indexAdjustment=*/IndexAdjustment++,
+            NumExpansions, /*ExpectParameterPack=*/false,
+            /*EvaluateConstraints=*/false);
+        if (!NewParm)
+          return true;
+      }
+
+      return false;
+    }
+    NewParm = SemaRef.SubstParmVarDecl(OldParm, TemplateArgs,
+                                       /*indexAdjustment=*/IndexAdjustment,
+                                       std::nullopt,
+                                       /*ExpectParameterPack=*/false);
+    if (!NewParm)
+      return true;
+    Scope.InstantiatedLocal(OldParm, NewParm);
+    return false;
+  }
+
+public:
+  InstantiateReferencedParameter(
+      Sema &SemaRef, const MultiLevelTemplateArgumentList &TemplateArgs,
+      FunctionDecl *PrimaryTemplatedFunction)
+      : inherited(SemaRef), TemplateArgs(TemplateArgs),
+        PrimaryTemplatedFunction(PrimaryTemplatedFunction) {}
+
+  Decl *TransformDecl(SourceLocation Loc, Decl *D) {
+    if (auto *PVD = dyn_cast_if_present<ParmVarDecl>(D);
+        PVD && PVD->getDeclContext() == PrimaryTemplatedFunction &&
+        !InstantiatedDecls.contains(PVD)) {
+      instantiateParameterToScope(PVD, *SemaRef.CurrentInstantiationScope);
----------------
zyn0217 wrote:

That would cause an implementation divergence with GCC: https://gcc.godbolt.org/z/7MobE9b61
If we instantiate all parameters, we wouldn't accept such a case that GCC currently accepts.

The standard seems *vague* ([It](https://cplusplus.github.io/CWG/issues/2369.html) just said `function type` shouldn't be replaced before the constraint evaluation) about this, but I presume the intent was to instantiate parameters on demand.

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