Sema: Don't synthesize null bodies for invalid accessors

We don't want hasBody() == true and getBody() == nullptr.

Author: slavapestov

lib/AST/ASTContext.cpp

@@ -3407,8 +3407,6 @@ ProtocolType::ProtocolType(ProtocolDecl *TheDecl, Type Parent,
   : NominalType(TypeKind::Protocol, &Ctx, TheDecl, Parent, properties) { }
 
 LValueType *LValueType::get(Type objectTy) {
-  assert(!objectTy->hasError() &&
-         "cannot have ErrorType wrapped inside LValueType");
   assert(!objectTy->is<LValueType>() && !objectTy->is<InOutType>() &&
          "cannot have 'inout' or @lvalue wrapped inside an @lvalue");
 

lib/Parse/ParseRequests.cpp

@@ -70,6 +70,7 @@ BraceStmt *ParseAbstractFunctionBodyRequest::evaluate(
 
     std::tie(body, isTypeChecked) = (afd->Synthesizer.Fn)(
         afd, afd->Synthesizer.Context);
+    assert(body && "cannot synthesize a null body");
     afd->setBodyKind(isTypeChecked ? BodyKind::TypeChecked : BodyKind::Parsed);
     return body;
   }

lib/Sema/MiscDiagnostics.cpp

@@ -2140,6 +2140,12 @@ class VarDeclUsageChecker : public ASTWalker {
       }
     }
 
+    // Don't walk into implicit accessors, since eg. an observer's setter
+    // references the variable, but we don't want to consider it as a real
+    // "use".
+    if (isa<AccessorDecl>(D) && D->isImplicit())
+      return false;
+
     if (auto *afd = dyn_cast<AbstractFunctionDecl>(D)) {
       // If this is a nested function with a capture list, mark any captured
       // variables.

lib/Sema/TypeCheckStorage.cpp

@@ -659,15 +659,24 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
     // Otherwise do a self-reference, which is dynamically bogus but
     // should be statically valid.  This should only happen in invalid cases.    
     } else {
-      assert(storage->isInvalid());
       semantics = AccessSemantics::Ordinary;
       selfAccessKind = SelfAccessorKind::Peer;
     }
     break;
 
   case TargetImpl::Wrapper: {
     auto var = cast<VarDecl>(accessor->getStorage());
-    storage = var->getPropertyWrapperBackingProperty();
+    auto *backing = var->getPropertyWrapperBackingProperty();
+
+    // Error recovery.
+    if (!backing) {
+      auto type = storage->getValueInterfaceType();
+      if (isLValue)
+        type = LValueType::get(type);
+      return new (ctx) ErrorExpr(SourceRange(), type);
+    }
+
+    storage = backing;
 
     // If the outermost property wrapper uses the enclosing self pattern,
     // record that.
@@ -691,7 +700,18 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
   case TargetImpl::WrapperStorage: {
     auto var =
         cast<VarDecl>(accessor->getStorage())->getOriginalWrappedProperty();
-    storage = var->getPropertyWrapperBackingProperty();
+    auto *backing = var->getPropertyWrapperBackingProperty();
+
+    // Error recovery.
+    if (!backing) {
+      auto type = storage->getValueInterfaceType();
+      if (isLValue)
+        type = LValueType::get(type);
+      return new (ctx) ErrorExpr(SourceRange(), type);
+    }
+
+    storage = backing;
+
     enclosingSelfAccess =
         getEnclosingSelfPropertyWrapperAccess(var, /*forProjected=*/true);
     if (!enclosingSelfAccess) {
@@ -976,12 +996,21 @@ void createPropertyStoreOrCallSuperclassSetter(AccessorDecl *accessor,
       value = synthesizeCopyWithZoneCall(value, property, ctx);
   }
 
+  // Error recovery.
+  if (value->getType()->hasError())
+    return;
+
   Expr *dest = buildStorageReference(accessor, storage, target,
                                      /*isLValue=*/true, ctx);
 
   // A lazy property setter will store a value of type T into underlying storage
   // of type T?.
   auto destType = dest->getType()->getWithoutSpecifierType();
+
+  // Error recovery.
+  if (destType->hasError())
+    return;
+
   if (!destType->isEqual(value->getType())) {
     assert(destType->getOptionalObjectType()->isEqual(value->getType()));
     value = new (ctx) InjectIntoOptionalExpr(value, destType);
@@ -1172,17 +1201,25 @@ synthesizeLazyGetterBody(AccessorDecl *Get, VarDecl *VD, VarDecl *Storage,
   // Take the initializer from the PatternBindingDecl for VD.
   // TODO: This doesn't work with complicated patterns like:
   //   lazy var (a,b) = foo()
-  auto *InitValue = VD->getParentInitializer();
   auto PBD = VD->getParentPatternBinding();
   unsigned entryIndex = PBD->getPatternEntryIndexForVarDecl(VD);
-  PBD->setInitializerSubsumed(entryIndex);
 
-  if (!PBD->isInitializerChecked(entryIndex))
-    TC.typeCheckPatternBinding(PBD, entryIndex);
+  Expr *InitValue;
+  if (PBD->getPatternList()[entryIndex].getInit()) {
+    PBD->setInitializerSubsumed(entryIndex);
+
+    if (!PBD->isInitializerChecked(entryIndex))
+      TC.typeCheckPatternBinding(PBD, entryIndex);
+
+    InitValue = PBD->getPatternList()[entryIndex].getInit();
+  } else {
+    InitValue = new (Ctx) ErrorExpr(SourceRange(), Tmp2VD->getType());
+  }
 
   // Recontextualize any closure declcontexts nested in the initializer to
   // realize that they are in the getter function.
   Get->getImplicitSelfDecl()->setDeclContext(Get);
+
   InitValue->walk(RecontextualizeClosures(Get));
 
   // Wrap the initializer in a LazyInitializerExpr to avoid walking it twice.
@@ -1226,6 +1263,12 @@ synthesizePropertyWrapperGetterBody(AccessorDecl *getter, ASTContext &ctx) {
   return synthesizeTrivialGetterBody(getter, TargetImpl::Wrapper, ctx);
 }
 
+static std::pair<BraceStmt *, bool>
+synthesizeInvalidAccessor(AccessorDecl *accessor, ASTContext &ctx) {
+  auto loc = accessor->getLoc();
+  return { BraceStmt::create(ctx, loc, ArrayRef<ASTNode>(), loc, true), true };
+}
+
 static std::pair<BraceStmt *, bool>
 synthesizeGetterBody(AccessorDecl *getter, ASTContext &ctx) {
   auto storage = getter->getStorage();
@@ -1257,7 +1300,7 @@ synthesizeGetterBody(AccessorDecl *getter, ASTContext &ctx) {
     return synthesizeTrivialGetterBody(getter, ctx);
 
   case ReadImplKind::Get:
-    llvm_unreachable("synthesizing getter that already exists?");
+    return synthesizeInvalidAccessor(getter, ctx);
 
   case ReadImplKind::Inherited:
     return synthesizeInheritedGetterBody(getter, ctx);
@@ -1490,7 +1533,7 @@ synthesizeSetterBody(AccessorDecl *setter, ASTContext &ctx) {
     return synthesizeInheritedWithObserversSetterBody(setter, ctx);
 
   case WriteImplKind::Set:
-    llvm_unreachable("synthesizing setter for unknown reason?");  
+    return synthesizeInvalidAccessor(setter, ctx);
 
   case WriteImplKind::MutableAddress:
     return synthesizeMutableAddressSetterBody(setter, ctx);
@@ -1547,17 +1590,14 @@ synthesizeModifyCoroutineBody(AccessorDecl *modify, ASTContext &ctx) {
   return synthesizeCoroutineAccessorBody(modify, ctx);
 }
 
-std::pair<BraceStmt *, bool>
+static std::pair<BraceStmt *, bool>
 synthesizeAccessorBody(AbstractFunctionDecl *fn, void *) {
   auto *accessor = cast<AccessorDecl>(fn);
   auto &ctx = accessor->getASTContext();
 
   if (ctx.Stats)
     ctx.Stats->getFrontendCounters().NumAccessorBodiesSynthesized++;
 
-  if (accessor->isInvalid() || ctx.hadError())
-    return { nullptr, true };
-
   switch (accessor->getAccessorKind()) {
   case AccessorKind::Get:
     return synthesizeGetterBody(accessor, ctx);

test/attr/attr_objc.swift

@@ -849,17 +849,21 @@ class infer_instanceVar1 {
   var observingAccessorsVar1: Int {
   // CHECK: @_hasStorage @objc var observingAccessorsVar1: Int {
     willSet {}
-    // CHECK-NEXT: {{^}} @objc get
+    // CHECK-NEXT: {{^}} @objc get {
+    // CHECK-NEXT:    return
+    // CHECK-NEXT: }
     didSet {}
-    // CHECK-NEXT: {{^}} @objc set
+    // CHECK-NEXT: {{^}} @objc set {
   }
 
   @objc var observingAccessorsVar1_: Int {
   // CHECK: {{^}} @objc @_hasStorage var observingAccessorsVar1_: Int {
     willSet {}
-    // CHECK-NEXT: {{^}} @objc get
+    // CHECK-NEXT: {{^}} @objc get {
+    // CHECK-NEXT:   return
+    // CHECK-NEXT: }
     didSet {}
-    // CHECK-NEXT: {{^}} @objc set
+    // CHECK-NEXT: {{^}} @objc set {
   }
 
 

test/multifile/synthesized-accessors/invalid/Inputs/library.swift

@@ -0,0 +1,4 @@
+public struct SomeStruct {
+  var x: InvalidType
+}
+

test/multifile/synthesized-accessors/invalid/main.swift

@@ -0,0 +1,11 @@
+// RUN: not %target-swift-frontend -emit-silgen %S/Inputs/library.swift -primary-file %S/main.swift
+
+public func f(x: SomeStruct) {}
+
+public protocol P {
+  @_borrowed var x: Int { get set }
+}
+
+public struct HasAccessors : P {
+  public var x: Int = 123
+}