Merge pull request swiftlang#33575 from xymus/dont-print-ioi-extensions

xymus · web-flow · commit 1fdcdcea50b9 · 2020-08-24T10:39:09.000-07:00
[ModuleInterface] Don't print extensions to implementation-only imported types
diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h
@@ -685,6 +685,12 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   void
   getImportedModulesForLookup(SmallVectorImpl<ImportedModule> &imports) const;
 
+  /// Has \p module been imported via an '@_implementationOnly' import
+  /// instead of another kind of import?
+  ///
+  /// This assumes that \p module was imported.
+  bool isImportedImplementationOnly(const ModuleDecl *module) const;
+
   /// Uniques the items in \p imports, ignoring the source locations of the
   /// access paths.
   ///
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -169,6 +169,24 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(bool preferTypeRepr,
       if (auto *ED = dyn_cast<ExtensionDecl>(D)) {
         if (!shouldPrint(ED->getExtendedNominal(), options))
           return false;
+
+        // Skip extensions to implementation-only imported types that have
+        // no public members.
+        auto localModule = ED->getParentModule();
+        auto nominalModule = ED->getExtendedNominal()->getParentModule();
+        if (localModule != nominalModule &&
+            localModule->isImportedImplementationOnly(nominalModule)) {
+
+          bool shouldPrintMembers = llvm::any_of(
+                                      ED->getMembers(),
+                                      [&](const Decl *member) -> bool {
+            return shouldPrint(member, options);
+          });
+
+          if (!shouldPrintMembers)
+            return false;
+        }
+
         for (const Requirement &req : ED->getGenericRequirements()) {
           if (!isPublicOrUsableFromInline(req.getFirstType()))
             return false;
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -1994,6 +1994,27 @@ bool SourceFile::isImportedImplementationOnly(const ModuleDecl *module) const {
   return !imports.isImportedBy(module, getParentModule());
 }
 
+bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
+  auto &imports = getASTContext().getImportCache();
+
+  // Look through non-implementation-only imports to see if module is imported
+  // in some other way. Otherwise we assume it's implementation-only imported.
+  ModuleDecl::ImportFilter filter = {
+    ModuleDecl::ImportFilterKind::Public,
+    ModuleDecl::ImportFilterKind::Private,
+    ModuleDecl::ImportFilterKind::SPIAccessControl,
+    ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay};
+  SmallVector<ModuleDecl::ImportedModule, 4> results;
+  getImportedModules(results, filter);
+
+  for (auto &desc : results) {
+    if (imports.isImportedBy(module, desc.importedModule))
+      return false;
+  }
+
+  return true;
+}
+
 void SourceFile::lookupImportedSPIGroups(
                         const ModuleDecl *importedModule,
                         llvm::SmallSetVector<Identifier, 4> &spiGroups) const {
diff --git a/test/ModuleInterface/empty-extension.swift b/test/ModuleInterface/empty-extension.swift
@@ -0,0 +1,49 @@
+// Test that we don't print extensions to implementation-only imported types.
+
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -emit-module %s -DIOI_LIB -module-name IOILib -emit-module-path %t/IOILib.swiftmodule
+// RUN: %target-swift-frontend -emit-module %s -DEXPORTED_LIB -module-name IndirectLib -emit-module-path %t/IndirectLib.swiftmodule -I %t
+// RUN: %target-swift-frontend -emit-module %s -DLIB -module-name Lib -emit-module-path %t/Lib.swiftmodule -I %t
+
+// RUN: %target-swift-frontend-typecheck -emit-module-interface-path %t/out.swiftinterface %s -I %t -swift-version 5 -enable-library-evolution
+// RUN: %FileCheck %s < %t/out.swiftinterface
+
+#if IOI_LIB
+
+public struct IOIImportedType {
+  public func foo() {}
+}
+
+#elseif EXPORTED_LIB
+
+public struct ExportedType {
+  public func foo() {}
+}
+
+#elseif LIB
+
+@_exported import IndirectLib
+
+public struct NormalImportedType {
+  public func foo() {}
+}
+
+#else // Client
+
+import Lib
+@_implementationOnly import IOILib
+
+public protocol PublicProto {
+  func foo()
+}
+extension IOIImportedType : PublicProto {}
+// CHECK-NOT: IOIImportedType
+
+extension NormalImportedType : PublicProto {}
+// CHECK: extension NormalImportedType
+
+extension ExportedType : PublicProto {}
+// CHECK: extension ExportedType
+
+#endif
diff --git a/test/SPI/Inputs/ioi_helper.swift b/test/SPI/Inputs/ioi_helper.swift
@@ -0,0 +1 @@
+public struct IOIPublicStruct {}
diff --git a/test/SPI/experimental_spi_imports_swiftinterface.swift b/test/SPI/experimental_spi_imports_swiftinterface.swift
@@ -4,7 +4,7 @@
 
 /// Generate 3 empty modules.
 // RUN: touch %t/empty.swift
-// RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name ExperimentalImported -emit-module-path %t/ExperimentalImported.swiftmodule -swift-version 5 -enable-library-evolution
+// RUN: %target-swift-frontend -emit-module %S/Inputs/ioi_helper.swift -module-name ExperimentalImported -emit-module-path %t/ExperimentalImported.swiftmodule -swift-version 5 -enable-library-evolution
 // RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name IOIImported -emit-module-path %t/IOIImported.swiftmodule -swift-version 5 -enable-library-evolution
 // RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name SPIImported -emit-module-path %t/SPIImported.swiftmodule -swift-version 5 -enable-library-evolution
 
@@ -24,3 +24,10 @@
 @_spi(dummy) import SPIImported
 // CHECK-PUBLIC: {{^}}import SPIImported
 // CHECK-PRIVATE: @_spi{{.*}} import SPIImported
+
+@_spi(X)
+extension IOIPublicStruct {
+  public func foo() {}
+}
+// CHECK-PUBLIC-NOT: IOIPublicStruct
+// CHECK-PRIVATE: @_spi{{.*}} extension IOIPublicStruct
diff --git a/test/SPI/private_swiftinterface.swift b/test/SPI/private_swiftinterface.swift
@@ -3,6 +3,7 @@
 
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module %S/Inputs/spi_helper.swift -module-name SPIHelper -emit-module-path %t/SPIHelper.swiftmodule -swift-version 5 -enable-library-evolution -emit-module-interface-path %t/SPIHelper.swiftinterface -emit-private-module-interface-path %t/SPIHelper.private.swiftinterface
+// RUN: %target-swift-frontend -emit-module %S/Inputs/ioi_helper.swift -module-name IOIHelper -emit-module-path %t/IOIHelper.swiftmodule -swift-version 5 -enable-library-evolution -emit-module-interface-path %t/IOIHelper.swiftinterface -emit-private-module-interface-path %t/IOIHelper.private.swiftinterface
 
 /// Make sure that the public swiftinterface of spi_helper doesn't leak SPI.
 // RUN: %FileCheck -check-prefix=CHECK-HELPER %s < %t/SPIHelper.swiftinterface
@@ -25,6 +26,7 @@
 // CHECK-PUBLIC: import SPIHelper
 // CHECK-PRIVATE: @_spi(OtherSPI) @_spi(HelperSPI) import SPIHelper
 
+@_implementationOnly import IOIHelper
 public func foo() {}
 // CHECK-PUBLIC: foo()
 // CHECK-PRIVATE: foo()
@@ -155,6 +157,11 @@ extension PublicType: SPIProto2 where T: SPIProto2 {}
 // CHECK-PRIVATE: extension PublicType : {{.*}}.SPIProto2 where T : {{.*}}.SPIProto2
 // CHECK-PUBLIC-NOT: _ConstraintThatIsNotPartOfTheAPIOfThisLibrary
 
+public protocol LocalPublicProto {}
+extension IOIPublicStruct : LocalPublicProto {}
+// CHECK-PRIVATE-NOT: IOIPublicStruct
+// CHECK-PUBLIC-NOT: IOIPublicStruct
+
 // The dummy conformance should be only in the private swiftinterface for
 // SPI extensions.
 @_spi(LocalSPI)
