Properly instantiate a decltype in argument's default initializer

[Mordante]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=28500

[comex]

Ping.

[rsmith]

This isn't the right approach: instead of re-instantiating the parameter when we encounter a use of it, we should inject the parameters into the local instantiation scope before trying to instantiate a default argument (Sema::CheckCXXDefaultArgExpr should call addInstantiatedParametersToScope to populate the LocalInstantiationScope before calling SubstInitializer).

[Mordante]

Changed the approach to use addInstantiatedParametersToScope as suggested by @rsmith. Since the function was static in another file I made it a member of LocalInstantiationScope and adjusted all callers.

Minor changes:

• Unit test remove link to Bugzilla
• MutiLevelArgList -> MultiLevelArgList

[comex]

Ping. I'm not qualified to review this myself but I'd like to see the bug fixed. :)

[Mordante]

Friendly ping.

clang/include/clang/Sema/Template.h

public:
     }
 
     /// Retrieve the partially-substitued template parameter pack.
     ///
     /// If there is no partially-substituted parameter pack, returns NULL.
     NamedDecl *
     getPartiallySubstitutedPack(const TemplateArgument **ExplicitArgs = nullptr,
                                 unsigned *NumExplicitArgs = nullptr) const;
+
+    /// Introduce the instantiated function parameters into the local
+    /// instantiation scope, and set the parameter names to those used
+    /// in the template.
+    bool addInstantiatedParameters(
+        FunctionDecl *Function, const FunctionDecl *PatternDecl,
+        const MultiLevelTemplateArgumentList &TemplateArgs);
   };
 
   class TemplateDeclInstantiator
     : public DeclVisitor<TemplateDeclInstantiator, Decl *>
   {
     Sema &SemaRef;
     Sema::ArgumentPackSubstitutionIndexRAII SubstIndex;
     DeclContext *Owner;

clang/lib/Sema/SemaExpr.cpp

if (Param->hasUninstantiatedDefaultArg()) {
     //   template<typename Tp>
     //   // bug: default argument A<T>::FooImpl() is evaluated with 2-level
     //   // template argument list [[T], [Tp]], should be [[Tp]].
     //   friend A<Tp> Foo(int a);
     // };
     //
     // template<typename T>
     // A<T> Foo(int a = A<T>::FooImpl());
-    MultiLevelTemplateArgumentList MutiLevelArgList
+    MultiLevelTemplateArgumentList MultiLevelArgList
       = getTemplateInstantiationArgs(FD, nullptr, /*RelativeToPrimary=*/true);
 
     InstantiatingTemplate Inst(*this, CallLoc, Param,
-                               MutiLevelArgList.getInnermost());
+                               MultiLevelArgList.getInnermost());
     if (Inst.isInvalid())
       return true;
     if (Inst.isAlreadyInstantiating()) {
       Diag(Param->getBeginLoc(), diag::err_recursive_default_argument) << FD;
       Param->setInvalidDecl();
       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);
       LocalInstantiationScope Local(*this);
-      Result = SubstInitializer(UninstExpr, MutiLevelArgList,
+
+      const FunctionDecl *PatternDecl = FD->getTemplateInstantiationPattern();
+      if (!PatternDecl)
+        PatternDecl = FD;
+
+      Local.addInstantiatedParameters(FD, PatternDecl, MultiLevelArgList);
+      Result = SubstInitializer(UninstExpr, MultiLevelArgList,
                                 /*DirectInit*/false);
     }
     if (Result.isInvalid())
       return true;
 
     // Check the expression as an initializer for the parameter.
     InitializedEntity Entity
       = InitializedEntity::InitializeParameter(Context, Param);

clang/lib/Sema/SemaTemplateInstantiate.cpp

for (const LocalInstantiationScope *Current = this; Current;
     }
 
     if (!Current->CombineWithOuterScope)
       break;
   }
 
   return nullptr;
 }
+
+bool LocalInstantiationScope::addInstantiatedParameters(
+    FunctionDecl *Function, const FunctionDecl *PatternDecl,
+    const MultiLevelTemplateArgumentList &TemplateArgs) {
+  unsigned FParamIdx = 0;
+  for (unsigned I = 0, N = PatternDecl->getNumParams(); I != N; ++I) {
+    const ParmVarDecl *PatternParam = PatternDecl->getParamDecl(I);
+    if (!PatternParam->isParameterPack()) {
+      // Simple case: not a parameter pack.
+      assert(FParamIdx < Function->getNumParams());
+      ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx);
+      FunctionParam->setDeclName(PatternParam->getDeclName());
+      // If the parameter's type is not dependent, update it to match the type
+      // in the pattern. They can differ in top-level cv-qualifiers, and we want
+      // the pattern's type here. If the type is dependent, they can't differ,
+      // per core issue 1668. Substitute into the type from the pattern, in case
+      // it's instantiation-dependent.
+      // FIXME: Updating the type to work around this is at best fragile.
+      if (!PatternDecl->getType()->isDependentType()) {
+        QualType T = SemaRef.SubstType(PatternParam->getType(), TemplateArgs,
+                                       FunctionParam->getLocation(),
+                                       FunctionParam->getDeclName());
+        if (T.isNull())
+          return true;
+        FunctionParam->setType(T);
+      }
+
+      InstantiatedLocal(PatternParam, FunctionParam);
+      ++FParamIdx;
+      continue;
+    }
+
+    // Expand the parameter pack.
+    MakeInstantiatedLocalArgPack(PatternParam);
+    Optional<unsigned> NumArgumentsInExpansion =
+        SemaRef.getNumArgumentsInExpansion(PatternParam->getType(),
+                                           TemplateArgs);
+    if (NumArgumentsInExpansion) {
+      QualType PatternType =
+          PatternParam->getType()->castAs<PackExpansionType>()->getPattern();
+      for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) {
+        ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx);
+        FunctionParam->setDeclName(PatternParam->getDeclName());
+        if (!PatternDecl->getType()->isDependentType()) {
+          Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(SemaRef, Arg);
+          QualType T = SemaRef.SubstType(PatternType, TemplateArgs,
+                                   FunctionParam->getLocation(),
+                                   FunctionParam->getDeclName());
+          if (T.isNull())
+            return true;
+          FunctionParam->setType(T);
+        }
+
+        InstantiatedLocalPackArg(PatternParam, FunctionParam);
+        ++FParamIdx;
+      }
+    }
+  }
+
+  return false;
+}

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

if (SemaRef.SubstParmTypes(D->getLocation(), D->parameters(), nullptr,
                                TemplateArgs, ParamTypes, &Params,
                                ExtParamInfos))
       return nullptr;
   }
 
   return NewTInfo;
 }
 
-/// 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) {
-  unsigned FParamIdx = 0;
-  for (unsigned I = 0, N = PatternDecl->getNumParams(); I != N; ++I) {
-    const ParmVarDecl *PatternParam = PatternDecl->getParamDecl(I);
-    if (!PatternParam->isParameterPack()) {
-      // Simple case: not a parameter pack.
-      assert(FParamIdx < Function->getNumParams());
-      ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx);
-      FunctionParam->setDeclName(PatternParam->getDeclName());
-      // If the parameter's type is not dependent, update it to match the type
-      // in the pattern. They can differ in top-level cv-qualifiers, and we want
-      // the pattern's type here. If the type is dependent, they can't differ,
-      // per core issue 1668. Substitute into the type from the pattern, in case
-      // 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,
-                                 FunctionParam->getLocation(),
-                                 FunctionParam->getDeclName());
-        if (T.isNull())
-          return true;
-        FunctionParam->setType(T);
-      }
-
-      Scope.InstantiatedLocal(PatternParam, FunctionParam);
-      ++FParamIdx;
-      continue;
-    }
-
-    // Expand the parameter pack.
-    Scope.MakeInstantiatedLocalArgPack(PatternParam);
-    Optional<unsigned> NumArgumentsInExpansion
-      = S.getNumArgumentsInExpansion(PatternParam->getType(), TemplateArgs);
-    if (NumArgumentsInExpansion) {
-      QualType PatternType =
-          PatternParam->getType()->castAs<PackExpansionType>()->getPattern();
-      for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) {
-        ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx);
-        FunctionParam->setDeclName(PatternParam->getDeclName());
-        if (!PatternDecl->getType()->isDependentType()) {
-          Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(S, Arg);
-          QualType T = S.SubstType(PatternType, TemplateArgs,
-                                   FunctionParam->getLocation(),
-                                   FunctionParam->getDeclName());
-          if (T.isNull())
-            return true;
-          FunctionParam->setType(T);
-        }
-
-        Scope.InstantiatedLocalPackArg(PatternParam, FunctionParam);
-        ++FParamIdx;
-      }
-    }
-  }
-
-  return false;
-}
-
 void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
                                     FunctionDecl *Decl) {
   const FunctionProtoType *Proto = Decl->getType()->castAs<FunctionProtoType>();
   if (Proto->getExceptionSpecType() != EST_Uninstantiated)
     return;
 
   InstantiatingTemplate Inst(*this, PointOfInstantiation, Decl,
                              InstantiatingTemplate::ExceptionSpecification());
void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
   // PushDeclContext because we don't have a scope.
   Sema::ContextRAII savedContext(*this, Decl);
   LocalInstantiationScope Scope(*this);
 
   MultiLevelTemplateArgumentList TemplateArgs =
     getTemplateInstantiationArgs(Decl, nullptr, /*RelativeToPrimary*/true);
 
   FunctionDecl *Template = Proto->getExceptionSpecTemplate();
-  if (addInstantiatedParametersToScope(*this, Decl, Template, Scope,
-                                       TemplateArgs)) {
+  if (Scope.addInstantiatedParameters(Decl, Template, TemplateArgs)) {
     UpdateExceptionSpec(Decl, EST_None);
     return;
   }
 
   SubstExceptionSpec(Decl, Template->getType()->castAs<FunctionProtoType>(),
                      TemplateArgs);
 }
 
else {
     SubstQualifier(*this, PatternDecl, Function, TemplateArgs);
 
     ActOnStartOfFunctionDef(nullptr, Function);
 
     // Enter the scope of this instantiation. We don't use
     // PushDeclContext because we don't have a scope.
     Sema::ContextRAII savedContext(*this, Function);
 
-    if (addInstantiatedParametersToScope(*this, Function, PatternDecl, Scope,
-                                         TemplateArgs))
+    if (Scope.addInstantiatedParameters(Function, PatternDecl, TemplateArgs))
       return;
 
     StmtResult Body;
     if (PatternDecl->hasSkippedBody()) {
       ActOnSkippedFunctionBody(Function);
       Body = nullptr;
     } else {
       if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(Function)) {

clang/test/SemaTemplate/default-arguments-cxx0x.cpp

namespace rdar34167492 {
   };
 
   class A {
     A() noexcept = default;
     A(const A&) noexcept = default;
     S<int> _a{};
   };
 }
+
+// Failure to resolve the decltype part during instantiation caused an
+// assertion failure
+namespace PR28500 {
+namespace original {
+template <class T>
+void bar(T t = [](decltype(t) i) { return 0; }(0)) {}
+void foo() {
+  bar<int>();
+}
+} // namespace original
+
+namespace cast {
+template <class T>
+void bar(T t = decltype(t)(0)) {}
+void foo() {
+  bar<int>();
+  bar<double>();
+}
+} // namespace cast
+
+namespace value {
+template <class T>
+void bar(T t = decltype(t)()) {}
+void foo() {
+  bar<int>();
+  bar<double>();
+}
+} // namespace value
+} // namespace PR28500