Instantiate argument default parameters (#5443)

llvm-beanz · web-flow · commit 573b65221079 · 2023-07-24T10:19:07.000-05:00
This addresses a latent bug in Clang that was fixed _way_ after DXC was forked. We encounter it in DXC because of the way that we inject AST bits through the external sema source, but it likely wasn't exposable through C++. This change is mostly a back porting of the following upstream LLVM commits: llvm-project/73c6a2448f24 llvm-project/f721e0582b15 llvm-project/c601377b2376 llvm-project/4409a83c2935 The end result of this change is that we still lazily instantiate default arguments, but we do force them to be instantiated during sema when they are used. Fixes #5145
diff --git a/tools/clang/include/clang/AST/DeclBase.h b/tools/clang/include/clang/AST/DeclBase.h
@@ -721,6 +721,22 @@ class Decl {
     return getParentFunctionOrMethod() == nullptr;
   }
 
+  /// HLSL Change Begin - back port from llvm-project/73c6a2448f24 &
+  /// f721e0582b15. Determine whether a substitution into this declaration would
+  /// occur as part of a substitution into a dependent local scope. Such a
+  /// substitution transitively substitutes into all constructs nested within
+  /// this declaration.
+  ///
+  /// This recognizes non-defining declarations as well as members of local
+  /// classes and lambdas:
+  /// \code
+  ///     template<typename T> void foo() { void bar(); }
+  ///     template<typename T> void foo2() { class ABC { void bar(); }; }
+  ///     template<typename T> inline int x = [](){ return 0; }();
+  /// \endcode
+  bool isInLocalScopeForInstantiation() const;
+  /// HLSL Change End - back port from llvm-project/73c6a2448f24 & f721e0582b15.
+
   /// \brief If this decl is defined inside a function/method/block it returns
   /// the corresponding DeclContext, otherwise it returns null.
   const DeclContext *getParentFunctionOrMethod() const;
diff --git a/tools/clang/include/clang/Sema/Sema.h b/tools/clang/include/clang/Sema/Sema.h
@@ -2736,6 +2736,13 @@ class Sema {
                              const ObjCObjectPointerType *OPT,
                              bool ErrorRecovery);
 
+  /// HLSL Change Begin - back ported from llvm-project/c601377b2376.
+  bool addInstantiatedParametersToScope(
+      FunctionDecl *Function, const FunctionDecl *PatternDecl,
+      LocalInstantiationScope &Scope,
+      const MultiLevelTemplateArgumentList &TemplateArgs);
+  /// HLSL Change End - back ported from llvm-project/c601377b2376.
+
 public:
   const TypoExprState &getTypoExprState(TypoExpr *TE) const;
 
@@ -6973,6 +6980,12 @@ class Sema {
                       const MultiLevelTemplateArgumentList &TemplateArgs,
                       SmallVectorImpl<QualType> &ParamTypes,
                       SmallVectorImpl<ParmVarDecl *> *OutParams = nullptr);
+  /// HLSL Change Begin - back ported from llvm-project/4409a83c2935.
+  bool SubstDefaultArgument(SourceLocation Loc, ParmVarDecl *Param,
+                            const MultiLevelTemplateArgumentList &TemplateArgs,
+                            bool ForCallExpr = false);
+  /// HLSL Change End - back ported from llvm-project/4409a83c2935.
+
   ExprResult SubstExpr(Expr *E,
                        const MultiLevelTemplateArgumentList &TemplateArgs);
 
diff --git a/tools/clang/lib/AST/DeclBase.cpp b/tools/clang/lib/AST/DeclBase.cpp
@@ -254,6 +254,25 @@ void Decl::setDeclContextsImpl(DeclContext *SemaDC, DeclContext *LexicalDC,
   }
 }
 
+/// HLSL Change Begin - back port from llvm-project/73c6a2448f24 & f721e0582b15.
+bool Decl::isInLocalScopeForInstantiation() const {
+  const DeclContext *LDC = getLexicalDeclContext();
+  if (!LDC->isDependentContext())
+    return false;
+  while (true) {
+    if (LDC->isFunctionOrMethod())
+      return true;
+    if (!isa<TagDecl>(LDC))
+      return false;
+    if (const auto *CRD = dyn_cast<CXXRecordDecl>(LDC))
+      if (CRD->isLambda())
+        return true;
+    LDC = LDC->getLexicalParent();
+  }
+  return false;
+}
+/// HLSL Change End - back port from llvm-project/73c6a2448f24 & f721e0582b15.
+
 bool Decl::isInAnonymousNamespace() const {
   const DeclContext *DC = getDeclContext();
   do {
diff --git a/tools/clang/lib/Sema/SemaExpr.cpp b/tools/clang/lib/Sema/SemaExpr.cpp
@@ -4380,50 +4380,15 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
   }
   
   if (Param->hasUninstantiatedDefaultArg()) {
-    Expr *UninstExpr = Param->getUninstantiatedDefaultArg();
-
-    EnterExpressionEvaluationContext EvalContext(*this, PotentiallyEvaluated,
-                                                 Param);
-
+    /// HLSL Change Begin - back ported from llvm-project/4409a83c2935.
     // Instantiate the expression.
     MultiLevelTemplateArgumentList MutiLevelArgList
       = getTemplateInstantiationArgs(FD, nullptr, /*RelativeToPrimary=*/true);
 
-    InstantiatingTemplate Inst(*this, CallLoc, Param,
-                               MutiLevelArgList.getInnermost());
-    if (Inst.isInvalid())
-      return ExprError();
-
-    ExprResult Result;
-    {
-      // C++ [dcl.fct.default]p5:
-      //   The names in the [default argument] expression are bound, and
-      //   the semantic constraints are checked, at the point where the
-      //   default argument expression appears.
-      ContextRAII SavedContext(*this, FD);
-      LocalInstantiationScope Local(*this);
-      Result = SubstExpr(UninstExpr, MutiLevelArgList);
-    }
-    if (Result.isInvalid())
+    if (SubstDefaultArgument(CallLoc, Param, MutiLevelArgList,
+                             /*ForCallExpr*/ true))
       return ExprError();
-
-    // Check the expression as an initializer for the parameter.
-    InitializedEntity Entity
-      = InitializedEntity::InitializeParameter(Context, Param);
-    InitializationKind Kind
-      = InitializationKind::CreateCopy(Param->getLocation(),
-             /*FIXME:EqualLoc*/UninstExpr->getLocStart());
-    Expr *ResultE = Result.getAs<Expr>();
-
-    InitializationSequence InitSeq(*this, Entity, Kind, ResultE);
-    Result = InitSeq.Perform(*this, Entity, Kind, ResultE);
-    if (Result.isInvalid())
-      return ExprError();
-
-    Expr *Arg = Result.getAs<Expr>();
-    CheckCompletedExpr(Arg, Param->getOuterLocStart());
-    // Build the default argument expression.
-    return CXXDefaultArgExpr::Create(Context, CallLoc, Param, Arg);
+    /// HLSL Change End - back ported from llvm-project/4409a83c2935.
   }
 
   // If the default expression creates temporaries, we need to
diff --git a/tools/clang/lib/Sema/SemaTemplateInstantiate.cpp b/tools/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1742,6 +1742,86 @@ bool Sema::SubstParmTypes(SourceLocation Loc,
                                                   OutParams);
 }
 
+/// HLSL Change Begin - back ported from llvm-project/4409a83c2935.
+/// Substitute the given template arguments into the default argument.
+bool Sema::SubstDefaultArgument(
+    SourceLocation Loc,
+    ParmVarDecl *Param,
+    const MultiLevelTemplateArgumentList &TemplateArgs,
+    bool ForCallExpr) {
+  FunctionDecl *FD = cast<FunctionDecl>(Param->getDeclContext());
+  Expr *PatternExpr = Param->getUninstantiatedDefaultArg();
+
+  EnterExpressionEvaluationContext EvalContext(
+      *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+
+  InstantiatingTemplate Inst(*this, Loc, Param, TemplateArgs.getInnermost());
+  if (Inst.isInvalid())
+    return true;
+
+  ExprResult Result;
+  {
+    // C++ [dcl.fct.default]p5:
+    //   The names in the [default argument] expression are bound, and
+    //   the semantic constraints are checked, at the point where the
+    //   default argument expression appears.
+    ContextRAII SavedContext(*this, FD);
+    std::unique_ptr<LocalInstantiationScope> LIS;
+
+    if (ForCallExpr) {
+      // When instantiating a default argument due to use in a call expression,
+      // an instantiation scope that includes the parameters of the callee is
+      // required to satisfy references from the default argument. For example:
+      //   template<typename T> void f(T a, int = decltype(a)());
+      //   void g() { f(0); }
+      LIS = std::make_unique<LocalInstantiationScope>(*this);
+      FunctionDecl *PatternFD = FD->getTemplateInstantiationPattern();
+      if (addInstantiatedParametersToScope(FD, PatternFD, *LIS, TemplateArgs))
+        return true;
+    }
+
+    Result = SubstInitializer(PatternExpr, TemplateArgs,
+                              /*DirectInit*/false);
+  }
+  if (Result.isInvalid())
+    return true;
+
+  if (ForCallExpr) {
+    // Check the expression as an initializer for the parameter.
+    if (RequireCompleteType(Param->getLocation(), Param->getType(),
+                          diag::err_typecheck_decl_incomplete_type))
+       return true;
+    InitializedEntity Entity
+      = InitializedEntity::InitializeParameter(Context, Param);
+    InitializationKind Kind = InitializationKind::CreateCopy(
+        Param->getLocation(),
+        /*FIXME:EqualLoc*/ PatternExpr->getLocStart());
+    Expr *ResultE = Result.getAs<Expr>();
+
+    InitializationSequence InitSeq(*this, Entity, Kind, ResultE);
+    Result = InitSeq.Perform(*this, Entity, Kind, ResultE);
+    if (Result.isInvalid())
+      return true;
+
+    Result =
+        ActOnFinishFullExpr(Result.getAs<Expr>(), Param->getOuterLocStart(),
+                            /*DiscardedValue*/ false);
+  } else {
+    // FIXME: Obtain the source location for the '=' token.
+    SourceLocation EqualLoc = PatternExpr->getLocStart();
+    Result = SetParamDefaultArgument(Param, Result.getAs<Expr>(), EqualLoc);
+  }
+  if (Result.isInvalid())
+      return true;
+
+  // Remember the instantiated default argument.
+  Param->setDefaultArg(Result.getAs<Expr>());
+
+  return false;
+}
+
+/// HLSL Change End - back ported from llvm-project/4409a83c2935.
+
 /// \brief Perform substitution on the base class specifiers of the
 /// given class template specialization.
 ///
diff --git a/tools/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/tools/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1565,6 +1565,33 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
       Previous.clear();
   }
 
+  // HLSL Change Begin - back ported from llvm-project/4409a83c2935.
+  // Per [temp.inst], default arguments in function declarations at local scope
+  // are instantiated along with the enclosing declaration. For example:
+  //
+  //   template<typename T>
+  //   void ft() {
+  //     void f(int = []{ return T::value; }());
+  //   }
+  //   template void ft<int>(); // error: type 'int' cannot be used prior
+  //                                      to '::' because it has no members
+  //
+  // The error is issued during instantiation of ft<int>() because substitution
+  // into the default argument fails; the default argument is instantiated even
+  // though it is never used.
+  if (Function->isLocalExternDecl()) {
+    for (ParmVarDecl *PVD : Function->parameters()) {
+      if (!PVD->hasDefaultArg())
+        continue;
+      if (SemaRef.SubstDefaultArgument(D->getInnerLocStart(), PVD,
+                                       TemplateArgs)) {
+        Function->setInvalidDecl();
+        return nullptr;
+      }
+    }
+  }
+  // HLSL Change End - back ported from llvm-project/4409a83c2935.
+
   SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous,
                                    isExplicitSpecialization);
 
@@ -1862,6 +1889,33 @@ TemplateDeclInstantiator::VisitCXXMethodDecl(CXXMethodDecl *D,
       Previous.clear();
   }
 
+  // HLSL Change Begin - back ported from llvm-project/4409a83c2935.
+  // Per [temp.inst], default arguments in member functions of local classes
+  // are instantiated along with the member function declaration. For example:
+  //
+  //   template<typename T>
+  //   void ft() {
+  //     struct lc {
+  //       int operator()(int p = []{ return T::value; }());
+  //     };
+  //   }
+  //   template void ft<int>(); // error: type 'int' cannot be used prior
+  //                                      to '::'because it has no members
+  //
+  // The error is issued during instantiation of ft<int>()::lc::operator()
+  // because substitution into the default argument fails; the default argument
+  // is instantiated even though it is never used.
+  if (D->isInLocalScopeForInstantiation()) {
+    for (unsigned P = 0; P < Params.size(); ++P) {
+      if (!Params[P]->hasDefaultArg())
+        continue;
+      if (SemaRef.SubstDefaultArgument(StartLoc, Params[P], TemplateArgs)) {
+        return nullptr;
+      }
+    }
+  }
+  // HLSL Change End - back ported from llvm-project/4409a83c2935.
+
   if (!IsClassScopeSpecialization)
     SemaRef.CheckFunctionDeclaration(nullptr, Method, Previous, false);
 
@@ -3127,10 +3181,11 @@ TemplateDeclInstantiator::SubstFunctionType(FunctionDecl *D,
 /// Introduce the instantiated function parameters into the local
 /// instantiation scope, and set the parameter names to those used
 /// in the template.
-static bool addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function,
-                                             const FunctionDecl *PatternDecl,
-                                             LocalInstantiationScope &Scope,
-                           const MultiLevelTemplateArgumentList &TemplateArgs) {
+/// HLSL Change Begin - back ported from llvm-project/601377b23767.
+bool Sema::addInstantiatedParametersToScope(
+    FunctionDecl *Function, const FunctionDecl *PatternDecl,
+    LocalInstantiationScope &Scope,
+    const MultiLevelTemplateArgumentList &TemplateArgs) {
   unsigned FParamIdx = 0;
   for (unsigned I = 0, N = PatternDecl->getNumParams(); I != N; ++I) {
     const ParmVarDecl *PatternParam = PatternDecl->getParamDecl(I);
@@ -3146,7 +3201,7 @@ static bool addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function,
       // it's instantiation-dependent.
       // FIXME: Updating the type to work around this is at best fragile.
       if (!PatternDecl->getType()->isDependentType()) {
-        QualType T = S.SubstType(PatternParam->getType(), TemplateArgs,
+        QualType T = SubstType(PatternParam->getType(), TemplateArgs,
                                  FunctionParam->getLocation(),
                                  FunctionParam->getDeclName());
         if (T.isNull())
@@ -3162,7 +3217,7 @@ static bool addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function,
     // Expand the parameter pack.
     Scope.MakeInstantiatedLocalArgPack(PatternParam);
     Optional<unsigned> NumArgumentsInExpansion
-      = S.getNumArgumentsInExpansion(PatternParam->getType(), TemplateArgs);
+      = getNumArgumentsInExpansion(PatternParam->getType(), TemplateArgs);
     assert(NumArgumentsInExpansion &&
            "should only be called when all template arguments are known");
     QualType PatternType =
@@ -3171,8 +3226,8 @@ static bool addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function,
       ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx);
       FunctionParam->setDeclName(PatternParam->getDeclName());
       if (!PatternDecl->getType()->isDependentType()) {
-        Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(S, Arg);
-        QualType T = S.SubstType(PatternType, TemplateArgs,
+        Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, Arg);
+        QualType T = SubstType(PatternType, TemplateArgs,
                                  FunctionParam->getLocation(),
                                  FunctionParam->getDeclName());
         if (T.isNull())
@@ -3187,6 +3242,7 @@ static bool addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function,
 
   return false;
 }
+/// HLSL Change End - back ported from llvm-project/601377b23767.
 
 void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
                                     FunctionDecl *Decl) {
@@ -3212,7 +3268,8 @@ void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
     getTemplateInstantiationArgs(Decl, nullptr, /*RelativeToPrimary*/true);
 
   FunctionDecl *Template = Proto->getExceptionSpecTemplate();
-  if (addInstantiatedParametersToScope(*this, Decl, Template, Scope,
+  // HLSL Change - back ported from llvm-project/601377b23767.
+  if (addInstantiatedParametersToScope(Decl, Template, Scope,
                                        TemplateArgs)) {
     UpdateExceptionSpec(Decl, EST_None);
     return;
@@ -3494,7 +3551,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
     // PushDeclContext because we don't have a scope.
     Sema::ContextRAII savedContext(*this, Function);
 
-    if (addInstantiatedParametersToScope(*this, Function, PatternDecl, Scope,
+    // HLSL Change - back ported from llvm-project/601377b23767.
+    if (addInstantiatedParametersToScope(Function, PatternDecl, Scope,
                                          TemplateArgs))
       return;
 
diff --git a/tools/clang/test/CodeGenSPIRV/fn.param.default.hlsl b/tools/clang/test/CodeGenSPIRV/fn.param.default.hlsl
@@ -12,8 +12,7 @@ float4 main(uint vertex_id : SV_VertexID) : SV_Position
   // CHECK: OpStore %param_var_b %float_2
   // CHECK: [[first:%\d+]] = OpFunctionCall %float %test %param_var_a %param_var_b
   // CHECK: OpStore %param_var_a_0 %float_4
-  // CHECK: [[default:%\d+]] = OpConvertSToF %float %int_0
-  // CHECK: OpStore %param_var_b_0 [[default]]
+  // CHECK: OpStore %param_var_b_0 %float_0
   // CHECK: [[second:%\d+]] = OpFunctionCall %float %test %param_var_a_0 %param_var_b_0
   // CHECK: {{%\d+}} = OpCompositeConstruct %v4float [[first]] [[second]] %float_0 %float_0
   return float4(test<float>(1,2), test<float>(4), 0, 0);
diff --git a/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArg.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArg.hlsl
diff --git a/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArgMember.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArgMember.hlsl
diff --git a/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArgResource.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateDefaultArgResource.hlsl
