PR38627: Fix handling of exception specification adjustment for

destructors.

We previously tried to patch up the exception specification after
completing the class, which went wrong when the exception specification
was needed within the class body (in particular, by a friend
redeclaration of the destructor in a nested class). We now mark the
destructor as having a not-yet-computed exception specification
immediately after creating it.

This requires delaying various checks against the exception
specification (where we'd previously have just got the wrong exception
specification, and now find we have an exception specification that we
can't compute yet) when those checks fire while the class is being
defined.

This also exposed an issue that we were missing a CodeSynthesisContext
for computation of exception specifications (otherwise we'd fail to make
the module containing the definition of the class visible when computing
its members' exception specs). Adding that incidentally also gives us a
diagnostic quality improvement.

This has also exposed an pre-existing problem: making the exception
specification evaluation context a non-SFINAE context (as it should be)
results in a bootstrap failure; PR38850 filed for this.

llvm-svn: 341499

6 files changed, 102 insertions, 45 deletions

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index bd2637a72ef..82a04e1070d 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -922,10 +922,9 @@ void Sema::ActOnEndOfTranslationUnit() {
 
   // All delayed member exception specs should be checked or we end up accepting
   // incompatible declarations.
-  // FIXME: This is wrong for TUKind == TU_Prefix. In that case, we need to
-  // write out the lists to the AST file (if any).
+  assert(DelayedOverridingExceptionSpecChecks.empty());
+  assert(DelayedEquivalentExceptionSpecChecks.empty());
   assert(DelayedDefaultedMemberExceptionSpecs.empty());
-  assert(DelayedExceptionSpecChecks.empty());
 
   // All dllexport classes should have been processed already.
   assert(DelayedDllExportClasses.empty());
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e62728c124c..c705b4920e6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7956,14 +7956,11 @@ static FunctionDecl* CreateNewFunctionDecl(Sema &SemaRef, Declarator &D,
                                     NameInfo, R, TInfo, isInline,
                                     /*isImplicitlyDeclared=*/false);
 
-      // If the class is complete, then we now create the implicit exception
-      // specification. If the class is incomplete or dependent, we can't do
-      // it yet.
-      if (SemaRef.getLangOpts().CPlusPlus11 && !Record->isDependentType() &&
-          Record->getDefinition() && !Record->isBeingDefined() &&
-          R->getAs<FunctionProtoType>()->getExceptionSpecType() == EST_None) {
-        SemaRef.AdjustDestructorExceptionSpec(Record, NewDD);
-      }
+      // If the destructor needs an implicit exception specification, set it
+      // now. FIXME: It'd be nice to be able to create the right type to start
+      // with, but the type needs to reference the destructor declaration.
+      if (SemaRef.getLangOpts().CPlusPlus11)
+        SemaRef.AdjustDestructorExceptionSpec(NewDD);
 
       IsVirtualOkay = true;
       return NewDD;
@@ -15810,13 +15807,6 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
       }
 
       if (!CXXRecord->isDependentType()) {
-        if (CXXRecord->hasUserDeclaredDestructor()) {
-          // Adjust user-defined destructor exception spec.
-          if (getLangOpts().CPlusPlus11)
-            AdjustDestructorExceptionSpec(CXXRecord,
-                                          CXXRecord->getDestructor());
-        }
-
         // Add any implicitly-declared members to this class.
         AddImplicitlyDeclaredMembersToClass(CXXRecord);
 
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 61a7730d9b0..d3f7af25295 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6635,20 +6635,27 @@ void Sema::CheckExplicitlyDefaultedMemberExceptionSpec(
 }
 
 void Sema::CheckDelayedMemberExceptionSpecs() {
-  decltype(DelayedExceptionSpecChecks) Checks;
-  decltype(DelayedDefaultedMemberExceptionSpecs) Specs;
+  decltype(DelayedOverridingExceptionSpecChecks) Overriding;
+  decltype(DelayedEquivalentExceptionSpecChecks) Equivalent;
+  decltype(DelayedDefaultedMemberExceptionSpecs) Defaulted;
 
-  std::swap(Checks, DelayedExceptionSpecChecks);
-  std::swap(Specs, DelayedDefaultedMemberExceptionSpecs);
+  std::swap(Overriding, DelayedOverridingExceptionSpecChecks);
+  std::swap(Equivalent, DelayedEquivalentExceptionSpecChecks);
+  std::swap(Defaulted, DelayedDefaultedMemberExceptionSpecs);
 
   // Perform any deferred checking of exception specifications for virtual
   // destructors.
-  for (auto &Check : Checks)
+  for (auto &Check : Overriding)
     CheckOverridingFunctionExceptionSpec(Check.first, Check.second);
 
+  // Perform any deferred checking of exception specifications for befriended
+  // special members.
+  for (auto &Check : Equivalent)
+    CheckEquivalentExceptionSpec(Check.second, Check.first);
+
   // Check that any explicitly-defaulted methods have exception specifications
   // compatible with their implicit exception specifications.
-  for (auto &Spec : Specs)
+  for (auto &Spec : Defaulted)
     CheckExplicitlyDefaultedMemberExceptionSpec(Spec.first, Spec.second);
 }
 
@@ -10685,19 +10692,48 @@ void SpecialMemberExceptionSpecInfo::visitSubobjectCall(
     ExceptSpec.CalledDecl(getSubobjectLoc(Subobj), MD);
 }
 
+namespace {
+/// RAII object to register a special member as being currently declared.
+struct ComputingExceptionSpec {
+  Sema &S;
+
+  ComputingExceptionSpec(Sema &S, CXXMethodDecl *MD, SourceLocation Loc)
+      : S(S) {
+    Sema::CodeSynthesisContext Ctx;
+    Ctx.Kind = Sema::CodeSynthesisContext::ExceptionSpecEvaluation;
+    Ctx.PointOfInstantiation = Loc;
+    Ctx.Entity = MD;
+    S.pushCodeSynthesisContext(Ctx);
+  }
+  ~ComputingExceptionSpec() {
+    S.popCodeSynthesisContext();
+  }
+};
+}
+
 static Sema::ImplicitExceptionSpecification
 ComputeDefaultedSpecialMemberExceptionSpec(
     Sema &S, SourceLocation Loc, CXXMethodDecl *MD, Sema::CXXSpecialMember CSM,
     Sema::InheritedConstructorInfo *ICI) {
+  ComputingExceptionSpec CES(S, MD, Loc);
+
   CXXRecordDecl *ClassDecl = MD->getParent();
 
   // C++ [except.spec]p14:
   //   An implicitly declared special member function (Clause 12) shall have an
   //   exception-specification. [...]
-  SpecialMemberExceptionSpecInfo Info(S, MD, CSM, ICI, Loc);
+  SpecialMemberExceptionSpecInfo Info(S, MD, CSM, ICI, MD->getLocation());
   if (ClassDecl->isInvalidDecl())
     return Info.ExceptSpec;
 
+  // FIXME: If this diagnostic fires, we're probably missing a check for
+  // attempting to resolve an exception specification before it's known
+  // at a higher level.
+  if (S.RequireCompleteType(MD->getLocation(),
+                            S.Context.getRecordType(ClassDecl),
+                            diag::err_exception_spec_incomplete_type))
+    return Info.ExceptSpec;
+
   // C++1z [except.spec]p7:
   //   [Look for exceptions thrown by] a constructor selected [...] to
   //   initialize a potentially constructed subobject,
@@ -11172,8 +11208,9 @@ void Sema::ActOnFinishCXXMemberDecls() {
   // If the context is an invalid C++ class, just suppress these checks.
   if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(CurContext)) {
     if (Record->isInvalidDecl()) {
+      DelayedOverridingExceptionSpecChecks.clear();
+      DelayedEquivalentExceptionSpecChecks.clear();
       DelayedDefaultedMemberExceptionSpecs.clear();
-      DelayedExceptionSpecChecks.clear();
       return;
     }
     checkForMultipleExportedDefaultConstructors(*this, Record);
@@ -11195,11 +11232,13 @@ void Sema::referenceDLLExportedClassMethods() {
   }
 }
 
-void Sema::AdjustDestructorExceptionSpec(CXXRecordDecl *ClassDecl,
-                                         CXXDestructorDecl *Destructor) {
+void Sema::AdjustDestructorExceptionSpec(CXXDestructorDecl *Destructor) {
   assert(getLangOpts().CPlusPlus11 &&
          "adjusting dtor exception specs was introduced in c++11");
 
+  if (Destructor->isDependentContext())
+    return;
+
   // C++11 [class.dtor]p3:
   //   A declaration of a destructor that does not have an exception-
   //   specification is implicitly considered to have the same exception-
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index ec9dab8f53a..f3e1563c3a8 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -230,6 +230,16 @@ Sema::UpdateExceptionSpec(FunctionDecl *FD,
     Context.adjustExceptionSpec(Redecl, ESI);
 }
 
+static bool exceptionSpecNotKnownYet(const FunctionDecl *FD) {
+  auto *MD = dyn_cast<CXXMethodDecl>(FD);
+  if (!MD)
+    return false;
+
+  auto EST = MD->getType()->castAs<FunctionProtoType>()->getExceptionSpecType();
+  return EST == EST_Unparsed ||
+         (EST == EST_Unevaluated && MD->getParent()->isBeingDefined());
+};
+
 static bool CheckEquivalentExceptionSpecImpl(
     Sema &S, const PartialDiagnostic &DiagID, const PartialDiagnostic &NoteID,
     const FunctionProtoType *Old, SourceLocation OldLoc,
@@ -278,6 +288,14 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
     ReturnValueOnError = false;
   }
 
+  // If we're befriending a member function of a class that's currently being
+  // defined, we might not be able to work out its exception specification yet.
+  // If not, defer the check until later.
+  if (exceptionSpecNotKnownYet(Old) || exceptionSpecNotKnownYet(New)) {
+    DelayedEquivalentExceptionSpecChecks.push_back({New, Old});
+    return false;
+  }
+
   // Check the types as written: they must match before any exception
   // specification adjustment is applied.
   if (!CheckEquivalentExceptionSpecImpl(
@@ -904,26 +922,21 @@ bool Sema::CheckOverridingFunctionExceptionSpec(const CXXMethodDecl *New,
   if (New->getType()->castAs<FunctionProtoType>()->getExceptionSpecType() ==
       EST_Unparsed)
     return false;
-  if (getLangOpts().CPlusPlus11 && isa<CXXDestructorDecl>(New)) {
-    // Don't check uninstantiated template destructors at all. We can only
-    // synthesize correct specs after the template is instantiated.
-    if (New->getParent()->isDependentType())
-      return false;
-    if (New->getParent()->isBeingDefined()) {
-      // The destructor might be updated once the definition is finished. So
-      // remember it and check later.
-      DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
-      return false;
-    }
-  }
-  // If the old exception specification hasn't been parsed yet, remember that
-  // we need to perform this check when we get to the end of the outermost
+
+  // Don't check uninstantiated template destructors at all. We can only
+  // synthesize correct specs after the template is instantiated.
+  if (isa<CXXDestructorDecl>(New) && New->getParent()->isDependentType())
+    return false;
+
+  // If the old exception specification hasn't been parsed yet, or the new
+  // exception specification can't be computed yet, remember that we need to
+  // perform this check when we get to the end of the outermost
   // lexically-surrounding class.
-  if (Old->getType()->castAs<FunctionProtoType>()->getExceptionSpecType() ==
-      EST_Unparsed) {
-    DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
+  if (exceptionSpecNotKnownYet(Old) || exceptionSpecNotKnownYet(New)) {
+    DelayedOverridingExceptionSpecChecks.push_back({New, Old});
     return false;
   }
+
   unsigned DiagID = diag::err_override_exception_spec;
   if (getLangOpts().MicrosoftExt)
     DiagID = diag::ext_override_exception_spec;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 9a5879e6757..5666cf04a24 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -199,6 +199,7 @@ bool Sema::CodeSynthesisContext::isInstantiationRecord() const {
   case DefaultTemplateArgumentChecking:
   case DeclaringSpecialMember:
   case DefiningSynthesizedFunction:
+  case ExceptionSpecEvaluation:
     return false;
 
   // This function should never be called when Kind's value is Memoization.
@@ -621,6 +622,12 @@ void Sema::PrintInstantiationStack() {
       break;
     }
 
+    case CodeSynthesisContext::ExceptionSpecEvaluation:
+      Diags.Report(Active->PointOfInstantiation,
+                   diag::note_evaluating_exception_spec_here)
+          << cast<FunctionDecl>(Active->Entity);
+      break;
+
     case CodeSynthesisContext::ExceptionSpecInstantiation:
       Diags.Report(Active->PointOfInstantiation,
                    diag::note_template_exception_spec_instantiation_here)
@@ -695,6 +702,12 @@ Optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const {
       // there is no SFINAE.
       return None;
 
+    case CodeSynthesisContext::ExceptionSpecEvaluation:
+      // FIXME: This should not be treated as a SFINAE context, because
+      // we will cache an incorrect exception specification. However, clang
+      // bootstrap relies this! See PR31692.
+      break;
+
     case CodeSynthesisContext::Memoization:
       break;
     }
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 80980106c4d..3ed9eeed16e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3690,6 +3690,9 @@ TemplateDeclInstantiator::InitMethodInstantiation(CXXMethodDecl *New,
   if (InitFunctionInstantiation(New, Tmpl))
     return true;
 
+  if (isa<CXXDestructorDecl>(New) && SemaRef.getLangOpts().CPlusPlus11)
+    SemaRef.AdjustDestructorExceptionSpec(cast<CXXDestructorDecl>(New));
+
   New->setAccess(Tmpl->getAccess());
   if (Tmpl->isVirtualAsWritten())
     New->setVirtualAsWritten(true);