Merge pull request swiftlang#70027 from DougGregor/typechecked-function-body

Author: DougGregor

include/swift/AST/Decl.h

@@ -7245,7 +7245,7 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
   ///
   /// Note that a true return value does not imply that the body was actually
   /// parsed.
-  bool hasBody() const { return getBodyKind() != BodyKind::None; }
+  bool hasBody() const;
 
   /// Returns true if the text of this function's body can be retrieved either
   /// by extracting the text from the source buffer or reading the inlinable

lib/AST/Decl.cpp

@@ -9102,6 +9102,24 @@ bool AbstractFunctionDecl::isSendable() const {
   return getAttrs().hasAttribute<SendableAttr>();
 }
 
+bool AbstractFunctionDecl::hasBody() const {
+  switch (getBodyKind()) {
+  case BodyKind::Deserialized:
+  case BodyKind::Parsed:
+  case BodyKind::SILSynthesize:
+  case BodyKind::Synthesize:
+  case BodyKind::Unparsed:
+    return true;
+
+  case BodyKind::None:
+    return false;
+
+  case BodyKind::TypeChecked:
+    return BodyAndFP.getBody() != nullptr;
+  }
+}
+
+
 BraceStmt *AbstractFunctionDecl::getBody(bool canSynthesize) const {
   if ((getBodyKind() == BodyKind::Synthesize ||
        getBodyKind() == BodyKind::Unparsed) &&
@@ -10857,13 +10875,13 @@ void ParseAbstractFunctionBodyRequest::cacheResult(
   switch (afd->getBodyKind()) {
   case BodyKind::Deserialized:
   case BodyKind::SILSynthesize:
-  case BodyKind::None:
     // The body is always empty, so don't cache anything.
     assert(!value.getFingerprint().has_value() && value.getBody() == nullptr);
     return;
 
   case BodyKind::Parsed:
   case BodyKind::TypeChecked:
+  case BodyKind::None:
     afd->BodyAndFP = value;
     return;
 

lib/AST/TypeCheckRequests.cpp

@@ -1540,7 +1540,6 @@ TypeCheckFunctionBodyRequest::getCachedResult() const {
   switch (afd->getBodyKind()) {
   case BodyKind::Deserialized:
   case BodyKind::SILSynthesize:
-  case BodyKind::None:
     // These cases don't have any body available.
     return nullptr;
 
@@ -1550,6 +1549,7 @@ TypeCheckFunctionBodyRequest::getCachedResult() const {
   case BodyKind::Synthesize:
   case BodyKind::Parsed:
   case BodyKind::Unparsed:
+  case BodyKind::None:
     return llvm::None;
   }
   llvm_unreachable("Unhandled BodyKind in switch");

lib/ConstExtract/ConstExtract.cpp

@@ -412,11 +412,13 @@ extractTypePropertyInfo(VarDecl *propertyDecl) {
   }
 
   if (auto accessorDecl = propertyDecl->getAccessor(AccessorKind::Get)) {
-    auto node = accessorDecl->getTypecheckedBody()->getFirstElement();
-    if (auto *stmt = node.dyn_cast<Stmt *>()) {
-      if (stmt->getKind() == StmtKind::Return) {
-        return {propertyDecl,
-                extractCompileTimeValue(cast<ReturnStmt>(stmt)->getResult())};
+    if (auto body = accessorDecl->getTypecheckedBody()) {
+      auto node = body->getFirstElement();
+      if (auto *stmt = node.dyn_cast<Stmt *>()) {
+        if (stmt->getKind() == StmtKind::Return) {
+          return {propertyDecl,
+                  extractCompileTimeValue(cast<ReturnStmt>(stmt)->getResult())};
+        }
       }
     }
   }

lib/Sema/DebuggerTestingTransform.cpp

@@ -88,7 +88,7 @@ class DebuggerTestingTransform : public ASTWalker {
     // Whitelist the kinds of decls to transform.
     // TODO: Expand the set of decls visited here.
     if (auto *FD = dyn_cast<AbstractFunctionDecl>(D))
-      return Action::VisitChildrenIf(FD->getBody());
+      return Action::VisitChildrenIf(FD->getTypecheckedBody());
     if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(D))
       return Action::VisitChildrenIf(TLCD->getBody());
     if (isa<NominalTypeDecl>(D))

lib/Sema/PCMacro.cpp

@@ -336,7 +336,7 @@ class Instrumenter : InstrumenterBase {
     if (D->isImplicit())
       return D;
     if (auto *FD = dyn_cast<FuncDecl>(D)) {
-      if (BraceStmt *B = FD->getBody()) {
+      if (BraceStmt *B = FD->getTypecheckedBody()) {
         const ParameterList *PL = FD->getParameters();
         BraceStmt *NB = transformBraceStmt(B, PL);
         // Since it would look strange going straight to the first line in a

lib/Sema/PlaygroundTransform.cpp

@@ -308,7 +308,7 @@ class Instrumenter : InstrumenterBase {
     if (D->isImplicit())
       return D;
     if (auto *FD = dyn_cast<FuncDecl>(D)) {
-      if (BraceStmt *B = FD->getBody()) {
+      if (BraceStmt *B = FD->getTypecheckedBody()) {
         const ParameterList *PL = FD->getParameters();
         TargetKindSetter TKS(BracePairs, BracePair::TargetKinds::Return);
         BraceStmt *NB = transformBraceStmt(B, PL);

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3434,57 +3434,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     // PatternBindingDecl.
   }
 
-  /// Determine whether the given declaration should not have a definition.
-  static bool requiresNoDefinition(Decl *decl) {
-    if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
-      // Function with @_extern should not have a body.
-      return func->getAttrs().hasAttribute<ExternAttr>();
-    }
-    // Everything else can have a definition.
-    return false;
-  }
-
-  /// Determine whether the given declaration requires a definition.
-  ///
-  /// Only valid for declarations that can have definitions, i.e.,
-  /// functions, initializers, etc.
-  static bool requiresDefinition(Decl *decl) {
-    // Invalid, implicit, and Clang-imported declarations never
-    // require a definition.
-    if (decl->isInvalid() || decl->isImplicit() || decl->hasClangNode())
-      return false;
-
-    // Protocol requirements do not require definitions.
-    if (isa<ProtocolDecl>(decl->getDeclContext()))
-      return false;
-
-    // Functions can have _silgen_name, semantics, and NSManaged attributes.
-    if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
-      if (func->getAttrs().hasAttribute<SILGenNameAttr>() ||
-          func->getAttrs().hasAttribute<ExternAttr>() ||
-          func->getAttrs().hasAttribute<SemanticsAttr>() ||
-          func->getAttrs().hasAttribute<NSManagedAttr>())
-        return false;
-    }
-
-    // Declarations in SIL and module interface files don't require
-    // definitions.
-    if (auto sourceFile = decl->getDeclContext()->getParentSourceFile()) {
-      switch (sourceFile->Kind) {
-      case SourceFileKind::SIL:
-      case SourceFileKind::Interface:
-        return false;
-      case SourceFileKind::Library:
-      case SourceFileKind::Main:
-      case SourceFileKind::MacroExpansion:
-        break;
-      }
-    }
-
-    // Everything else requires a definition.
-    return true;
-  }
-
   /// FIXME: This is an egregious hack to turn off availability checking
   /// for specific functions that were missing availability in older versions
   /// of existing libraries that we must nonetheless still support.
@@ -3541,13 +3490,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         !hasHistoricallyWrongAvailability(FD))
       TypeChecker::checkConcurrencyAvailability(FD->getAsyncLoc(), FD);
 
-    if (requiresDefinition(FD) && !FD->hasBody()) {
-      // Complain if we should have a body.
-      FD->diagnose(diag::func_decl_without_brace);
-    } else if (requiresNoDefinition(FD) && FD->hasBody()) {
-      // Complain if we have a body but shouldn't.
-      FD->diagnose(diag::func_decl_no_body_expected);
-    } else if (FD->getDeclContext()->isLocalContext()) {
+    if (FD->getDeclContext()->isLocalContext()) {
       // Check local function bodies right away.
       (void)FD->getTypecheckedBody();
       TypeChecker::computeCaptures(FD);
@@ -4043,10 +3986,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     checkExplicitAvailability(CD);
 
-    if (requiresDefinition(CD) && !CD->hasBody()) {
-      // Complain if we should have a body.
-      CD->diagnose(diag::missing_initializer_def);
-    } else if (CD->getDeclContext()->isLocalContext()) {
+    if (CD->getDeclContext()->isLocalContext()) {
       // Check local function bodies right away.
       (void)CD->getTypecheckedBody();
     } else if (!CD->isBodySkipped()) {

lib/Sema/TypeCheckStmt.cpp

@@ -2717,19 +2717,98 @@ PreCheckFunctionBodyRequest::evaluate(Evaluator &evaluator,
   return body;
 }
 
+/// Determine whether the given declaration requires a definition.
+///
+/// Only valid for declarations that can have definitions, i.e.,
+/// functions, initializers, etc.
+static bool requiresDefinition(Decl *decl) {
+  // Invalid, implicit, and Clang-imported declarations never
+  // require a definition.
+  if (decl->isInvalid() || decl->isImplicit() || decl->hasClangNode())
+    return false;
+
+  // Protocol requirements do not require definitions.
+  if (isa<ProtocolDecl>(decl->getDeclContext()))
+    return false;
+
+  // Functions can have _silgen_name, semantics, and NSManaged attributes.
+  if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
+    if (func->getAttrs().hasAttribute<SILGenNameAttr>() ||
+        func->getAttrs().hasAttribute<ExternAttr>() ||
+        func->getAttrs().hasAttribute<SemanticsAttr>() ||
+        func->getAttrs().hasAttribute<NSManagedAttr>())
+      return false;
+  }
+
+  // Declarations in SIL and module interface files don't require
+  // definitions.
+  auto dc = decl->getDeclContext();
+  if (auto sourceFile = dc->getParentSourceFile()) {
+    switch (sourceFile->Kind) {
+    case SourceFileKind::SIL:
+    case SourceFileKind::Interface:
+      return false;
+    case SourceFileKind::Library:
+    case SourceFileKind::Main:
+    case SourceFileKind::MacroExpansion:
+      break;
+    }
+  }
+
+  // Declarations deserialized from a module file don't require definitions.
+  if (auto fileUnit = dyn_cast<FileUnit>(dc->getModuleScopeContext()))
+    if (fileUnit->getKind() == FileUnitKind::SerializedAST)
+      return false;
+
+  // Everything else requires a definition.
+  return true;
+}
+
+/// Determine whether the given declaration should not have a definition.
+static bool requiresNoDefinition(Decl *decl) {
+  if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
+    // Function with @_extern should not have a body.
+    return func->getAttrs().hasAttribute<ExternAttr>();
+  }
+  // Everything else can have a definition.
+  return false;
+}
+
 BraceStmt *
 TypeCheckFunctionBodyRequest::evaluate(Evaluator &eval,
                                        AbstractFunctionDecl *AFD) const {
-  assert(!AFD->isBodySkipped());
-
   ASTContext &ctx = AFD->getASTContext();
 
   llvm::Optional<FunctionBodyTimer> timer;
   const auto &tyOpts = ctx.TypeCheckerOpts;
   if (tyOpts.DebugTimeFunctionBodies || tyOpts.WarnLongFunctionBodies)
     timer.emplace(AFD);
 
+  /// If the function body has been skipped, there's nothing to do here.
+  if (AFD->isBodySkipped())
+    return nullptr;
+
   BraceStmt *body = AFD->getBody();
+
+  // If there is no function body, there is nothing to type-check.
+  if (!body) {
+    // If a definition is required here, complain.
+    if (requiresDefinition(AFD)) {
+      if (isa<ConstructorDecl>(AFD))
+        AFD->diagnose(diag::missing_initializer_def);
+      else
+        AFD->diagnose(diag::func_decl_without_brace);
+    }
+
+    return nullptr;
+  }
+
+  // If the function body must not have a definition, complain and drop it.
+  if (requiresNoDefinition(AFD)) {
+    AFD->diagnose(diag::func_decl_no_body_expected);
+    return nullptr;
+  }
+
   assert(body && "Expected body to type-check");
 
   // It's possible we synthesized an already type-checked body, in which case

test/Parse/recovery.swift

@@ -580,9 +580,8 @@ class С_43383 {
 extension С_43383 {
     print("The room where it happened, the room where it happened")
     // expected-error @-1 {{expected 'func' keyword in instance method declaration}}
-    // expected-error @-2 {{expected '{' in body of function declaration}}
-    // expected-error @-3 {{invalid redeclaration of 'print()'}}
-    // expected-error @-4 {{expected parameter name followed by ':'}}
+    // expected-error @-2 {{invalid redeclaration of 'print()'}}
+    // expected-error @-3 {{expected parameter name followed by ':'}}
 }