[5.1] Serialization: Also serialize the filename for storage decls with private accessors

If a value decl is internal hasTestableOrPrivateImport will succeed (or
fail) without looking at the filename. However this breaks when we query
an internal storage decl with private formal access for a private
setter: the query would fail because no filename was serialized for the
decl (we only serialize filenames for private decls). So in the special
case of storage with private accessor also serialize the
filename.

rdar://48516545

Author: aschwaighofer

include/swift/AST/Decl.h

@@ -4621,6 +4621,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

@@ -4435,6 +4435,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

@@ -2696,19 +2696,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)) {
@@ -2718,8 +2736,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()