[c++20] Compute exception specifications for defaulted comparisons.

zygoloid · zygoloid · commit 4a4e90a82388 · 2019-12-15T22:02:31.000-08:00
This requires us to essentially fully form the body of the defaulted
comparison, but from an unevaluated context. Naively this would require
generating the function definition twice; instead, we ensure that the
function body is implicitly defined before performing the check, and
walk the actual body where possible.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -5232,7 +5232,10 @@ class Sema final {
     void CalledDecl(SourceLocation CallLoc, const CXXMethodDecl *Method);
 
     /// Integrate an invoked expression into the collected data.
-    void CalledExpr(Expr *E);
+    void CalledExpr(Expr *E) { CalledStmt(E); }
+
+    /// Integrate an invoked statement into the collected data.
+    void CalledStmt(Stmt *S);
 
     /// Overwrite an EPI's exception specification with this
     /// computed exception specification.
@@ -5294,7 +5297,7 @@ class Sema final {
 
   /// Evaluate the implicit exception specification for a defaulted
   /// special member function.
-  void EvaluateImplicitExceptionSpec(SourceLocation Loc, CXXMethodDecl *MD);
+  void EvaluateImplicitExceptionSpec(SourceLocation Loc, FunctionDecl *FD);
 
   /// Check the given noexcept-specifier, convert its expression, and compute
   /// the appropriate ExceptionSpecificationType.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -217,8 +217,8 @@ Sema::ImplicitExceptionSpecification::CalledDecl(SourceLocation CallLoc,
       Exceptions.push_back(E);
 }
 
-void Sema::ImplicitExceptionSpecification::CalledExpr(Expr *E) {
-  if (!E || ComputedEST == EST_MSAny)
+void Sema::ImplicitExceptionSpecification::CalledStmt(Stmt *S) {
+  if (!S || ComputedEST == EST_MSAny)
     return;
 
   // FIXME:
@@ -242,7 +242,7 @@ void Sema::ImplicitExceptionSpecification::CalledExpr(Expr *E) {
   // implicit definition. For now, we assume that any non-nothrow expression can
   // throw any exception.
 
-  if (Self->canThrow(E))
+  if (Self->canThrow(S))
     ComputedEST = EST_None;
 }
 
@@ -6814,20 +6814,50 @@ static bool defaultedSpecialMemberIsConstexpr(
   return true;
 }
 
+namespace {
+/// RAII object to register a defaulted function as having its exception
+/// specification computed.
+struct ComputingExceptionSpec {
+  Sema &S;
+
+  ComputingExceptionSpec(Sema &S, FunctionDecl *FD, SourceLocation Loc)
+      : S(S) {
+    Sema::CodeSynthesisContext Ctx;
+    Ctx.Kind = Sema::CodeSynthesisContext::ExceptionSpecEvaluation;
+    Ctx.PointOfInstantiation = Loc;
+    Ctx.Entity = FD;
+    S.pushCodeSynthesisContext(Ctx);
+  }
+  ~ComputingExceptionSpec() {
+    S.popCodeSynthesisContext();
+  }
+};
+}
+
 static Sema::ImplicitExceptionSpecification
 ComputeDefaultedSpecialMemberExceptionSpec(
     Sema &S, SourceLocation Loc, CXXMethodDecl *MD, Sema::CXXSpecialMember CSM,
     Sema::InheritedConstructorInfo *ICI);
 
 static Sema::ImplicitExceptionSpecification
-computeImplicitExceptionSpec(Sema &S, SourceLocation Loc, CXXMethodDecl *MD) {
-  auto CSM = S.getSpecialMember(MD);
-  if (CSM != Sema::CXXInvalid)
-    return ComputeDefaultedSpecialMemberExceptionSpec(S, Loc, MD, CSM, nullptr);
+ComputeDefaultedComparisonExceptionSpec(Sema &S, SourceLocation Loc,
+                                        FunctionDecl *FD,
+                                        Sema::DefaultedComparisonKind DCK);
 
-  auto *CD = cast<CXXConstructorDecl>(MD);
+static Sema::ImplicitExceptionSpecification
+computeImplicitExceptionSpec(Sema &S, SourceLocation Loc, FunctionDecl *FD) {
+  auto DFK = S.getDefaultedFunctionKind(FD);
+  if (DFK.isSpecialMember())
+    return ComputeDefaultedSpecialMemberExceptionSpec(
+        S, Loc, cast<CXXMethodDecl>(FD), DFK.asSpecialMember(), nullptr);
+  if (DFK.isComparison())
+    return ComputeDefaultedComparisonExceptionSpec(S, Loc, FD,
+                                                   DFK.asComparison());
+
+  auto *CD = cast<CXXConstructorDecl>(FD);
   assert(CD->getInheritedConstructor() &&
-         "only special members have implicit exception specs");
+         "only defaulted functions and inherited constructors have implicit "
+         "exception specs");
   Sema::InheritedConstructorInfo ICI(
       S, Loc, CD->getInheritedConstructor().getShadowDecl());
   return ComputeDefaultedSpecialMemberExceptionSpec(
@@ -6849,25 +6879,17 @@ static FunctionProtoType::ExtProtoInfo getImplicitMethodEPI(Sema &S,
   return EPI;
 }
 
-void Sema::EvaluateImplicitExceptionSpec(SourceLocation Loc, CXXMethodDecl *MD) {
-  const FunctionProtoType *FPT = MD->getType()->castAs<FunctionProtoType>();
+void Sema::EvaluateImplicitExceptionSpec(SourceLocation Loc, FunctionDecl *FD) {
+  const FunctionProtoType *FPT = FD->getType()->castAs<FunctionProtoType>();
   if (FPT->getExceptionSpecType() != EST_Unevaluated)
     return;
 
   // Evaluate the exception specification.
-  auto IES = computeImplicitExceptionSpec(*this, Loc, MD);
+  auto IES = computeImplicitExceptionSpec(*this, Loc, FD);
   auto ESI = IES.getExceptionSpec();
 
   // Update the type of the special member to use it.
-  UpdateExceptionSpec(MD, ESI);
-
-  // A user-provided destructor can be defined outside the class. When that
-  // happens, be sure to update the exception specification on both
-  // declarations.
-  const FunctionProtoType *CanonicalFPT =
-    MD->getCanonicalDecl()->getType()->castAs<FunctionProtoType>();
-  if (CanonicalFPT->getExceptionSpecType() == EST_Unevaluated)
-    UpdateExceptionSpec(MD->getCanonicalDecl(), ESI);
+  UpdateExceptionSpec(FD, ESI);
 }
 
 void Sema::CheckExplicitlyDefaultedFunction(Scope *S, FunctionDecl *FD) {
@@ -8092,12 +8114,20 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
   //   declaration, it is implicitly considered to be constexpr.
   // FIXME: Only applying this to the first declaration seems problematic, as
   // simple reorderings can affect the meaning of the program.
-  if (First) {
-    if (!FD->isConstexpr() && Info.Constexpr)
-      FD->setConstexprKind(CSK_constexpr);
-
-    // FIXME: Set up an implicit exception specification, or if given an
-    // explicit one, check that it matches.
+  if (First && !FD->isConstexpr() && Info.Constexpr)
+    FD->setConstexprKind(CSK_constexpr);
+
+  // C++2a [except.spec]p3:
+  //   If a declaration of a function does not have a noexcept-specifier
+  //   [and] is defaulted on its first declaration, [...] the exception
+  //   specification is as specified below
+  if (FD->getExceptionSpecType() == EST_None) {
+    auto *FPT = FD->getType()->castAs<FunctionProtoType>();
+    FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+    EPI.ExceptionSpec.Type = EST_Unevaluated;
+    EPI.ExceptionSpec.SourceDecl = FD;
+    FD->setType(Context.getFunctionType(FPT->getReturnType(),
+                                        FPT->getParamTypes(), EPI));
   }
 
   return false;
@@ -8126,17 +8156,11 @@ void Sema::DefineDefaultedComparison(SourceLocation UseLoc, FunctionDecl *FD,
 
   SynthesizedFunctionScope Scope(*this, FD);
 
-  // The exception specification is needed because we are defining the
-  // function.
-  // FIXME: Handle this better. Computing the exception specification will
-  // eventually need the function body.
-  ResolveExceptionSpec(UseLoc, FD->getType()->castAs<FunctionProtoType>());
-
   // Add a context note for diagnostics produced after this point.
   Scope.addContextNote(UseLoc);
 
-  // Build and set up the function body.
   {
+    // Build and set up the function body.
     CXXRecordDecl *RD = cast<CXXRecordDecl>(FD->getLexicalParent());
     SourceLocation BodyLoc =
         FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation();
@@ -8150,10 +8174,58 @@ void Sema::DefineDefaultedComparison(SourceLocation UseLoc, FunctionDecl *FD,
     FD->markUsed(Context);
   }
 
+  // The exception specification is needed because we are defining the
+  // function. Note that this will reuse the body we just built.
+  ResolveExceptionSpec(UseLoc, FD->getType()->castAs<FunctionProtoType>());
+
   if (ASTMutationListener *L = getASTMutationListener())
     L->CompletedImplicitDefinition(FD);
 }
 
+static Sema::ImplicitExceptionSpecification
+ComputeDefaultedComparisonExceptionSpec(Sema &S, SourceLocation Loc,
+                                        FunctionDecl *FD,
+                                        Sema::DefaultedComparisonKind DCK) {
+  ComputingExceptionSpec CES(S, FD, Loc);
+  Sema::ImplicitExceptionSpecification ExceptSpec(S);
+
+  if (FD->isInvalidDecl())
+    return ExceptSpec;
+
+  // The common case is that we just defined the comparison function. In that
+  // case, just look at whether the body can throw.
+  if (FD->hasBody()) {
+    ExceptSpec.CalledStmt(FD->getBody());
+  } else {
+    // Otherwise, build a body so we can check it. This should ideally only
+    // happen when we're not actually marking the function referenced. (This is
+    // only really important for efficiency: we don't want to build and throw
+    // away bodies for comparison functions more than we strictly need to.)
+
+    // Pretend to synthesize the function body in an unevaluated context.
+    // Note that we can't actually just go ahead and define the function here:
+    // we are not permitted to mark its callees as referenced.
+    Sema::SynthesizedFunctionScope Scope(S, FD);
+    EnterExpressionEvaluationContext Context(
+        S, Sema::ExpressionEvaluationContext::Unevaluated);
+
+    CXXRecordDecl *RD = cast<CXXRecordDecl>(FD->getLexicalParent());
+    SourceLocation BodyLoc =
+        FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation();
+    StmtResult Body =
+        DefaultedComparisonSynthesizer(S, RD, FD, DCK, BodyLoc).build();
+    if (!Body.isInvalid())
+      ExceptSpec.CalledStmt(Body.get());
+
+    // FIXME: Can we hold onto this body and just transform it to potentially
+    // evaluated when we're asked to define the function rather than rebuilding
+    // it? Either that, or we should only build the bits of the body that we
+    // need (the expressions, not the statements).
+  }
+
+  return ExceptSpec;
+}
+
 void Sema::CheckDelayedMemberExceptionSpecs() {
   decltype(DelayedOverridingExceptionSpecChecks) Overriding;
   decltype(DelayedEquivalentExceptionSpecChecks) Equivalent;
@@ -12336,25 +12408,6 @@ 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();
-  }
-};
-}
-
 bool Sema::tryResolveExplicitSpecifier(ExplicitSpecifier &ExplicitSpec) {
   llvm::APSInt Result;
   ExprResult Converted = CheckConvertedConstantExpression(
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -205,7 +205,7 @@ Sema::ResolveExceptionSpec(SourceLocation Loc, const FunctionProtoType *FPT) {
 
   // Compute or instantiate the exception specification now.
   if (SourceFPT->getExceptionSpecType() == EST_Unevaluated)
-    EvaluateImplicitExceptionSpec(Loc, cast<CXXMethodDecl>(SourceDecl));
+    EvaluateImplicitExceptionSpec(Loc, SourceDecl);
   else
     InstantiateExceptionSpec(Loc, SourceDecl);
 
@@ -1221,6 +1221,17 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
     return mergeCanThrow(CT, canSubStmtsThrow(*this, BTE));
   }
 
+  case Expr::PseudoObjectExprClass: {
+    auto *POE = cast<PseudoObjectExpr>(S);
+    CanThrowResult CT = CT_Cannot;
+    for (const Expr *E : POE->semantics()) {
+      CT = mergeCanThrow(CT, canThrow(E));
+      if (CT == CT_Can)
+        break;
+    }
+    return CT;
+  }
+
     // ObjC message sends are like function calls, but never have exception
     // specs.
   case Expr::ObjCMessageExprClass:
@@ -1327,15 +1338,14 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
   case Expr::ObjCAvailabilityCheckExprClass:
   case Expr::OffsetOfExprClass:
   case Expr::PackExpansionExprClass:
-  case Expr::PseudoObjectExprClass:
   case Expr::SubstNonTypeTemplateParmExprClass:
   case Expr::SubstNonTypeTemplateParmPackExprClass:
   case Expr::FunctionParmPackExprClass:
   case Expr::UnaryExprOrTypeTraitExprClass:
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
-    // FIXME: Can any of the above throw?  If so, when?
+    // FIXME: Many of the above can throw.
     return CT_Cannot;
 
   case Expr::AddrLabelExprClass:
diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp
@@ -24,10 +24,10 @@ struct A {
   friend bool operator>=(const A&, const A&) = default;
 };
 struct TestA {
-  friend constexpr bool operator==(const A&, const A&);
-  friend constexpr bool operator!=(const A&, const A&);
+  friend constexpr bool operator==(const A&, const A&) noexcept;
+  friend constexpr bool operator!=(const A&, const A&) noexcept;
 
-  friend constexpr std::strong_ordering operator<=>(const A&, const A&);
+  friend constexpr std::strong_ordering operator<=>(const A&, const A&) noexcept;
   friend constexpr bool operator<(const A&, const A&);
   friend constexpr bool operator<=(const A&, const A&);
   friend constexpr bool operator>(const A&, const A&);
@@ -51,10 +51,10 @@ struct TestReversedA {
   friend constexpr bool operator>(const ReversedA&, const ReversedA&);
   friend constexpr bool operator<=(const ReversedA&, const ReversedA&);
   friend constexpr bool operator<(const ReversedA&, const ReversedA&);
-  friend constexpr std::strong_ordering operator<=>(const ReversedA&, const ReversedA&);
+  friend constexpr std::strong_ordering operator<=>(const ReversedA&, const ReversedA&) noexcept;
 
-  friend constexpr bool operator!=(const ReversedA&, const ReversedA&);
-  friend constexpr bool operator==(const ReversedA&, const ReversedA&);
+  friend constexpr bool operator!=(const ReversedA&, const ReversedA&) noexcept;
+  friend constexpr bool operator==(const ReversedA&, const ReversedA&) noexcept;
 };
 
 struct B {
@@ -69,8 +69,8 @@ struct B {
   friend bool operator>=(const B&, const B&) = default;
 };
 struct TestB {
-  friend constexpr bool operator==(const B&, const B&);
-  friend constexpr bool operator!=(const B&, const B&);
+  friend constexpr bool operator==(const B&, const B&) noexcept;
+  friend constexpr bool operator!=(const B&, const B&) noexcept;
 
   friend constexpr std::strong_ordering operator<=>(const B&, const B&);
   friend constexpr bool operator<(const B&, const B&);
diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p4.cpp
@@ -21,8 +21,8 @@ namespace N {
 
   constexpr bool (*test_a_not_found)(const A&, const A&) = &operator==; // expected-error {{undeclared}}
 
-  constexpr bool operator==(const A&, const A&);
-  constexpr bool (*test_a)(const A&, const A&) = &operator==;
+  constexpr bool operator==(const A&, const A&) noexcept;
+  constexpr bool (*test_a)(const A&, const A&) noexcept = &operator==;
   static_assert((*test_a)(A(), A()));
 }
 
diff --git a/clang/test/CXX/except/except.spec/p11-2a.cpp b/clang/test/CXX/except/except.spec/p11-2a.cpp

Original file line number	Diff line number	Diff line change
`@@ -21,8 +21,8 @@ namespace N {`
`21`	`21`
`22`	`22`	`constexpr bool (*test_a_not_found)(const A&, const A&) = &operator==; // expected-error {{undeclared}}`
`23`	`23`
`24`		`- constexpr bool operator==(const A&, const A&);`
`25`		`- constexpr bool (*test_a)(const A&, const A&) = &operator==;`
	`24`	`+ constexpr bool operator==(const A&, const A&) noexcept;`
	`25`	`+ constexpr bool (*test_a)(const A&, const A&) noexcept = &operator==;`
`26`	`26`	`static_assert((*test_a)(A(), A()));`
`27`	`27`	`}`
`28`	`28`