[TypeChecker] Propagate HadError from elements in BraceStmts

ahoppen · ahoppen · commit 3f82b4cb8876 · 2022-06-28T13:45:25.000+02:00
Previously, if a `BraceStmt` had an element with an error, we still considered the entire `BraceStmt` as being type-checked successfully. This caused syntactic diagnostics to run for statements that weren’t valid. Propagate that information. Most places still ignore whether the brace statement contained an error because they ignore the return value of `typecheckFunctionBody()` but `typeCheckClosureBody` now returns the error, preventing syntactic diagnostics from running for closures in result builders.

rdar://95629905
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -412,9 +412,9 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
   SWIFT_INLINE_BITFIELD(SubscriptDecl, VarDecl, 2,
     StaticSpelling : 2
   );
-  SWIFT_INLINE_BITFIELD(AbstractFunctionDecl, ValueDecl, 3+2+8+1+1+1+1+1+1,
+  SWIFT_INLINE_BITFIELD(AbstractFunctionDecl, ValueDecl, 4+2+8+1+1+1+1+1+1,
     /// \see AbstractFunctionDecl::BodyKind
-    BodyKind : 3,
+    BodyKind : 4,
 
     /// \see AbstractFunctionDecl::SILSynthesizeKind
     SILSynthesizeKind : 2,
@@ -6236,13 +6236,17 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
     /// Function body is present and type-checked.
     TypeChecked,
 
+    /// The function body is present and was type-checked but at least one
+    /// statement within the body contained an error.
+    TypeCheckedWithError,
+
     // Function body will be synthesized by SILGen.
     SILSynthesize,
 
     /// Function body text was deserialized from a .swiftmodule.
     Deserialized
 
-    // This enum currently needs to fit in a 3-bit bitfield.
+    // This enum currently needs to fit in a 4-bit bitfield.
   };
 
   BodyKind getBodyKind() const {
@@ -6478,9 +6482,18 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
   /// \sa hasBody()
   BraceStmt *getBody(bool canSynthesize = true) const;
 
+  struct TypecheckedBodyResult {
+    /// The body of the function with as many statements type checked as
+    /// possible.
+    BraceStmt *TypecheckedBody;
+    /// If a single statement in the type checked body has an error, this is
+    /// \c true.
+    bool HadError;
+  };
+
   /// Retrieve the type-checked body of the given function, or \c nullptr if
   /// there's no body available.
-  BraceStmt *getTypecheckedBody() const;
+  TypecheckedBodyResult getTypecheckedBody() const;
 
   /// Set a new body for the function.
   void setBody(BraceStmt *S, BodyKind NewBodyKind);
@@ -6570,7 +6583,8 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
   }
 
   bool isBodyTypeChecked() const {
-    return getBodyKind() == BodyKind::TypeChecked;
+    return getBodyKind() == BodyKind::TypeChecked ||
+           getBodyKind() == BodyKind::TypeCheckedWithError;
   }
 
   bool isBodySILSynthesize() const {
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -1480,25 +1480,32 @@ class LazyStoragePropertyRequest :
   bool isCached() const { return true; }
 };
 
+struct TypeCheckFunctionBodyResult {
+  BraceStmt *TypeCheckedBody;
+  bool HadError;
+};
+
 /// Request to retrieve the type-checked body of the given function.
 class TypeCheckFunctionBodyRequest
-    : public SimpleRequest<
-          TypeCheckFunctionBodyRequest, BraceStmt *(AbstractFunctionDecl *),
-          RequestFlags::SeparatelyCached | RequestFlags::DependencySource> {
+    : public SimpleRequest<TypeCheckFunctionBodyRequest,
+                           TypeCheckFunctionBodyResult(AbstractFunctionDecl *),
+                           RequestFlags::SeparatelyCached |
+                               RequestFlags::DependencySource> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
   // Evaluation.
-  BraceStmt *evaluate(Evaluator &evaluator, AbstractFunctionDecl *func) const;
+  TypeCheckFunctionBodyResult evaluate(Evaluator &evaluator,
+                                       AbstractFunctionDecl *func) const;
 
 public:
   // Separate caching.
   bool isCached() const { return true; }
-  Optional<BraceStmt *> getCachedResult() const;
-  void cacheResult(BraceStmt *body) const;
+  Optional<TypeCheckFunctionBodyResult> getCachedResult() const;
+  void cacheResult(TypeCheckFunctionBodyResult result) const;
 
 public:
   // Incremental dependencies.
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -309,7 +309,7 @@ SWIFT_REQUEST(TypeChecker, SynthesizeAccessorRequest,
 SWIFT_REQUEST(TypeChecker, TangentStoredPropertyRequest,
               llvm::Expected<VarDecl *>(VarDecl *, CanType), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, TypeCheckFunctionBodyRequest,
-              BraceStmt *(AbstractFunctionDecl *), SeparatelyCached,
+              TypeCheckFunctionBodyResult(AbstractFunctionDecl *), SeparatelyCached,
               NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, TypeCheckASTNodeAtLocRequest,
               bool(TypeCheckASTNodeAtLocContext, SourceLoc),
diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
@@ -474,6 +474,7 @@ class Verifier : public ASTWalker {
       switch (afd->getBodyKind()) {
       case AbstractFunctionDecl::BodyKind::None:
       case AbstractFunctionDecl::BodyKind::TypeChecked:
+      case AbstractFunctionDecl::BodyKind::TypeCheckedWithError:
       case AbstractFunctionDecl::BodyKind::Skipped:
       case AbstractFunctionDecl::BodyKind::SILSynthesize:
       case AbstractFunctionDecl::BodyKind::Deserialized:
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -7819,11 +7819,14 @@ BraceStmt *AbstractFunctionDecl::getBody(bool canSynthesize) const {
       .getBody();
 }
 
-BraceStmt *AbstractFunctionDecl::getTypecheckedBody() const {
+AbstractFunctionDecl::TypecheckedBodyResult
+AbstractFunctionDecl::getTypecheckedBody() const {
   auto &ctx = getASTContext();
   auto *mutableThis = const_cast<AbstractFunctionDecl *>(this);
-  return evaluateOrDefault(
-      ctx.evaluator, TypeCheckFunctionBodyRequest{mutableThis}, nullptr);
+  auto res = evaluateOrDefault(ctx.evaluator,
+                               TypeCheckFunctionBodyRequest{mutableThis},
+                               {nullptr, true});
+  return {res.TypeCheckedBody, res.HadError};
 }
 
 void AbstractFunctionDecl::setBody(BraceStmt *S, BodyKind NewBodyKind) {
@@ -7832,6 +7835,7 @@ void AbstractFunctionDecl::setBody(BraceStmt *S, BodyKind NewBodyKind) {
 
   Optional<Fingerprint> fp = None;
   if (getBodyKind() == BodyKind::TypeChecked ||
+      getBodyKind() == BodyKind::TypeCheckedWithError ||
       getBodyKind() == BodyKind::Parsed) {
     fp = BodyAndFP.getFingerprint();
   }
@@ -7849,7 +7853,8 @@ void AbstractFunctionDecl::setBodyToBeReparsed(SourceRange bodyRange) {
   assert(bodyRange.isValid());
   assert(getBodyKind() == BodyKind::Unparsed ||
          getBodyKind() == BodyKind::Parsed ||
-         getBodyKind() == BodyKind::TypeChecked);
+         getBodyKind() == BodyKind::TypeChecked ||
+         getBodyKind() == BodyKind::TypeCheckedWithError);
 
   keepOriginalBodySourceRange();
   BodyRange = bodyRange;
@@ -7874,6 +7879,7 @@ SourceRange AbstractFunctionDecl::getBodySourceRange() const {
 
   case BodyKind::Parsed:
   case BodyKind::TypeChecked:
+  case BodyKind::TypeCheckedWithError:
     if (auto body = getBody(/*canSynthesize=*/false))
       return body->getSourceRange();
 
@@ -8276,6 +8282,7 @@ bool AbstractFunctionDecl::hasInlinableBodyText() const {
   case BodyKind::Unparsed:
   case BodyKind::Parsed:
   case BodyKind::TypeChecked:
+  case BodyKind::TypeCheckedWithError:
     if (auto body = getBody())
       return !body->isImplicit();
     return false;
@@ -9401,6 +9408,7 @@ ParseAbstractFunctionBodyRequest::getCachedResult() const {
     return BodyAndFingerprint{};
 
   case BodyKind::TypeChecked:
+  case BodyKind::TypeCheckedWithError:
   case BodyKind::Parsed:
     return afd->BodyAndFP;
 
@@ -9426,6 +9434,7 @@ void ParseAbstractFunctionBodyRequest::cacheResult(
 
   case BodyKind::Parsed:
   case BodyKind::TypeChecked:
+  case BodyKind::TypeCheckedWithError:
     afd->BodyAndFP = value;
     return;
 
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
@@ -1375,7 +1375,8 @@ void TypeCheckSourceFileRequest::cacheResult(evaluator::SideEffect) const {
 // TypeCheckFunctionBodyRequest computation.
 //----------------------------------------------------------------------------//
 
-Optional<BraceStmt *> TypeCheckFunctionBodyRequest::getCachedResult() const {
+Optional<TypeCheckFunctionBodyResult>
+TypeCheckFunctionBodyRequest::getCachedResult() const {
   using BodyKind = AbstractFunctionDecl::BodyKind;
   auto *afd = std::get<0>(getStorage());
   switch (afd->getBodyKind()) {
@@ -1384,10 +1385,12 @@ Optional<BraceStmt *> TypeCheckFunctionBodyRequest::getCachedResult() const {
   case BodyKind::None:
   case BodyKind::Skipped:
     // These cases don't have any body available.
-    return nullptr;
+    return TypeCheckFunctionBodyResult{nullptr, false};
 
   case BodyKind::TypeChecked:
-    return afd->BodyAndFP.getBody();
+    return TypeCheckFunctionBodyResult{afd->BodyAndFP.getBody(), false};
+  case BodyKind::TypeCheckedWithError:
+    return TypeCheckFunctionBodyResult{afd->BodyAndFP.getBody(), true};
 
   case BodyKind::Synthesize:
   case BodyKind::Parsed:
@@ -1397,9 +1400,13 @@ Optional<BraceStmt *> TypeCheckFunctionBodyRequest::getCachedResult() const {
   llvm_unreachable("Unhandled BodyKind in switch");
 }
 
-void TypeCheckFunctionBodyRequest::cacheResult(BraceStmt *body) const {
+void TypeCheckFunctionBodyRequest::cacheResult(
+    TypeCheckFunctionBodyResult result) const {
   auto *afd = std::get<0>(getStorage());
-  afd->setBody(body, AbstractFunctionDecl::BodyKind::TypeChecked);
+  afd->setBody(result.TypeCheckedBody,
+               result.HadError
+                   ? AbstractFunctionDecl::BodyKind::TypeCheckedWithError
+                   : AbstractFunctionDecl::BodyKind::TypeChecked);
 }
 
 evaluator::DependencySource
diff --git a/lib/Parse/ParseRequests.cpp b/lib/Parse/ParseRequests.cpp
@@ -100,6 +100,7 @@ ParseAbstractFunctionBodyRequest::evaluate(Evaluator &evaluator,
     return {};
 
   case BodyKind::TypeChecked:
+  case BodyKind::TypeCheckedWithError:
   case BodyKind::Parsed:
     return afd->BodyAndFP;
 
diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp
@@ -461,11 +461,12 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
     emitMemberInitializers(ctor, selfDecl, nominal);
   }
 
-  emitProfilerIncrement(ctor->getTypecheckedBody());
+  auto typecheckedBody = ctor->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
   // Emit the constructor body.
-  emitStmt(ctor->getTypecheckedBody());
+  emitStmt(typecheckedBody.TypecheckedBody);
 
-  
   // Build a custom epilog block, since the AST representation of the
   // constructor decl (which has no self in the return type) doesn't match the
   // SIL representation.
@@ -693,7 +694,9 @@ void SILGenFunction::emitClassConstructorAllocator(ConstructorDecl *ctor) {
   SILValue initedSelfValue = emitApplyWithRethrow(Loc, initVal.forward(*this),
                                                   initTy, subMap, args);
 
-  emitProfilerIncrement(ctor->getTypecheckedBody());
+  auto typecheckedBody = ctor->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
 
   // Return the initialized 'self'.
   B.createReturn(ImplicitReturnLocation(Loc), initedSelfValue);
@@ -732,7 +735,10 @@ void SILGenFunction::emitConstructorPrologActorHop(
 void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
   MagicFunctionName = SILGenModule::getMagicFunctionName(ctor);
 
-  assert(ctor->getTypecheckedBody() && "Class constructor without a body?");
+  assert(!ctor->getTypecheckedBody().HadError &&
+         "Class constructor with an error?");
+  assert(ctor->getTypecheckedBody().TypecheckedBody &&
+         "Class constructor without a body?");
 
   // True if this constructor delegates to a peer constructor with self.init().
   bool isDelegating = false;
@@ -902,9 +908,11 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
     emitMemberInitializers(ctor, selfDecl, selfClassDecl);
   }
 
-  emitProfilerIncrement(ctor->getTypecheckedBody());
+  auto typecheckedBody = ctor->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
   // Emit the constructor body.
-  emitStmt(ctor->getTypecheckedBody());
+  emitStmt(typecheckedBody.TypecheckedBody);
 
   // Emit the call to super.init() right before exiting from the initializer.
   if (NeedsBoxForSelf) {
diff --git a/lib/SILGen/SILGenDestructor.cpp b/lib/SILGen/SILGenDestructor.cpp
@@ -38,9 +38,11 @@ void SILGenFunction::emitDestroyingDestructor(DestructorDecl *dd) {
   // We won't actually emit the block until we finish with the destructor body.
   prepareEpilog(None, false, CleanupLocation(Loc));
 
-  emitProfilerIncrement(dd->getTypecheckedBody());
+  auto typecheckedBody = dd->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
   // Emit the destructor body.
-  emitStmt(dd->getTypecheckedBody());
+  emitStmt(typecheckedBody.TypecheckedBody);
 
   Optional<SILValue> maybeReturnValue;
   SILLocation returnLoc(Loc);
@@ -170,7 +172,9 @@ void SILGenFunction::emitDeallocatingDestructor(DestructorDecl *dd) {
   selfForDealloc = B.createUncheckedRefCast(loc, selfForDealloc, classTy);
   B.createDeallocRef(loc, selfForDealloc);
 
-  emitProfilerIncrement(dd->getTypecheckedBody());
+  auto typecheckedBody = dd->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
 
   // Return.
   B.createReturn(loc, emitEmptyTuple(loc));
@@ -445,9 +449,11 @@ void SILGenFunction::emitObjCDestructor(SILDeclRef dtor) {
   // We won't actually emit the block until we finish with the destructor body.
   prepareEpilog(None, false, CleanupLocation(loc));
 
-  emitProfilerIncrement(dd->getTypecheckedBody());
+  auto typecheckedBody = dd->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
   // Emit the destructor body.
-  emitStmt(dd->getTypecheckedBody());
+  emitStmt(typecheckedBody.TypecheckedBody);
 
   Optional<SILValue> maybeReturnValue;
   SILLocation returnLoc(loc);
diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp
@@ -525,7 +525,9 @@ void SILGenFunction::emitFunction(FuncDecl *fd) {
   MagicFunctionName = SILGenModule::getMagicFunctionName(fd);
 
   auto captureInfo = SGM.M.Types.getLoweredLocalCaptures(SILDeclRef(fd));
-  emitProfilerIncrement(fd->getTypecheckedBody());
+  auto typecheckedBody = fd->getTypecheckedBody();
+  assert(!typecheckedBody.HadError);
+  emitProfilerIncrement(typecheckedBody.TypecheckedBody);
   emitProlog(captureInfo, fd->getParameters(), fd->getImplicitSelfDecl(), fd,
              fd->getResultInterfaceType(), fd->hasThrows(), fd->getThrowsLoc());
 
@@ -537,7 +539,7 @@ void SILGenFunction::emitFunction(FuncDecl *fd) {
                   fd->hasThrows(), CleanupLocation(fd));
     
     // Emit the actual function body as usual
-    emitStmt(fd->getTypecheckedBody());
+    emitStmt(typecheckedBody.TypecheckedBody);
 
     emitEpilog(fd);
   }
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -3229,7 +3229,7 @@ class ExprAvailabilityWalker : public ASTWalker {
       maybeDiagParameterizedExistentialErasure(EE, Where);
     }
     if (auto *CC = dyn_cast<ExplicitCastExpr>(E)) {
-      if (!isa<CoerceExpr>(CC) &&
+      if (!isa<CoerceExpr>(CC) && CC->getCastType() &&
           CC->getCastType()->hasParameterizedExistential()) {
         SourceLoc loc = CC->getCastTypeRepr() ? CC->getCastTypeRepr()->getLoc()
                                               : E->getLoc();
diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp
diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp
diff --git a/test/Parse/switch.swift b/test/Parse/switch.swift
diff --git a/validation-test/compiler_crashers_2_fixed/rdar95629905.swift b/validation-test/compiler_crashers_2_fixed/rdar95629905.swift

Original file line number	Diff line number	Diff line change
`@@ -3229,7 +3229,7 @@ class ExprAvailabilityWalker : public ASTWalker {`
`3229`	`3229`	`maybeDiagParameterizedExistentialErasure(EE, Where);`
`3230`	`3230`	`}`
`3231`	`3231`	`if (auto *CC = dyn_cast<ExplicitCastExpr>(E)) {`
`3232`		`- if (!isa<CoerceExpr>(CC) &&`
	`3232`	`+ if (!isa<CoerceExpr>(CC) && CC->getCastType() &&`
`3233`	`3233`	`CC->getCastType()->hasParameterizedExistential()) {`
`3234`	`3234`	`SourceLoc loc = CC->getCastTypeRepr() ? CC->getCastTypeRepr()->getLoc()`
`3235`	`3235`	`: E->getLoc();`