Sema: Move finishStorageImplInfo() to StorageImplInfoRequest evaluation

This merges Sema's special logic for updating the ImplInfo of lazy properties
and property wrappers with the StorageImplInfoRequest.

I had to move some code from the parser into the StorageImplInfoRequest, to
avoid request cycles caused by hasStorage() calls; this is the right thing to
do anyway.

Since hasStorage() now answers false for lazy properties and property wrappers,
we have to move some diagnostic checks from checkDeclAttributesEarly() to
the StorageImplInfoRequest implementation itself.

Over time, I expect all of the checks currently in checkDeclAttributesEarly()
and checkDeclAttributes() to either migrate to requests, or typeCheckDecl().

Author: slavapestov

include/swift/AST/DiagnosticsParse.def

@@ -304,8 +304,6 @@ ERROR(observing_accessor_conflicts_with_accessor,none,
       (unsigned, StringRef))
 ERROR(observing_accessor_in_subscript,none,
       "%select{'willSet'|'didSet'}0 is not allowed in subscripts", (unsigned))
-ERROR(getset_init,none,
-      "variable with getter/setter cannot have an initial value", ())
 ERROR(getset_cannot_be_implied,none,
       "variable with implied type cannot have implied getter/setter", ())
 

include/swift/AST/DiagnosticsSema.def

@@ -1377,6 +1377,9 @@ WARNING(enum_frozen_nonpublic,none,
         "%0 has no effect on non-public enums", (DeclAttribute))
 
 // Variables (var and let).
+ERROR(getset_init,none,
+      "variable with getter/setter cannot have an initial value", ())
+
 ERROR(unimplemented_static_var,none,
       "%select{ERROR|static|class}1 stored properties not supported"
       "%select{ in this context| in generic types| in classes| in protocol extensions}0"

lib/Parse/ParseDecl.cpp

@@ -5282,12 +5282,6 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
                              PatternInit != nullptr, Attributes, Decls);
       if (boundVar.hasCodeCompletion())
         return makeResult(makeParserCodeCompletionStatus());
-      if (PatternInit && boundVar.isNonNull() &&
-          !boundVar.get()->hasStorage()) {
-        diagnose(pattern->getLoc(), diag::getset_init)
-            .highlight(PatternInit->getSourceRange());
-        PatternInit = nullptr;
-      }
     }
 
     // Add all parsed vardecls to this scope.

lib/Sema/CodeSynthesis.cpp

@@ -2025,31 +2025,19 @@ void swift::triggerAccessorSynthesis(TypeChecker &TC,
   });
 }
 
-static StorageImplInfo getProtocolStorageImpl(AbstractStorageDecl *storage) {
-  auto protocol = cast<ProtocolDecl>(storage->getDeclContext());
-  if (protocol->isObjC()) {
-    return StorageImplInfo::getComputed(storage->supportsMutation());
-  } else {
-    return StorageImplInfo::getOpaque(storage->supportsMutation(),
-                                      storage->getOpaqueReadOwnership());
-  }
-}
-
 /// Given a storage declaration in a protocol, set it up with the right
 /// StorageImpl and add the right set of opaque accessors.
-static void finishProtocolStorageImplInfo(AbstractStorageDecl *storage) {
+static void finishProtocolStorageImplInfo(AbstractStorageDecl *storage,
+                                          StorageImplInfo &info) {
   if (auto *var = dyn_cast<VarDecl>(storage)) {
-    if (var->hasStorage()) {
-      auto &ctx = var->getASTContext();
+    if (info.hasStorage()) {
       // Protocols cannot have stored properties.
       if (var->isLet()) {
-        ctx.Diags.diagnose(var->getLoc(),
-                           diag::protocol_property_must_be_computed_var)
+        var->diagnose(diag::protocol_property_must_be_computed_var)
           .fixItReplace(var->getParentPatternBinding()->getLoc(), "var")
           .fixItInsertAfter(var->getTypeLoc().getLoc(), " { get }");
       } else {
-        auto diag = ctx.Diags.diagnose(var->getLoc(),
-                                       diag::protocol_property_must_be_computed);
+        auto diag = var->diagnose(diag::protocol_property_must_be_computed);
         auto braces = var->getBracesRange();
 
         if (braces.isValid())
@@ -2060,15 +2048,59 @@ static void finishProtocolStorageImplInfo(AbstractStorageDecl *storage) {
     }
   }
 
-  storage->setImplInfo(getProtocolStorageImpl(storage));
+  auto protocol = cast<ProtocolDecl>(storage->getDeclContext());
+  if (protocol->isObjC()) {
+    info = StorageImplInfo::getComputed(info.supportsMutation());
+  } else {
+    info = StorageImplInfo::getOpaque(info.supportsMutation(),
+                                      storage->getOpaqueReadOwnership());
+  }
 }
 
-static void finishLazyVariableImplInfo(VarDecl *var) {
-  // If there are already accessors, something is invalid; bail out.
-  if (!var->getImplInfo().isSimpleStored())
-    return;
+/// This emits a diagnostic with a fixit to remove the attribute.
+template<typename ...ArgTypes>
+void diagnoseAndRemoveAttr(Decl *D, DeclAttribute *attr,
+                           ArgTypes &&...Args) {
+  auto &ctx = D->getASTContext();
+  ctx.Diags.diagnose(attr->getLocation(), std::forward<ArgTypes>(Args)...)
+    .fixItRemove(attr->getRangeWithAt());
+}
+
+static void finishLazyVariableImplInfo(VarDecl *var,
+                                       StorageImplInfo &info) {
+  auto *attr = var->getAttrs().getAttribute<LazyAttr>();
+
+  // It cannot currently be used on let's since we don't have a mutability model
+  // that supports it.
+  if (var->isLet())
+    diagnoseAndRemoveAttr(var, attr, diag::lazy_not_on_let);
+
+  // lazy must have an initializer.
+  if (!var->getParentInitializer())
+    diagnoseAndRemoveAttr(var, attr, diag::lazy_requires_initializer);
+
+  bool invalid = false;
 
-  var->setImplInfo(StorageImplInfo::getMutableComputed());
+  if (isa<ProtocolDecl>(var->getDeclContext())) {
+    diagnoseAndRemoveAttr(var, attr, diag::lazy_not_in_protocol);
+    invalid = true;
+  }
+
+  // Lazy properties must be written as stored properties in the source.
+  if (!info.isSimpleStored()) {
+    diagnoseAndRemoveAttr(var, attr,
+                          info.hasStorage()
+                          ? diag::lazy_not_observable
+                          : diag::lazy_not_on_computed);
+    invalid = true;
+  }
+
+  // The pattern binding must only bind a single variable.
+  if (!var->getParentPatternBinding()->getSingleVar())
+    diagnoseAndRemoveAttr(var, attr, diag::lazy_requires_single_var);
+
+  if (!invalid)
+    info = StorageImplInfo::getMutableComputed();
 }
 
 /// Determine whether all of the wrapped-value setters for the property
@@ -2090,8 +2122,21 @@ static bool allPropertyWrapperValueSettersAreAccessible(VarDecl *var) {
   return true;
 }
 
-static void finishPropertyWrapperImplInfo(VarDecl *var) {
+static void finishPropertyWrapperImplInfo(VarDecl *var,
+                                          StorageImplInfo &info) {
   auto parentSF = var->getDeclContext()->getParentSourceFile();
+  if (!parentSF)
+    return;
+
+  // Properties with wrappers must not declare a getter or setter.
+  if (!info.hasStorage() && parentSF->Kind != SourceFileKind::Interface) {
+    auto &ctx = parentSF->getASTContext();
+    for (auto attr : var->getAttrs().getAttributes<CustomAttr>())
+      ctx.Diags.diagnose(attr->getLocation(), diag::property_wrapper_computed);
+
+    return;
+  }
+
   bool wrapperSetterIsUsable =
     var->getSetter() ||
     (parentSF &&
@@ -2100,36 +2145,68 @@ static void finishPropertyWrapperImplInfo(VarDecl *var) {
      allPropertyWrapperValueSettersAreAccessible(var));
 
   if (wrapperSetterIsUsable)
-    var->setImplInfo(StorageImplInfo::getMutableComputed());
+    info = StorageImplInfo::getMutableComputed();
   else
-    var->setImplInfo(StorageImplInfo::getImmutableComputed());
+    info = StorageImplInfo::getImmutableComputed();
 }
 
-static void finishNSManagedImplInfo(VarDecl *VD) {
-  // If it's not still stored, just bail out.
-  if (!VD->getImplInfo().isSimpleStored())
-    return;
+static void finishNSManagedImplInfo(VarDecl *var,
+                                    StorageImplInfo &info) {
+  auto *attr = var->getAttrs().getAttribute<NSManagedAttr>();
 
-  VD->setImplInfo(StorageImplInfo::getMutableComputed());
-}
+  if (var->isLet())
+    diagnoseAndRemoveAttr(var, attr, diag::attr_NSManaged_let_property);
 
-static void finishStorageImplInfo(AbstractStorageDecl *storage) {
-  if (isa<ProtocolDecl>(storage->getDeclContext())) {
-    finishProtocolStorageImplInfo(storage);
-    return;
+  auto diagnoseNotStored = [&](unsigned kind) {
+    diagnoseAndRemoveAttr(var, attr, diag::attr_NSManaged_not_stored, kind);
+  };
+
+  // @NSManaged properties must be written as stored.
+  if (info.isSimpleStored()) {
+    // @NSManaged properties end up being computed; complain if there is
+    // an initializer.
+    if (var->getParentInitializer()) {
+      auto &Diags = var->getASTContext().Diags;
+      Diags.diagnose(attr->getLocation(), diag::attr_NSManaged_initial_value)
+           .highlight(var->getParentInitializer()->getSourceRange());
+    }
+
+    // Otherwise, ok.
+    info = StorageImplInfo::getMutableComputed();
+
+  } else if (info.getReadImpl() == ReadImplKind::Address ||
+             info.getWriteImpl() == WriteImplKind::MutableAddress) {
+    diagnoseNotStored(/*addressed*/ 2);
+  } else if (info.getWriteImpl() == WriteImplKind::StoredWithObservers ||
+             info.getWriteImpl() == WriteImplKind::InheritedWithObservers) {
+    diagnoseNotStored(/*observing*/ 1);
+  } else {
+    diagnoseNotStored(/*computed*/ 0);
   }
+}
 
-  auto var = dyn_cast<VarDecl>(storage);
-  if (var == nullptr)
-    return;
+static void finishStorageImplInfo(AbstractStorageDecl *storage,
+                                  StorageImplInfo &info) {
+  if (auto var = dyn_cast<VarDecl>(storage)) {
+    if (!info.hasStorage()) {
+      if (auto *init = var->getParentInitializer()) {
+        auto &Diags = var->getASTContext().Diags;
+        Diags.diagnose(init->getLoc(), diag::getset_init)
+             .highlight(init->getSourceRange());
+      }
+    }
 
-  if (var->getAttrs().hasAttribute<LazyAttr>()) {
-    finishLazyVariableImplInfo(var);
-  } else if (var->getAttrs().hasAttribute<NSManagedAttr>()) {
-    finishNSManagedImplInfo(var);
-  } else if (var->hasAttachedPropertyWrapper()) {
-    finishPropertyWrapperImplInfo(var);
+    if (var->getAttrs().hasAttribute<LazyAttr>()) {
+      finishLazyVariableImplInfo(var, info);
+    } else if (var->getAttrs().hasAttribute<NSManagedAttr>()) {
+      finishNSManagedImplInfo(var, info);
+    } else if (var->hasAttachedPropertyWrapper()) {
+      finishPropertyWrapperImplInfo(var, info);
+    }
   }
+
+  if (isa<ProtocolDecl>(storage->getDeclContext()))
+    finishProtocolStorageImplInfo(storage, info);
 }
 
 static bool hasParsedAccessor(AbstractStorageDecl *storage, AccessorKind kind) {
@@ -2284,14 +2361,15 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
     readWriteImpl = ReadWriteImplKind::Immutable;
   }
 
-  return StorageImplInfo(readImpl, writeImpl, readWriteImpl);
+  StorageImplInfo info(readImpl, writeImpl, readWriteImpl);
+  finishStorageImplInfo(storage, info);
+
+  return info;
 }
 
 /// Try to add the appropriate accessors required a storage declaration.
 /// This needs to be idempotent.
 void swift::maybeAddAccessorsToStorage(AbstractStorageDecl *storage) {
-  finishStorageImplInfo(storage);
-
   // Implicit properties don't get accessors.
   if (storage->isImplicit() &&
       !(isa<VarDecl>(storage) &&

lib/Sema/TypeCheckAttr.cpp

@@ -501,36 +501,6 @@ void AttributeEarlyChecker::visitNSManagedAttr(NSManagedAttr *attr) {
   // Everything below deals with restrictions on @NSManaged properties.
   auto *VD = cast<VarDecl>(D);
 
-  if (VD->isLet())
-    diagnoseAndRemoveAttr(attr, diag::attr_NSManaged_let_property);
-
-  auto diagnoseNotStored = [&](unsigned kind) {
-    TC.diagnose(attr->getLocation(), diag::attr_NSManaged_not_stored, kind);
-    return attr->setInvalid();
-  };
-
-  // @NSManaged properties must be written as stored.
-  auto impl = VD->getImplInfo();
-  if (impl.isSimpleStored()) {
-    // @NSManaged properties end up being computed; complain if there is
-    // an initializer.
-    if (VD->getParentInitializer()) {
-      TC.diagnose(attr->getLocation(), diag::attr_NSManaged_initial_value)
-        .highlight(VD->getParentInitializer()->getSourceRange());
-      auto PBD = VD->getParentPatternBinding();
-      PBD->setInit(PBD->getPatternEntryIndexForVarDecl(VD), nullptr);
-    }
-    // Otherwise, ok.
-  } else if (impl.getReadImpl() == ReadImplKind::Address ||
-             impl.getWriteImpl() == WriteImplKind::MutableAddress) {
-    return diagnoseNotStored(/*addressed*/ 2);    
-  } else if (impl.getWriteImpl() == WriteImplKind::StoredWithObservers ||
-             impl.getWriteImpl() == WriteImplKind::InheritedWithObservers) {
-    return diagnoseNotStored(/*observing*/ 1);    
-  } else {
-    return diagnoseNotStored(/*computed*/ 0);
-  }
-
   // @NSManaged properties cannot be @NSCopying
   if (auto *NSCopy = VD->getAttrs().getAttribute<NSCopyingAttr>())
     diagnoseAndRemoveAttr(NSCopy, diag::attr_NSManaged_NSCopying);
@@ -562,21 +532,12 @@ void AttributeEarlyChecker::visitLazyAttr(LazyAttr *attr) {
   // lazy may only be used on properties.
   auto *VD = cast<VarDecl>(D);
 
-  // It cannot currently be used on let's since we don't have a mutability model
-  // that supports it.
-  if (VD->isLet())
-    diagnoseAndRemoveAttr(attr, diag::lazy_not_on_let);
-
   auto attrs = VD->getAttrs();
   // 'lazy' is not allowed to have reference attributes
   if (auto *refAttr = attrs.getAttribute<ReferenceOwnershipAttr>())
     diagnoseAndRemoveAttr(attr, diag::lazy_not_strong, refAttr->get());
 
-  // lazy is not allowed on a protocol requirement.
   auto varDC = VD->getDeclContext();
-  if (isa<ProtocolDecl>(varDC))
-    diagnoseAndRemoveAttr(attr, diag::lazy_not_in_protocol);
-
 
   // 'lazy' is not allowed on a global variable or on a static property (which
   // are already lazily initialized).
@@ -588,25 +549,6 @@ void AttributeEarlyChecker::visitLazyAttr(LazyAttr *attr) {
   } else if (!VD->getDeclContext()->isTypeContext()) {
     diagnoseAndRemoveAttr(attr, diag::lazy_must_be_property);
   }
-
-  // lazy must have an initializer, and the pattern binding must be a simple
-  // one.
-  if (!VD->getParentInitializer())
-    diagnoseAndRemoveAttr(attr, diag::lazy_requires_initializer);
-
-  if (!VD->getParentPatternBinding()->getSingleVar())
-    diagnoseAndRemoveAttr(attr, diag::lazy_requires_single_var);
-
-
-  // TODO: Lazy properties can't yet be observed.
-  auto impl = VD->getImplInfo();
-  if (impl.isSimpleStored()) {
-    // ok
-  } else if (VD->hasStorage()) {
-    diagnoseAndRemoveAttr(attr, diag::lazy_not_observable);
-  } else {
-    diagnoseAndRemoveAttr(attr, diag::lazy_not_on_computed);
-  }
 }
 
 bool AttributeEarlyChecker::visitAbstractAccessControlAttr(

lib/Sema/TypeCheckDecl.cpp

@@ -2224,6 +2224,15 @@ IsGetterMutatingRequest::evaluate(Evaluator &evaluator,
     }
   }
 
+  // Protocol requirements are always written as '{ get }' or '{ get set }';
+  // the @_borrowed attribute determines if getReadImpl() becomes Get or Read.
+  if (isa<ProtocolDecl>(storage->getDeclContext())) {
+    if (!storage->getGetter())
+      return false;
+
+    return storage->getGetter()->isMutating();
+  }
+
   switch (storage->getReadImpl()) {
   case ReadImplKind::Stored:
   case ReadImplKind::Inherited:

lib/Sema/TypeCheckPropertyWrapper.cpp

@@ -456,12 +456,6 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator,
         continue;
       }
     }
-
-    // Properties with wrappers must not declare a getter or setter.
-    if (!var->hasStorage() && sourceFile->Kind != SourceFileKind::Interface) {
-      ctx.Diags.diagnose(attr->getLocation(), diag::property_wrapper_computed);
-      continue;
-    }
 
     result.push_back(mutableAttr);
   }

test/decl/var/NSManaged_properties.swift

@@ -39,10 +39,8 @@ class SwiftGizmo : A {
   // expected-error@+1{{property cannot be marked @NSManaged because its type cannot be represented in Objective-C}}
   @NSManaged var nonobjc_var: SwiftProto?
 
-  // expected-error@+4 {{@NSManaged only allowed on an instance property or method}}
-  // expected-error@+3 {{@NSManaged property cannot have an initial value}}
-  // expected-error@+2 {{class stored properties not supported in classes; did you mean 'static'?}}
-  // expected-error@+1 {{'class var' declaration requires an initializer expression or getter/setter specifier}}
+  // expected-error@+2 {{@NSManaged only allowed on an instance property or method}}
+  // expected-error@+1 {{@NSManaged property cannot have an initial value}}
   @NSManaged class var d: Int = 4
 
   @NSManaged var e: Int { return 4 } // expected-error {{@NSManaged not allowed on computed properties}}