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

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

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

You have a double-semicolon here.

274

Why are you discounting lambdas here?

[sepavloff]

lib/AST/DeclBase.cpp

273

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

getLexicalParent()?

277–278

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

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.
+  /// In particular, it recognizes non-defining declarations such as:
+  /// \code
+  ///     void foo() { 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());
   }
 

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 {
+  return getLexicalDeclContext()->isFunctionOrMethod();
+}
+
 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()));
 
   return false;
 }
 
 bool Decl::isInStdNamespace() const {
   return getDeclContext()->isStdNamespace();
 }

lib/Sema/SemaDecl.cpp

void Sema::PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext) {
   if (AddToContext)
     CurContext->addDecl(D);
 
   // Out-of-line definitions shouldn't be pushed into scope in C++, unless they
   // are function-local declarations.
   if (getLangOpts().CPlusPlus && D->isOutOfLine() &&
       !D->getDeclContext()->getRedeclContext()->Equals(
         D->getLexicalDeclContext()->getRedeclContext()) &&
-      !D->getLexicalDeclContext()->isFunctionOrMethod())
+      !D->isLexicallyWithinFunctionOrMethod())
     return;
 
   // Template instantiations should also not be pushed into scope.
   if (isa<FunctionDecl>(D) &&
       cast<FunctionDecl>(D)->isFunctionTemplateSpecialization())
     return;
 
   // If this replaces anything in the current scope, 
if (Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
       // If this is a global redeclaration, just forget hereafter
       // about the "builtin-ness" of the function.
       //
       // Doing this for local extern declarations is problematic.  If
       // the builtin declaration remains visible, a second invalid
       // local declaration will produce a hard error; if it doesn't
       // remain visible, a single bogus local redeclaration (which is
       // actually only a warning) could break all the downstream code.
-      if (!New->getLexicalDeclContext()->isFunctionOrMethod())
+      if (!New->isLexicallyWithinFunctionOrMethod())
         New->getIdentifier()->revertBuiltin();
 
       return false;
     }
 
     PrevDiag = diag::note_previous_builtin_declaration;
   }
 
if (Previous.isShadowed())
     return false;
 
   if (S.getLangOpts().CPlusPlus) {
     // C++11 [dcl.array]p3:
     //   If there is a preceding declaration of the entity in the same
     //   scope in which the bound was specified, an omitted array bound
     //   is taken to be the same as in that earlier declaration.
     return NewVD->isPreviousDeclInSameBlockScope() ||
-           (!OldVD->getLexicalDeclContext()->isFunctionOrMethod() &&
-            !NewVD->getLexicalDeclContext()->isFunctionOrMethod());
+           (!OldVD->isLexicallyWithinFunctionOrMethod() &&
+            !NewVD->isLexicallyWithinFunctionOrMethod());
   } else {
     // If the old declaration was function-local, don't merge with its
     // type unless we're in the same function.
-    return !OldVD->getLexicalDeclContext()->isFunctionOrMethod() ||
+    return !OldVD->isLexicallyWithinFunctionOrMethod() ||
            OldVD->getLexicalDeclContext() == NewVD->getLexicalDeclContext();
   }
 }
 
 /// MergeVarDecl - We just parsed a variable 'New' which has the same name
 /// and scope as a previous declaration 'Old'.  Figure out how to resolve this
 /// situation, merging decls or emitting diagnostics as appropriate.
 ///
static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
   if (FD->isDeleted())
     return false;
 
   bool MissingPrototype = true;
   for (const FunctionDecl *Prev = FD->getPreviousDecl();
        Prev; Prev = Prev->getPreviousDecl()) {
     // Ignore any declarations that occur in function or method
     // scope, because they aren't visible from the header.
-    if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
+    if (Prev->isLexicallyWithinFunctionOrMethod())
       continue;
 
     MissingPrototype = !Prev->getType()->isFunctionProtoType();
     if (FD->getNumParams() == 0)
       PossibleZeroParamPrototype = Prev;
     break;
   }
 

lib/Sema/SemaExpr.cpp

for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
     // C++0x [basic.lookup.argdep]p3:
     //     -- a block-scope function declaration that is not a
     //        using-declaration
     // NOTE: we also trigger this for function templates (in fact, we
     // don't check the decl type at all, since all other decl types
     // turn off ADL anyway).
     if (isa<UsingShadowDecl>(D))
       D = cast<UsingShadowDecl>(D)->getTargetDecl();
-    else if (D->getLexicalDeclContext()->isFunctionOrMethod())
+    else if (D->isLexicallyWithinFunctionOrMethod())
       return false;
 
     // C++0x [basic.lookup.argdep]p3:
     //     -- a declaration that is neither a function or a function
     //        template
     // And also for builtin functions.
     if (isa<FunctionDecl>(D)) {
       FunctionDecl *FDecl = cast<FunctionDecl>(D);

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->isLexicallyWithinFunctionOrMethod()) {
+      // 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->isLexicallyWithinFunctionOrMethod()) {
+      // 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}}
+}
+