Merge pull request #69600 from tshortli/module-interface-lazy-typecheck-has-storage

AST: Trigger HasStorageRequest when retrieving semantic attributes

Author: tshortli

include/swift/AST/Decl.h

@@ -5374,6 +5374,7 @@ class BuiltinTupleDecl final : public NominalTypeDecl {
 /// AbstractStorageDecl - This is the common superclass for VarDecl and
 /// SubscriptDecl, representing potentially settable memory locations.
 class AbstractStorageDecl : public ValueDecl {
+  friend class HasStorageRequest;
   friend class SetterAccessLevelRequest;
   friend class IsGetterMutatingRequest;
   friend class IsSetterMutatingRequest;
@@ -5440,6 +5441,8 @@ class AbstractStorageDecl : public ValueDecl {
   llvm::PointerIntPair<AccessorRecord*, 3, OptionalEnum<AccessLevel>> Accessors;
 
   struct {
+    unsigned HasStorageComputed : 1;
+    unsigned HasStorage : 1;
     unsigned IsGetterMutatingComputed : 1;
     unsigned IsGetterMutating : 1;
     unsigned IsSetterMutatingComputed : 1;
@@ -5509,7 +5512,6 @@ class AbstractStorageDecl : public ValueDecl {
     return getImplInfo().getReadWriteImpl();
   }
 
-
   /// Return true if this is a VarDecl that has storage associated with
   /// it.
   bool hasStorage() const;

include/swift/AST/TypeCheckRequests.h

@@ -1779,10 +1779,9 @@ class MemberwiseInitPropertiesRequest :
   bool isCached() const { return true; }
 };
 
-class HasStorageRequest :
-    public SimpleRequest<HasStorageRequest,
-                         bool(AbstractStorageDecl *),
-                         RequestFlags::Cached> {
+class HasStorageRequest
+    : public SimpleRequest<HasStorageRequest, bool(AbstractStorageDecl *),
+                           RequestFlags::SeparatelyCached> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -1793,7 +1792,10 @@ class HasStorageRequest :
   bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
 
 public:
+  // Separate caching.
   bool isCached() const { return true; }
+  llvm::Optional<bool> getCachedResult() const;
+  void cacheResult(bool hasStorage) const;
 };
 
 class StorageImplInfoRequest :

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -311,7 +311,7 @@ SWIFT_REQUEST(TypeChecker, StorageImplInfoRequest,
               StorageImplInfo(AbstractStorageDecl *), SeparatelyCached,
               NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, HasStorageRequest,
-              bool(AbstractStorageDecl *), Cached,
+              bool(AbstractStorageDecl *), SeparatelyCached,
               NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, StoredPropertiesAndMissingMembersRequest,
               ArrayRef<Decl *>(NominalTypeDecl *), Cached, NoLocationInfo)

lib/AST/Decl.cpp

@@ -6743,13 +6743,11 @@ void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) {
   cacheImplInfo(implInfo);
 
   if (isImplicit()) {
-    auto &evaluator = getASTContext().evaluator;
-    HasStorageRequest request{this};
-    if (!evaluator.hasCachedResult(request))
-      evaluator.cacheOutput(request, implInfo.hasStorage());
-    else {
-      assert(
-        evaluateOrDefault(evaluator, request, false) == implInfo.hasStorage());
+    if (!LazySemanticInfo.HasStorageComputed) {
+      LazySemanticInfo.HasStorageComputed = true;
+      LazySemanticInfo.HasStorage = implInfo.hasStorage();
+    } else {
+      assert(LazySemanticInfo.HasStorage == implInfo.hasStorage());
     }
   }
 }

lib/AST/TypeCheckRequests.cpp

@@ -2077,7 +2077,10 @@ DeclAttributes SemanticDeclAttrsRequest::evaluate(Evaluator &evaluator,
   // Trigger requests that cause additional semantic attributes to be added.
   if (auto afd = dyn_cast<AbstractFunctionDecl>(decl)) {
     (void)afd->isTransparent();
+  } else if (auto asd = dyn_cast<AbstractStorageDecl>(decl)) {
+    (void)asd->hasStorage();
   }
+
   return decl->getAttrs();
 }
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2331,11 +2331,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       this->visitBoundVariable(var);
     });
 
-    // Add the '@_hasStorage' attribute if this property is stored.
-    if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
-      VD->getAttrs().add(new (getASTContext())
-                             HasStorageAttr(/*isImplicit=*/true));
-
     // Reject cases where this is a variable that has storage but it isn't
     // allowed.
     if (VD->hasStorage()) {

lib/Sema/TypeCheckStorage.cpp

@@ -3585,6 +3585,29 @@ bool HasStorageRequest::evaluate(Evaluator &evaluator,
   return hasStorage;
 }
 
+llvm::Optional<bool> HasStorageRequest::getCachedResult() const {
+  AbstractStorageDecl *decl = std::get<0>(getStorage());
+  if (decl->LazySemanticInfo.HasStorageComputed)
+    return static_cast<bool>(decl->LazySemanticInfo.HasStorage);
+  return llvm::None;
+}
+
+void HasStorageRequest::cacheResult(bool hasStorage) const {
+  AbstractStorageDecl *decl = std::get<0>(getStorage());
+  decl->LazySemanticInfo.HasStorageComputed = true;
+  decl->LazySemanticInfo.HasStorage = hasStorage;
+
+  // Add an attribute for printing, but only to VarDecls.
+  if (isa<ParamDecl>(decl))
+    return;
+  
+  if (auto varDecl = dyn_cast<VarDecl>(decl)) {
+    if (hasStorage && !varDecl->getAttrs().hasAttribute<HasStorageAttr>())
+      varDecl->getAttrs().add(new (varDecl->getASTContext())
+                              HasStorageAttr(/*isImplicit=*/true));
+  }
+}
+
 StorageImplInfo
 StorageImplInfoRequest::evaluate(Evaluator &evaluator,
                                  AbstractStorageDecl *storage) const {
@@ -3598,7 +3621,7 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
   if (auto *var = dyn_cast<VarDecl>(storage)) {
     // Allow the @_hasStorage attribute to override all the accessors we parsed
     // when making the final classification.
-    if (var->getAttrs().hasAttribute<HasStorageAttr>()) {
+    if (var->getParsedAttrs().hasAttribute<HasStorageAttr>()) {
       // The SIL rules for @_hasStorage are slightly different from the non-SIL
       // rules. In SIL mode, @_hasStorage marks that the type is simply stored,
       // and the only thing that determines mutability is the existence of the

test/Inputs/lazy_typecheck.swift

@@ -167,6 +167,14 @@ public struct PublicGenericStruct<T> {
   }
 }
 
+@frozen public struct FrozenPublicStruct {
+  private(set) var varWithPrivateSetter: Int = 1
+
+  public init(_ varWithPrivateSetter: Int) {
+    self.varWithPrivateSetter = varWithPrivateSetter
+  }
+}
+
 struct InternalStruct: NoTypecheckProto {
   var x: NoTypecheck
 

test/Inputs/lazy_typecheck_client.swift

@@ -32,7 +32,7 @@ func testGobalVars() {
   let _: (Int, Int) = (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY)
 }
 
-func testPublicStruct() {
+func testPublicStructs() {
   let s = PublicStruct(x: 1)
   let _: Int = s.publicMethod()
   let _: Int = s.publicProperty
@@ -42,6 +42,8 @@ func testPublicStruct() {
   let _: Int = s.publicTransparentProperty
   PublicStruct.publicStaticMethod()
   PublicStruct.activeMethod()
+
+  let _ = FrozenPublicStruct(1)
 }
 
 func testPublicClasses() {

test/attr/attr_objc.swift

@@ -983,7 +983,7 @@ class infer_instanceVar1 {
 
   @objc // access-note-move{{infer_instanceVar1.observingAccessorsVar1_}}
   var observingAccessorsVar1_: Int {
-  // CHECK: {{^}} @objc @_hasStorage var observingAccessorsVar1_: Int {
+  // CHECK: {{^}} {{@_hasStorage @objc|@objc @_hasStorage}} var observingAccessorsVar1_: Int {
     willSet {}
     // CHECK-NEXT: {{^}} @objc get {
     // CHECK-NEXT:   return