Merge pull request #23074 from aschwaighofer/private_import_internal_storage_private_accessor

Serialization: Also serialize the filename for internal storage decls with private accessors

Author: aschwaighofer

include/swift/AST/Decl.h

@@ -4549,6 +4549,9 @@ class AbstractStorageDecl : public ValueDecl {
   /// other modules.
   bool exportsPropertyDescriptor() const;
 
+  /// True if any of the accessors to the storage is private or fileprivate.
+  bool hasPrivateAccessor() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() >= DeclKind::First_AbstractStorageDecl &&

lib/AST/Decl.cpp

@@ -4419,6 +4419,14 @@ void AbstractStorageDecl::overwriteImplInfo(StorageImplInfo implInfo) {
   Accessors.getPointer()->overwriteImplInfo(implInfo);
 }
 
+bool AbstractStorageDecl::hasPrivateAccessor() const {
+  for (auto accessor : getAllAccessors()) {
+    if (hasPrivateOrFilePrivateFormalAccess(accessor))
+      return true;
+  }
+  return false;
+}
+
 void AbstractStorageDecl::setAccessors(StorageImplInfo implInfo,
                                        SourceLoc lbraceLoc,
                                        ArrayRef<AccessorDecl *> accessors,

lib/Serialization/Serialization.cpp

@@ -2693,19 +2693,37 @@ void Serializer::writeDecl(const Decl *D) {
   }
 
   if (auto *value = dyn_cast<ValueDecl>(D)) {
-    if (value->getFormalAccess() <= swift::AccessLevel::FilePrivate &&
-        !value->getDeclContext()->isLocalContext()) {
+    auto *storage = dyn_cast<AbstractStorageDecl>(value);
+    auto access = value->getFormalAccess();
+    // Emit the private descriminator for private decls.
+    // FIXME: We shouldn't need to encode this for /all/ private decls.
+    // In theory we can follow the same rules as mangling and only include
+    // the outermost private context.
+    bool shouldEmitPrivateDescriminator =
+        access <= swift::AccessLevel::FilePrivate &&
+        !value->getDeclContext()->isLocalContext();
+
+    // Emit the the filename for private mapping for private decls and
+    // decls with private accessors if compiled with -enable-private-imports.
+    bool shouldEmitFilenameForPrivate =
+        M->arePrivateImportsEnabled() &&
+        !value->getDeclContext()->isLocalContext() &&
+        (access <= swift::AccessLevel::FilePrivate ||
+         (storage &&
+          storage->getFormalAccess() >= swift::AccessLevel::Internal &&
+          storage->hasPrivateAccessor()));
+
+    if (shouldEmitFilenameForPrivate || shouldEmitPrivateDescriminator) {
       auto topLevelContext = value->getDeclContext()->getModuleScopeContext();
       if (auto *enclosingFile = dyn_cast<FileUnit>(topLevelContext)) {
-        // FIXME: We shouldn't need to encode this for /all/ private decls.
-        // In theory we can follow the same rules as mangling and only include
-        // the outermost private context.
-        Identifier discriminator =
-          enclosingFile->getDiscriminatorForPrivateValue(value);
-        unsigned abbrCode =
-          DeclTypeAbbrCodes[PrivateDiscriminatorLayout::Code];
-        PrivateDiscriminatorLayout::emitRecord(Out, ScratchRecord, abbrCode,
-                                        addDeclBaseNameRef(discriminator));
+        if (shouldEmitPrivateDescriminator) {
+          Identifier discriminator =
+              enclosingFile->getDiscriminatorForPrivateValue(value);
+          unsigned abbrCode =
+              DeclTypeAbbrCodes[PrivateDiscriminatorLayout::Code];
+          PrivateDiscriminatorLayout::emitRecord(
+              Out, ScratchRecord, abbrCode, addDeclBaseNameRef(discriminator));
+        }
         auto getFilename = [](FileUnit *enclosingFile,
                               const ValueDecl *decl) -> StringRef {
           if (auto *SF = dyn_cast<SourceFile>(enclosingFile)) {
@@ -2715,8 +2733,7 @@ void Serializer::writeDecl(const Decl *D) {
           }
           return StringRef();
         };
-        // Only if compiled with -enable-private-imports.
-        if (M->arePrivateImportsEnabled()) {
+        if (shouldEmitFilenameForPrivate) {
           auto filename = getFilename(enclosingFile, value);
           if (!filename.empty()) {
             auto filenameID = addFilename(filename);

test/Serialization/Inputs/private_import_other.swift

@@ -8,3 +8,10 @@ struct Value {
 extension Value {
   fileprivate func foo() {}
 }
+
+struct Internal {
+    private(set) var internalVarWithPrivateSetter : Int = 0
+    fileprivate(set) var internalVarWithFilePrivateSetter : Int = 0
+    public private(set) var publicVarWithPrivateSetter : Int = 0
+    public fileprivate(set) var publicVarWithFilePrivateSetter : Int = 0
+}

test/Serialization/private_import.swift

@@ -41,6 +41,15 @@
   @_private(sourceFile: "private_import_other.swift") import private_import
   @_private(sourceFile: "private_import.swift") import client
 
+  extension Internal {
+    mutating func set() {
+      self.internalVarWithPrivateSetter = 1
+      self.internalVarWithFilePrivateSetter = 1
+      self.publicVarWithPrivateSetter = 1
+      self.publicVarWithFilePrivateSetter = 1
+    }
+  }
+
   Base().foo()
   // This should not be ambigious.
   Base().bar()