[DNM][Serialization] Preserve indirect conformances knowledge with safety

include/swift/Basic/LangOptions.h

@@ -387,8 +387,7 @@ namespace swift {
 
     /// Enable early skipping deserialization of decls that are marked as
     /// unsafe to read.
-    bool EnableDeserializationSafety =
-      ::getenv("SWIFT_ENABLE_DESERIALIZATION_SAFETY");
+    bool EnableDeserializationSafety = true;
 
     /// Whether to enable the new operator decl and precedencegroup lookup
     /// behavior. This is a staging flag, and will be removed in the future.

lib/Frontend/CompilerInvocation.cpp

@@ -563,6 +563,17 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
                                OPT_disable_deserialization_safety)) {
     Opts.EnableDeserializationSafety
       = A->getOption().matches(OPT_enable_deserialization_safety);
+  } else if (auto A = Args.getLastArg(OPT_enable_access_control,
+                                      OPT_disable_access_control)) {
+    // Disable deserialization safety along with access control.
+    Opts.EnableDeserializationSafety
+      = A->getOption().matches(OPT_enable_access_control);
+  }
+
+  if (auto A = Args.getLastArg(OPT_enable_access_control,
+                               OPT_disable_access_control)) {
+    Opts.EnableAccessControl
+      = A->getOption().matches(OPT_enable_access_control);
   }
 
   // Whether '/.../' regex literals are enabled. This implies experimental
@@ -616,12 +627,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
       Args.hasArg(OPT_warn_on_potentially_unavailable_enum_case);
   Opts.WarnOnEditorPlaceholder |= Args.hasArg(OPT_warn_on_editor_placeholder);
 
-  if (auto A = Args.getLastArg(OPT_enable_access_control,
-                               OPT_disable_access_control)) {
-    Opts.EnableAccessControl
-      = A->getOption().matches(OPT_enable_access_control);
-  }
-
   if (auto A = Args.getLastArg(OPT_disable_typo_correction,
                                OPT_typo_correction_limit)) {
     if (A->getOption().matches(OPT_disable_typo_correction))

lib/IDE/IDETypeChecking.cpp

@@ -40,11 +40,20 @@ swift::getTopLevelDeclsForDisplay(ModuleDecl *M,
   auto startingSize = Results.size();
   M->getDisplayDecls(Results, Recursive);
 
-  // Force Sendable on all types, which might synthesize some extensions.
+  // Force Sendable on all public types, which might synthesize some extensions.
   // FIXME: We can remove this if @_nonSendable stops creating extensions.
   for (auto result : Results) {
-    if (auto NTD = dyn_cast<NominalTypeDecl>(result))
+    if (auto NTD = dyn_cast<NominalTypeDecl>(result)) {
+
+      // Restrict this logic to public and package types. Non-public types
+      // may refer to implementation details and fail at deserialization.
+      auto accessScope = NTD->getFormalAccessScope();
+      if (!M->isMainModule() &&
+          !accessScope.isPublic() && !accessScope.isPackage())
+        continue;
+
       (void)swift::isSendableType(M, NTD->getDeclaredInterfaceType());
+    }
   }
 
   // Remove what we fetched and fetch again, possibly now with additional

lib/Serialization/Serialization.cpp

@@ -3124,14 +3124,11 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       if (hasSafeMembers)
         return true;
 
-      // We can mark the extension unsafe only if it has no  public
+      // We can mark the extension unsafe only if it has no public
       // conformances.
       auto protocols = ext->getLocalProtocols(
                                         ConformanceLookupKind::OnlyExplicit);
-      bool hasSafeConformances = std::any_of(protocols.begin(),
-                                             protocols.end(),
-                                             isDeserializationSafe);
-      if (hasSafeConformances)
+      if (!protocols.empty())
         return true;
 
       // Truly empty extensions are safe, it may happen in swiftinterfaces.
@@ -3141,12 +3138,15 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       return false;
     }
 
+    if (isa<ProtocolDecl>(decl))
+      return true;
+
     auto value = cast<ValueDecl>(decl);
 
     // A decl is safe if formally accessible publicly.
     auto accessScope = value->getFormalAccessScope(/*useDC=*/nullptr,
                        /*treatUsableFromInlineAsPublic=*/true);
-    if (accessScope.isPublic())
+    if (accessScope.isPublic() || accessScope.isPackage())
       return true;
 
     if (auto accessor = dyn_cast<AccessorDecl>(value))

test/IDE/print_stdlib.swift

@@ -72,3 +72,8 @@
 // CHECK-GROUPS1-DAG: Collection/Type-erased
 // CHECK-GROUPS1-NOT: <NULL>
 // CHECK-GROUPS1: Module groups end.
+
+/// Check that we can still print the interface of the stdlib with
+/// deserialization safety enabled.
+// RUN: %target-swift-ide-test -print-module-groups -module-to-print=Swift -source-filename %s -print-interface -enable-deserialization-safety > %t-group.txt
+// RUN: %FileCheck -check-prefix=CHECK-GROUPS1 %s < %t-group.txt

test/Sema/package-only-references.swift

@@ -17,6 +17,11 @@
 // RUN:   -package-name MyPackage -I %t \
 // RUN:   -enable-experimental-feature AccessLevelOnImport
 
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -package-name MyPackage -I %t \
+// RUN:   -enable-deserialization-safety \
+// RUN:   -enable-experimental-feature AccessLevelOnImport
+
 /// A client outside of the package raises errors.
 // RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
 // RUN:   -package-name NotMyPackage -I %t \

test/Serialization/Safety/indirect-conformance.swift

@@ -0,0 +1,52 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// REQUIRES: asserts
+
+//--- Lib.swift
+
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
+// RUN:   -enable-library-evolution -swift-version 5 \
+// RUN:   -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -emit-module-interface-path %t/Lib.swiftinterface
+
+// RUN: cat %t/Lib.swiftinterface | %FileCheck %t/Lib.swift
+
+public protocol PublicProtocol : AnyObject {}
+// CHECK: public protocol PublicProtocol : AnyObject
+
+protocol InternalProtocol: PublicProtocol {}
+// CHECK-NOT: InternalProtocol
+
+public class IndirectConformant {
+    public init() {}
+}
+extension IndirectConformant: InternalProtocol {}
+// CHECK: extension Lib.IndirectConformant : Lib.PublicProtocol {}
+
+//--- Client.swift
+
+/// Works without safety.
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t
+
+/// Works with safety.
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -enable-deserialization-safety \
+// RUN:   -Xllvm -debug-only=Serialization 2>&1 \
+// RUN:   | %FileCheck %t/Client.swift
+
+/// Works with swiftinterface.
+// RUN: rm %t/Lib.swiftmodule
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t
+
+import Lib
+
+func requireConformanceToPublicProtocol(_ a: PublicProtocol) {}
+requireConformanceToPublicProtocol(IndirectConformant())
+
+/// Deserialization safety should keep the original chain. We're mostly
+/// documenting the current safety implementation details here, if we can get
+/// without deserializing 'InternalProtocol' it would be even better.
+// CHECK: Deserialized: 'IndirectConformant'
+// CHECK: Deserialized: 'PublicProtocol'
+// CHECK: Deserialized: 'InternalProtocol'

test/Serialization/Safety/skip-reading-internal-details.swift

@@ -18,6 +18,11 @@
 // RUN:   -disable-deserialization-safety 2>&1 \
 // RUN:   | %FileCheck --check-prefixes=NEEDED,UNSAFE %s
 
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
+// RUN:   -verify -Xllvm -debug-only=Serialization \
+// RUN:   -disable-access-control 2>&1 \
+// RUN:   | %FileCheck --check-prefixes=NEEDED,UNSAFE %s
+
 // RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
 // RUN:   -verify -Xllvm -debug-only=Serialization \
 // RUN:   -enable-deserialization-safety 2>&1 \

test/Serialization/Safety/unsafe-decls.swift

@@ -34,12 +34,12 @@
 public protocol PublicProto {}
 // SAFETY-PRIVATE: Serialization safety, safe: 'PublicProto'
 internal protocol InternalProto {}
-// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
+// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
 // NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
 private protocol PrivateProto {}
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
+// SAFETY-PRIVATE: Serialization safety, safe: 'PrivateProto'
 fileprivate protocol FileprivateProto {}
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
+// SAFETY-PRIVATE: Serialization safety, safe: 'FileprivateProto'
 
 internal struct InternalStruct : PublicProto {
 // SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalStruct'

test/Serialization/Safety/unsafe-extensions.swift

@@ -47,7 +47,7 @@ extension ExtendedPublic : PublicProto {
 
 /// Internal
 internal protocol InternalProto {}
-// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
+// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
 // NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
 internal struct ExtendedInternal {}
 // SAFETY-INTERNAL: Serialization safety, unsafe: 'ExtendedInternal'
@@ -63,7 +63,7 @@ extension ExtendedInternal : InternalProto {}
 
 /// Private
 private protocol PrivateProto {}
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
+// SAFETY-PRIVATE: Serialization safety, safe: 'PrivateProto'
 private struct ExtendedPrivate {}
 // SAFETY-PRIVATE: Serialization safety, unsafe: 'ExtendedPrivate'
 extension ExtendedPrivate {
@@ -75,7 +75,7 @@ extension ExtendedPrivate : PrivateProto {}
 
 /// Fileprivate
 private protocol FileprivateProto {}
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
+// SAFETY-PRIVATE: Serialization safety, safe: 'FileprivateProto'
 private struct ExtendedFileprivate {}
 // SAFETY-PRIVATE: Serialization safety, unsafe: 'ExtendedFileprivate'
 extension ExtendedFileprivate {
@@ -87,14 +87,14 @@ extension ExtendedFileprivate : FileprivateProto {}
 
 /// Back to public
 extension ExtendedPublic : InternalProto {
-// SAFETY-INTERNAL: Serialization safety, unsafe: 'extension ExtendedPublic'
+// SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
 // NO-SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
 }
 extension ExtendedPublic : PrivateProto {
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
+// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
 }
 extension ExtendedPublic : FileprivateProto {
-// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
+// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
 }
 
 extension ExtendedPublic {

tools/swift-ide-test/swift-ide-test.cpp

@@ -397,6 +397,11 @@ DisableAccessControl("disable-access-control",
     llvm::cl::desc("Disables access control, like a debugger"),
     llvm::cl::cat(Category));
 
+static llvm::cl::opt<bool>
+EnableDeserializationSafety("enable-deserialization-safety",
+    llvm::cl::desc("Avoid reading potentially unsafe decls from swiftmodules"),
+    llvm::cl::cat(Category));
+
 static llvm::cl::opt<bool> CodeCompleteInitsInPostfixExpr(
     "code-complete-inits-in-postfix-expr",
     llvm::cl::desc(
@@ -4447,6 +4452,8 @@ int main(int argc, char *argv[]) {
     options::ImportObjCHeader;
   InitInvok.getLangOptions().EnableAccessControl =
     !options::DisableAccessControl;
+  InitInvok.getLangOptions().EnableDeserializationSafety =
+    options::EnableDeserializationSafety;
   InitInvok.getLangOptions().EnableSwift3ObjCInference =
     options::EnableSwift3ObjCInference;
   InitInvok.getClangImporterOptions().ImportForwardDeclarations |=