Instantiate function declarations in instantiated functions.

[sepavloff]

Summary

If a function declaration is found inside a template function as in:

template<class T> void f() {
  void g(int x = T::v) except(T::w);
}

it must be instantiated along with the enclosing template function,
including default arguments and exception specification.

[rsmith]

Please add a member to Decl, something like isLexicallyWithinFunctionOrMethod, and use it in both places rather than duplicating the local class / local function declaration check.

lib/Sema/SemaTemplateInstantiate.cpp

1695

Is this necessary? A local function declaration can't be a definition.

[sepavloff]

lib/Sema/SemaTemplateInstantiate.cpp

1695

As clang do not support nested functions, it is not necessary. Remove it.

[sepavloff]

Address reviewer's comments.

[sepavloff]

Ping.

Thanks,
--Serge

[rsmith]

I think maybe you missed my comment from before: "Please add a member to Decl, something like isLexicallyWithinFunctionOrMethod, and use it in both places rather than duplicating the local class / local function declaration check."

[sepavloff]

Added method isLexicallyWithinFunctionOrMethod to class Decl.

Sorry, missed it from previous commit.

[rsmith]

lib/AST/DeclBase.cpp

269–271 ↗

(On Diff #31251)

This should also return true if the lexical decl context is a local class. The point is to allow you to remove the code duplication you added in the other changes in this patch.

[sepavloff]

Updated method isLexicallyWithinFunctionOrMethod according to Richard's notes.

[rsmith]

lib/AST/DeclBase.cpp

273 ↗

(On Diff #32310)

It's not necessary for this change, but to match its documentation this function should handle other kinds of TagDecl too (enums, C structs). Something like:

do {
  if (LDC->isFunctionOrMethod())
    return true;
  if (!isa<TagDecl>(LDC))
    return false;
  LDC = LDC->getLexicalParent();
} while (LDC);
return false;

... maybe?

274 ↗

(On Diff #32310)

You have a double-semicolon here.

274 ↗

(On Diff #32310)

Why are you discounting lambdas here?

[sepavloff]

lib/AST/DeclBase.cpp

273 ↗

(On Diff #32310)

The proposed code recognizes lambda as lexically contained within a function. As a result, the following test starts to fail (obtained from CXX\expr\expr.prim\expr.prim.lambda\default-arguments.cpp):

struct NoDefaultCtor {
  NoDefaultCtor(const NoDefaultCtor&); // expected-note{{candidate constructor}}
  ~NoDefaultCtor();
};

template<typename T>
void defargs_in_template_unused(T t) {
  auto l1 = [](const T& value = T()) { };
  l1(t);
}

template void defargs_in_template_unused(NoDefaultCtor);

because default value for lambda argument cannot be instantiated. It is not clear whether instantiation of the lambda default argument must always occur similar to DR1484. I couldn't find appropriate place in the standard. According to spirit it shouldn't as lambda is not a separate declaration but a part of instantiated content. If so 14.6.4.1p2 is more likely to be applied and the above test must pass.

Maybe we need to rename isLexicallyWithinFunctionOrMethod to shouldBeAlwaysInstantiated or something like that to avoid misunderstanding?

[sepavloff]

Lambda functions are now treated as local classes.

[rsmith]

LGTM with a couple of tweaks. Thanks!

lib/AST/DeclBase.cpp

276 ↗

(On Diff #32597)

getLexicalParent()?

277–278 ↗

(On Diff #32597)

I think we can actually recast this loop as while (true), since we must eventually reach the TU and return false.

lib/Sema/SemaTemplateInstantiate.cpp

if (OldParm->hasUninstantiatedDefaultArg()) {
     if (ClassD && ClassD->isLocalClass() && !ClassD->isLambda()) {
       // If this is a method of a local class, as per DR1484 its default
       // arguments must be instantiated.
       Sema::ContextRAII SavedContext(*this, ClassD);
       LocalInstantiationScope Local(*this);
       ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
       if (NewArg.isUsable())
         NewParm->setDefaultArg(NewArg.get());
+    } else if (OwningFunc->getLexicalDeclContext()->isFunctionOrMethod() &&
+               !OwningFunc->isThisDeclarationADefinition()) {
+      // This is a function declaration within a function definition, as in:
+      //     template<class T> void f() {
+      //       void g(int x = T::v);
+      //     }
+      Sema::ContextRAII SavedContext(*this, OwningFunc);
+      LocalInstantiationScope Local(*this);
+      ExprResult NewArg = SubstExpr(Arg, TemplateArgs);
+      if (NewArg.isUsable())
+        NewParm->setDefaultArg(NewArg.get());
     } else {
       // FIXME: if we non-lazily instantiated non-dependent default args for
       // non-dependent parameter types we could remove a bunch of duplicate
       // conversion warnings for such arguments.
       NewParm->setUninstantiatedDefaultArg(Arg);
     }
   }
 

lib/Sema/SemaTemplateInstantiateDecl.cpp

if (Proto->hasExceptionSpec() || Proto->getNoReturnAttr()) {
     // DR1330: In C++11, defer instantiation of a non-trivial
     // exception specification.
     // DR1484: Local classes and their members are instantiated along with the
     // containing function.
     bool RequireInstantiation = false;
     if (CXXRecordDecl *Cls = dyn_cast<CXXRecordDecl>(Tmpl->getDeclContext())) {
       if (Cls->isLocalClass())
         RequireInstantiation = true;
+    } else if (Tmpl->getLexicalDeclContext()->isFunctionOrMethod() &&
+               !Tmpl->isThisDeclarationADefinition()) {
+      // This is a non-defining declaration of a file scope function.
+      RequireInstantiation = true;
     }
     if (SemaRef.getLangOpts().CPlusPlus11 &&
         EPI.ExceptionSpec.Type != EST_None &&
         EPI.ExceptionSpec.Type != EST_DynamicNone &&
         EPI.ExceptionSpec.Type != EST_BasicNoexcept &&
         !RequireInstantiation) {
       FunctionDecl *ExceptionSpecTemplate = Tmpl;
       if (EPI.ExceptionSpec.Type == EST_Uninstantiated)

test/SemaTemplate/default-arguments.cpp

namespace DR1635 {
   template <class T> struct X {
     template <class U = typename T::type> static void f(int) {} // expected-error {{type 'int' cannot be used prior to '::' because it has no members}} \
                                                                 // expected-warning {{C++11}}
     static void f(...) {}
   };
 
   int g() { X<int>::f(0); } // expected-note {{in instantiation of template class 'DR1635::X<int>' requested here}}
 }
+
+namespace NondefDecls {
+  template<typename T> void f1() {
+    int g1(int defarg = T::error);  // expected-error{{type 'int' cannot be used prior to '::' because it has no members}}
+  }
+  template void f1<int>();  // expected-note{{in instantiation of function template specialization 'NondefDecls::f1<int>' requested here}}
+}

test/SemaTemplate/instantiate-exception-spec-cxx11.cpp

void z() {
     f<int, char, double>(nullptr, nullptr, nullptr);
     g<true, false, true>(nullptr, nullptr, nullptr);
     i<int, long, short>();
     j<true, true>();
     j<true, false>(); // expected-note {{in instantiation of}}
   }
 
 }
+
+namespace NondefDecls {
+  template<typename T> void f1() {
+    int g1(int) noexcept(T::error); // expected-error{{type 'int' cannot be used prior to '::' because it has no members}}
+  }
+  template void f1<int>(); // expected-note{{in instantiation of function template specialization 'NondefDecls::f1<int>' requested here}}
+}
+