Postpone instantiation of constexpr functions

[sepavloff]

Summary

Previous behavior was to instantiate constexpr function in the point where
it is used. It causes crash when compiling the following code:

template<typename T> constexpr T foo(T x) {
  return foo((int)x);
}

The function template specialization foo<int> was instantiated because it
was used in the body of template foo<T> but definition of foo<T> is not
available yet.

With this change a constexpr function template specialization is instantiated
at the end of translation unit unless it is called in a constant expression
outside template definition.

This change also fixes PR33561.

[rsmith]

The current behaviour is correct according to the latest discussions of CWG. We shouldn't be delaying instantiation; we just need to fix the recent regression for functions where we try to instantiate functions that don't have a complete body yet.

[sepavloff]

Updated patch

I missed CWG issue 1581. It make patch description invalid, but the patch
itself can be repaired with small modification.

This fix originated from the attempt to solve the problem described in
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20170731/199590.html.
Compiler crashed on the code:

template <class T>
constexpr void f(T) {
  f(0);
}

When parsing template body clang encounters f(0) and tries to immediately
instantiate f<int>. It is not possible, because body of the function template
f<T> is not available yet. Before r305903 (Function with unparsed body is a
definition) the function template f<T> was considered as undefined and
instantiation request was silently ignored, so no error observed. With r305903
f<T> is reported as defined but instantiation pattern is not available for it,
and the compilation crashes.

Instantiation request cannot be fulfilled during parsing of f<T>, so
the instantiation of f<int> must be postponed. CWG issue 1581 deals with
function calls which can be skipped for some reason. Return value of the call
in this case is ignored, so instantiation of such constexpr function makes
sense only if the body can trigger an error at instantiation, by static_assert,
throw or thing like T:error. If the above is true, constexpr functions does not
need to be instantiated in the point of use, it is sufficient to instantiate it
somewhere later, so that the diagnostics be be present.

According to investigation in https://bugs.llvm.org/show_bug.cgi?id=33561 too
early instantiation is a reason of hang if -fdelayed-template-parsing is used.
At least the reduced test case provided in that bug compiles successfully if
this patch is applied.

The fix was changed a bit so that constexpr functions are added to
PendingInstantiations, the call to isConstexpr is restored in SemaExpr.cpp.
The test from CWG issue 1581 was also added.

[rsmith]

lib/Sema/Sema.cpp

189

It's not correct/reasonable to instantiate whenever an expression happens to be evaluated, in numerous ways:

1. You'll get the "point of instantiation" wrong -- the standard only permits instantiation (as-if) immediately at the point of use or at the end of the translation unit
2. You risk instantiating at some points where no instantiation is permitted at all -- for instance, if SemaChecking decides it wants to try to evaluate some expression to produce a warning, and the code it tries to evaluate happens to be in an unevaluated operand, you are not permitted to instantiate things it references.
3. It risks making the set of instantiations / effective points of instantiation dependent on the warning flags that happen to be in effect, since existing code may or may not choose to evaluate an expression depending on warning flags.

Fundamentally, either constant expression evaluation needs to be side-effect free, or we need to distinguish between places where we happen to evaluate things and places where we evaluate things because the language requires them to be constant expressions. The current direction of CWG1581 is to make the "should we instantiate?" determination based solely on the syntactic context, which means that the constant expression evaluator should never trigger any instantiations.

In order to fix the case where a function/variable template specialization that is usable in a constant expression is defined after the point of use, I think we should check for pending instantiations of that function / variable template when we complete its definition, and perform those instantiations at that time.

include/clang/AST/ASTContext.h

public:
   /// expressions.
   QualType GetBuiltinType(unsigned ID, GetBuiltinTypeError &Error,
                           unsigned *IntegerConstantArgs = nullptr) const;
 
 private:
   CanQualType getFromTargetType(unsigned Type) const;
   TypeInfo getTypeInfoImpl(const Type *T) const;
 
+
+  //===--------------------------------------------------------------------===//
+  //                         Template instantiation.
+  //===--------------------------------------------------------------------===//
+public:
+
+  /// Helper class that provides interface to template instantiation facility.
+  class InstantiationHelper {
+  public:
+    virtual ~InstantiationHelper();
+    virtual bool instantiateFunctionDefinition(
+        SourceLocation PointOfInstantiation,
+        FunctionDecl *Function) = 0;
+  };
+
+  void setInstantiator(InstantiationHelper *Helper);
+  InstantiationHelper *removeInstantiator();
+  bool instantiateFunctionDefinition(SourceLocation PointOfInstantiation,
+                                     FunctionDecl *Function);
+private:
+  InstantiationHelper *Instantiator = nullptr;
+
+
   //===--------------------------------------------------------------------===//
   //                         Type Predicates.
   //===--------------------------------------------------------------------===//
 
 public:
   /// \brief Return one of the GCNone, Weak or Strong Objective-C garbage
   /// collection attributes.
   Qualifiers::GC getObjCGCAttrKind(QualType Ty) const;

lib/AST/ASTContext.cpp

 
 unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
   if (AS >= LangAS::FirstTargetAddressSpace)
     return AS - LangAS::FirstTargetAddressSpace;
   else
     return (*AddrSpaceMap)[AS];
 }
 
+ASTContext::InstantiationHelper::~InstantiationHelper() {
+}
+
+void ASTContext::setInstantiator(ASTContext::InstantiationHelper *Inst) {
+  Instantiator = Inst;
+}
+
+ASTContext::InstantiationHelper *ASTContext::removeInstantiator() {
+  InstantiationHelper *Res = Instantiator;
+  Instantiator = nullptr;
+  return Res;
+}
+
+bool ASTContext::instantiateFunctionDefinition(
+    SourceLocation PointOfInstantiation,
+    FunctionDecl *Function) {
+  if (!Instantiator)
+    return false;
+  return Instantiator->instantiateFunctionDefinition(PointOfInstantiation,
+                                                     Function);
+}
+
 // Explicitly instantiate this in case a Redeclarable<T> is used from a TU that
 // doesn't include ASTContext.h
 template
 clang::LazyGenerationalUpdatePtr<
     const Decl *, Decl *, &ExternalASTSource::CompleteRedeclChain>::ValueType
 clang::LazyGenerationalUpdatePtr<
     const Decl *, Decl *, &ExternalASTSource::CompleteRedeclChain>::makeValue(
         const clang::ASTContext &Ctx, Decl *Value);

lib/AST/ExprConstant.cpp

static bool diagnoseUnreadableFields(EvalInfo &Info, const Expr *E,
   for (auto &BaseSpec : RD->bases())
     if (diagnoseUnreadableFields(Info, E, BaseSpec.getType()))
       return true;
 
   // All mutable fields were empty, and thus not actually read.
   return false;
 }
 
+/// Returns body of the function described by the specified declaration,
+/// instantiating its definition if necessary.
+///
+/// \param[in]  FD The function which body is retrieved.
+/// \param[in]  Loc Location that becomes a point of instantiation.
+/// \param[out] Definition The redeclaration that provides the body.
+///
+/// Behaves like FinctionDecl::getBody, but also can implicitly instantiate
+/// function body.
+///
+static Stmt *getFunctionBody(const FunctionDecl *FD, SourceLocation Loc,
+                             const FunctionDecl *&Definition) {
+  Definition = nullptr;
+  Stmt *Body = FD->getBody(Definition);
+  // Try instantiation function body is possible. Do it only for constexpr
+  // declarations, others are not allowed in constant expressions.
+  if (!Body && FD->isImplicitlyInstantiable() && FD->isConstexpr()) {
+    FD->getASTContext().instantiateFunctionDefinition(Loc,
+        const_cast<FunctionDecl *>(FD));
+    Body = FD->getBody(Definition);
+  }
+  return Body;
+}
+
 /// Kinds of access we can perform on an object, for diagnostics.
 enum AccessKinds {
   AK_Read,
   AK_Assign,
   AK_Increment,
   AK_Decrement
 };
 
bool handleCallExpr(const CallExpr *E, APValue &Result,
 
     // DR1358 allows virtual constexpr functions in some cases. Don't allow
     // calls to such functions in constant expressions.
     if (This && !HasQualifier &&
         isa<CXXMethodDecl>(FD) && cast<CXXMethodDecl>(FD)->isVirtual())
       return Error(E, diag::note_constexpr_virtual_call);
 
     const FunctionDecl *Definition = nullptr;
-    Stmt *Body = FD->getBody(Definition);
+    Stmt *Body = getFunctionBody(FD, Callee->getLocStart(), Definition);
 
     if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body) ||
         !HandleFunctionCall(E->getExprLoc(), Definition, This, Args, Body, Info,
                             Result, ResultSlot))
       return false;
 
     return true;
   }
if (CheckTrivialDefaultConstructor(Info, E->getExprLoc(), FD, ZeroInit)) {
     //     constexpr default constructor, in which case we should start the
     //     lifetimes of all the base subobjects (there can be no data member
     //     subobjects in this case) per [basic.life]p1.
     // Either way, ZeroInitialization is appropriate.
     return ZeroInitialization(E, T);
   }
 
   const FunctionDecl *Definition = nullptr;
-  auto Body = FD->getBody(Definition);
+  Stmt *Body = getFunctionBody(FD, E->getLocStart(), Definition);
 
   if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body))
     return false;
 
   // Avoid materializing a temporary for an elidable copy/move constructor.
   if (E->isElidable() && !ZeroInit)
     if (const MaterializeTemporaryExpr *ME
           = dyn_cast<MaterializeTemporaryExpr>(E->getArg(0)))
if (!Info.CurrentCall) {
     return false;
   }
 
   const CXXConstructorDecl *FD = E->getConstructor();
   if (FD->isInvalidDecl() || FD->getParent()->isInvalidDecl())
     return false;
 
   const FunctionDecl *Definition = nullptr;
-  auto Body = FD->getBody(Definition);
+  Stmt *Body = getFunctionBody(FD, E->getExprLoc(), Definition);
 
   if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body))
     return false;
 
   return HandleConstructorCall(E, This, Info.CurrentCall->Arguments,
                                cast<CXXConstructorDecl>(Definition), Info,
                                Result);
 }
bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
   APValue Scratch;
   if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     // Evaluate the call as a constant initializer, to allow the construction
     // of objects of non-literal types.
     Info.setEvaluatingDecl(This.getLValueBase(), Scratch);
     HandleConstructorCall(&VIE, This, Args, CD, Info, Scratch);
   } else {
     SourceLocation Loc = FD->getLocation();
+    const FunctionDecl *Definition;
+    Stmt *Body = getFunctionBody(FD, Loc, Definition);
     HandleFunctionCall(Loc, FD, (MD && MD->isInstance()) ? &This : nullptr,
-                       Args, FD->getBody(), Info, Scratch, nullptr);
+                       Args, Body, Info, Scratch, nullptr);
   }
 
   return Diags.empty();
 }
 
 bool Expr::isPotentialConstantExprUnevaluated(Expr *E,
                                               const FunctionDecl *FD,
                                               SmallVectorImpl<

lib/Sema/Sema.cpp

default:
       break;
     }
   }
 };
 
 } // end namespace sema
 } // end namespace clang
 
+namespace {
+class SemaTemplateInstantiator : public ASTContext::InstantiationHelper {
+  Sema &S;
+public:
+  ~SemaTemplateInstantiator() {}
+  SemaTemplateInstantiator(Sema &S) : S(S) {}
+  bool instantiateFunctionDefinition(SourceLocation PointOfInstantiation,
+                                     FunctionDecl *Function) override {
+    S.InstantiateFunctionDefinition(PointOfInstantiation, Function);
+    return Function->hasBody();
+  }
+};
+}
+
 Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
            TranslationUnitKind TUKind, CodeCompleteConsumer *CodeCompleter)
     : ExternalSource(nullptr), isMultiplexExternalSource(false),
       FPFeatures(pp.getLangOpts()), LangOpts(pp.getLangOpts()), PP(pp),
       Context(ctxt), Consumer(consumer), Diags(PP.getDiagnostics()),
       SourceMgr(PP.getSourceManager()), CollectStats(false),
       CodeCompleter(CodeCompleter), CurContext(nullptr),
       OriginalLexicalContext(nullptr), MSStructPragmaOn(false),
Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
   // Initilization of data sharing attributes stack for OpenMP
   InitDataSharingAttributesStack();
 
   std::unique_ptr<sema::SemaPPCallbacks> Callbacks =
       llvm::make_unique<sema::SemaPPCallbacks>();
   SemaPPCallbackHandler = Callbacks.get();
   PP.addPPCallbacks(std::move(Callbacks));
   SemaPPCallbackHandler->set(*this);
+
+  Context.setInstantiator(new SemaTemplateInstantiator(*this));
 }
 
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
     PushOnScopeChains(Context.buildImplicitTypedef(T, Name), TUScope);
 }
 
Sema::~Sema() {
 
   // Destroys data sharing attributes stack for OpenMP
   DestroyDataSharingAttributesStack();
 
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
 
+  ASTContext::InstantiationHelper *Inst = Context.removeInstantiator();
+  delete Inst;
+
   assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
 /// context.  If we're still in a system header, and we can plausibly
 /// make the relevant declaration unavailable instead of erroring, do
 /// so and return true.
 bool Sema::makeUnavailableInSystemHeader(SourceLocation loc,

lib/Sema/SemaExpr.cpp

if (Func->isImplicitlyInstantiable()) {
     }
 
     if (!AlreadyInstantiated || Func->isConstexpr()) {
       if (isa<CXXRecordDecl>(Func->getDeclContext()) &&
           cast<CXXRecordDecl>(Func->getDeclContext())->isLocalClass() &&
           CodeSynthesisContexts.size())
         PendingLocalImplicitInstantiations.push_back(
             std::make_pair(Func, PointOfInstantiation));
-      else if (Func->isConstexpr())
-        // Do not defer instantiations of constexpr functions, to avoid the
-        // expression evaluator needing to call back into Sema if it sees a
-        // call to such a function.
-        InstantiateFunctionDefinition(PointOfInstantiation, Func);
       else {
         Func->setInstantiationIsPending(true);
         PendingInstantiations.push_back(std::make_pair(Func,
                                                        PointOfInstantiation));
         // Notify the consumer that a function was implicitly instantiated.
         Consumer.HandleCXXImplicitFunctionInstantiation(Func);
       }
     }

test/SemaTemplate/constexpr-instantiate.cpp

 }
 
 namespace PR20090 {
   template <typename T> constexpr T fact(T n) {
     return n == 0 ? 1 : [=] { return n * fact(n - 1); }();
   }
   static_assert(fact(0) == 1, "");
 }
+
+namespace CWG_1581 {
+  template<int N> struct U {};
+  template<typename T> constexpr int h(T) { return T::error; } // expected-error{{type 'int' cannot be used prior to '::' because it has no members}}
+  int k2 = sizeof(U<0 && h(0)>); // expected-note{{in instantiation of function template specialization 'CWG_1581::h<int>' requested here}}
+}

test/SemaTemplate/instantiate-constexpr-function.cpp

+// RUN: %clang_cc1 -std=c++14 -verify %s
+// expected-no-diagnostics
+
+template<typename T> constexpr void func_01(T) {
+  func_01(0);
+}
+
+
+template<typename T> constexpr unsigned func_02a() {
+  return sizeof(T);
+}
+template<typename T> constexpr T func_02b(T x) {
+  return x + func_02a<int>();
+}
+constexpr long val_02 = func_02b(14L);
+
+
+template<typename T> constexpr T func_03(T) {
+  return T::xyz;
+}
+template<typename T> T func_04(T x) {
+  return x;
+}
+template<> constexpr long func_04(long x) {
+  return 66;
+}
+constexpr long var_04 = func_04(0L);
+static_assert(var_04 == 66, "error");
+
+
+template<typename T> struct C_05 {
+  constexpr T func_05() { return T::xyz; }
+};
+C_05<int> var_05;