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 ↗

(On Diff #29704)

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

[sepavloff]

lib/Sema/SemaTemplateInstantiate.cpp

1695 ↗

(On Diff #29704)

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.

cfe/trunk/include/clang/AST/DeclBase.h

public:
   /// isDefinedOutsideFunctionOrMethod - This predicate returns true if this
   /// scoped decl is defined outside the current function or method.  This is
   /// roughly global variables and functions, but also handles enums (which
   /// could be defined inside or outside a function etc).
   bool isDefinedOutsideFunctionOrMethod() const {
     return getParentFunctionOrMethod() == nullptr;
   }
 
+  /// \brief Returns true if this declaration lexically is inside a function.
+  /// It recognizes non-defining declarations as well as members of local
+  /// classes:
+  /// \code
+  ///     void foo() { void bar(); }
+  ///     void foo2() { class ABC { void bar(); }; }
+  /// \endcode
+  bool isLexicallyWithinFunctionOrMethod() const;
+
   /// \brief If this decl is defined inside a function/method/block it returns
   /// the corresponding DeclContext, otherwise it returns null.
   const DeclContext *getParentFunctionOrMethod() const;
   DeclContext *getParentFunctionOrMethod() {
     return const_cast<DeclContext*>(
                     const_cast<const Decl*>(this)->getParentFunctionOrMethod());
   }
 

cfe/trunk/lib/AST/DeclBase.cpp

void Decl::setDeclContextsImpl(DeclContext *SemaDC, DeclContext *LexicalDC,
   } else {
     Decl::MultipleDC *MDC = new (Ctx) Decl::MultipleDC();
     MDC->SemanticDC = SemaDC;
     MDC->LexicalDC = LexicalDC;
     DeclCtx = MDC;
   }
 }
 
+bool Decl::isLexicallyWithinFunctionOrMethod() const {
+  const DeclContext *LDC = getLexicalDeclContext();
+  do {
+    if (LDC->isFunctionOrMethod())
+      return true;
+    if (!isa<TagDecl>(LDC))
+      return false;
+    LDC = LDC->getParent();
+  } while (LDC);
+  return false;
+}
+
 bool Decl::isInAnonymousNamespace() const {
   const DeclContext *DC = getDeclContext();
   do {
     if (const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
       if (ND->isAnonymousNamespace())
         return true;
   } while ((DC = DC->getParent()));
 

cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp

ParmVarDecl *Sema::SubstParmVarDecl(ParmVarDecl *OldParm,
   if (OldParm->hasUninstantiatedDefaultArg()) {
     Expr *Arg = OldParm->getUninstantiatedDefaultArg();
     NewParm->setUninstantiatedDefaultArg(Arg);
   } else if (OldParm->hasUnparsedDefaultArg()) {
     NewParm->setUnparsedDefaultArg();
     UnparsedDefaultArgInstantiations[OldParm].push_back(NewParm);
   } else if (Expr *Arg = OldParm->getDefaultArg()) {
     FunctionDecl *OwningFunc = cast<FunctionDecl>(OldParm->getDeclContext());
-    CXXRecordDecl *ClassD = dyn_cast<CXXRecordDecl>(OwningFunc->getDeclContext());
-    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);
+    if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
+      // Instantiate default arguments for methods of local classes (DR1484)
+      // and non-defining declarations.
+      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.

cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New,
 
   if (Proto->hasExceptionSpec() || Proto->getNoReturnAttr()) {
     FunctionProtoType::ExtProtoInfo EPI = Proto->getExtProtoInfo();
 
     // 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;
-    }
     if (SemaRef.getLangOpts().CPlusPlus11 &&
         EPI.ExceptionSpec.Type != EST_None &&
         EPI.ExceptionSpec.Type != EST_DynamicNone &&
         EPI.ExceptionSpec.Type != EST_BasicNoexcept &&
-        !RequireInstantiation) {
+        !Tmpl->isLexicallyWithinFunctionOrMethod()) {
       FunctionDecl *ExceptionSpecTemplate = Tmpl;
       if (EPI.ExceptionSpec.Type == EST_Uninstantiated)
         ExceptionSpecTemplate = EPI.ExceptionSpec.SourceTemplate;
       ExceptionSpecificationType NewEST = EST_Uninstantiated;
       if (EPI.ExceptionSpec.Type == EST_Unevaluated)
         NewEST = EST_Unevaluated;
 
       // Mark the function has having an uninstantiated exception specification.

cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp

 
 struct NonPOD {
   NonPOD();
   NonPOD(const NonPOD&);
   ~NonPOD();
 };
 
 struct NoDefaultCtor {
-  NoDefaultCtor(const NoDefaultCtor&); // expected-note{{candidate constructor}}
+  NoDefaultCtor(const NoDefaultCtor&); // expected-note{{candidate constructor}} \
+                                       // expected-note{{candidate constructor not viable: requires 1 argument, but 0 were provided}}
   ~NoDefaultCtor();
 };
 
 template<typename T>
 void defargs_in_template_unused(T t) {
-  auto l1 = [](const T& value = T()) { };
+  auto l1 = [](const T& value = T()) { };  // expected-error{{no matching constructor for initialization of 'NoDefaultCtor'}}
   l1(t);
 }
 
 template void defargs_in_template_unused(NonPOD);
-template void defargs_in_template_unused(NoDefaultCtor);
+template void defargs_in_template_unused(NoDefaultCtor);  // expected-note{{in instantiation of function template specialization 'defargs_in_template_unused<NoDefaultCtor>' requested here}}
 
 template<typename T>
 void defargs_in_template_used() {
-  auto l1 = [](const T& value = T()) { }; // expected-error{{no matching constructor for initialization of 'NoDefaultCtor'}}
-  l1(); // expected-note{{in instantiation of default function argument expression for 'operator()<NoDefaultCtor>' required here}}
+  auto l1 = [](const T& value = T()) { }; // expected-error{{no matching constructor for initialization of 'NoDefaultCtor'}} \
+                                          // expected-note{{candidate function not viable: requires single argument 'value', but no arguments were provided}} \
+                                          // expected-note{{conversion candidate of type 'void (*)(const NoDefaultCtor &)'}}
+  l1(); // expected-error{{no matching function for call to object of type '(lambda at }}
 }
 
 template void defargs_in_template_used<NonPOD>();
 template void defargs_in_template_used<NoDefaultCtor>(); // expected-note{{in instantiation of function template specialization}}
 

cfe/trunk/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}}
+}

cfe/trunk/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}}
+}
+