sepavloff updated this revision to Diff 109928.
sepavloff added a comment.

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.


https://reviews.llvm.org/D36353

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaTemplate/constexpr-instantiate.cpp
  test/SemaTemplate/instantiate-constexpr-function.cpp

Index: test/SemaTemplate/instantiate-constexpr-function.cpp
===================================================================
--- /dev/null
+++ test/SemaTemplate/instantiate-constexpr-function.cpp
@@ -0,0 +1,34 @@
+// 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;
Index: test/SemaTemplate/constexpr-instantiate.cpp
===================================================================
--- test/SemaTemplate/constexpr-instantiate.cpp
+++ test/SemaTemplate/constexpr-instantiate.cpp
@@ -259,3 +259,9 @@
   }
   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}}
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13635,11 +13635,6 @@
           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,
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -113,6 +113,20 @@
 } // 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),
@@ -171,6 +185,8 @@
   SemaPPCallbackHandler = Callbacks.get();
   PP.addPPCallbacks(std::move(Callbacks));
   SemaPPCallbackHandler->set(*this);
+
+  Context.setInstantiator(new SemaTemplateInstantiator(*this));
 }
 
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
@@ -359,6 +375,9 @@
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
 
+  ASTContext::InstantiationHelper *Inst = Context.removeInstantiator();
+  delete Inst;
+
   assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2564,6 +2564,30 @@
   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,
@@ -4715,7 +4739,7 @@
       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,
@@ -6285,7 +6309,7 @@
   }
 
   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;
@@ -6317,7 +6341,7 @@
     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;
@@ -10714,8 +10738,10 @@
     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();
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9596,6 +9596,28 @@
     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
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1883,6 +1883,29 @@
   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.
   //===--------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D36353: I... Serge Pavlov via Phabricator via cfe-commits
    • [PATCH] D363... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D363... Serge Pavlov via Phabricator via cfe-commits
    • [PATCH] D363... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to