Merge pull request #33207 from xymus/spi-ioi-check-workaround

[ModuleInterface] Print some implementation-only imports in the private interface

Author: xymus

include/swift/AST/DiagnosticsSema.def

@@ -2712,6 +2712,12 @@ ERROR(decl_from_hidden_module,none,
       "it is an SPI imported from %3|"
       "it is SPI}4",
       (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
+WARNING(decl_from_hidden_module_warn,none,
+      "cannot use %0 %1 %select{in SPI|as property wrapper in SPI|"
+      "in an extension with public or '@usableFromInline' members|"
+      "in an extension with conditional conformances}2; "
+      "%select{%3 has been imported as implementation-only}4",
+      (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 %select{here|as property wrapper here|"
       "in an extension with public or '@usableFromInline' members|"

include/swift/Frontend/ModuleInterfaceSupport.h

@@ -43,6 +43,10 @@ struct ModuleInterfaceOptions {
 
   // Print SPI decls and attributes.
   bool PrintSPIs = false;
+
+  /// Print imports with both @_implementationOnly and @_spi, only applies
+  /// when PrintSPIs is true.
+  bool ExperimentalSPIImports = false;
 };
 
 extern version::Version InterfaceFormatVersion;

include/swift/Option/FrontendOptions.td

@@ -643,6 +643,10 @@ def module_interface_preserve_types_as_written :
   HelpText<"When emitting a module interface, preserve types as they were "
            "written in the source">;
 
+def experimental_spi_imports :
+  Flag<["-"], "experimental-spi-imports">,
+  HelpText<"Enable experimental support for SPI imports">;
+
 def experimental_print_full_convention :
  Flag<["-"], "experimental-print-full-convention">,
  HelpText<"When emitting a module interface, emit additional @convention "

lib/Frontend/CompilerInvocation.cpp

@@ -313,6 +313,8 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
     Args.hasArg(OPT_module_interface_preserve_types_as_written);
   Opts.PrintFullConvention |=
     Args.hasArg(OPT_experimental_print_full_convention);
+  Opts.ExperimentalSPIImports |=
+    Args.hasArg(OPT_experimental_spi_imports);
 }
 
 /// Save a copy of any flags marked as ModuleInterfaceOption, if running

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -100,11 +100,28 @@ static void printImports(raw_ostream &out,
                          ModuleDecl *M) {
   // FIXME: This is very similar to what's in Serializer::writeInputBlock, but
   // it's not obvious what higher-level optimization would be factored out here.
+  ModuleDecl::ImportFilter allImportFilter;
+  allImportFilter |= ModuleDecl::ImportFilterKind::Public;
+  allImportFilter |= ModuleDecl::ImportFilterKind::Private;
+  allImportFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
+
+  // With -experimental-spi-imports:
+  // When printing the private swiftinterface file, print implementation-only
+  // imports only if they are also SPI. First, list all implementation-only
+  // imports and filter them later.
+  llvm::SmallSet<ModuleDecl::ImportedModule, 4,
+                 ModuleDecl::OrderImportedModules> ioiImportSet;
+  if (Opts.PrintSPIs && Opts.ExperimentalSPIImports) {
+    allImportFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
+
+    SmallVector<ModuleDecl::ImportedModule, 4> ioiImport;
+    M->getImportedModules(ioiImport,
+                          ModuleDecl::ImportFilterKind::ImplementationOnly);
+    ioiImportSet.insert(ioiImport.begin(), ioiImport.end());
+  }
+
   SmallVector<ModuleDecl::ImportedModule, 8> allImports;
-  M->getImportedModules(allImports,
-                        {ModuleDecl::ImportFilterKind::Public,
-                         ModuleDecl::ImportFilterKind::Private,
-                         ModuleDecl::ImportFilterKind::SPIAccessControl});
+  M->getImportedModules(allImports, allImportFilter);
   ModuleDecl::removeDuplicateImports(allImports);
   diagnoseScopedImports(M->getASTContext().Diags, allImports);
 
@@ -124,13 +141,21 @@ static void printImports(raw_ostream &out,
       continue;
     }
 
+    llvm::SmallSetVector<Identifier, 4> spis;
+    M->lookupImportedSPIGroups(importedModule, spis);
+
+    // Only print implementation-only imports which have an SPI import.
+    if (ioiImportSet.count(import)) {
+      if (spis.empty())
+        continue;
+      out << "@_implementationOnly ";
+    }
+
     if (publicImportSet.count(import))
       out << "@_exported ";
 
     // SPI attribute on imports
     if (Opts.PrintSPIs) {
-      llvm::SmallSetVector<Identifier, 4> spis;
-      M->lookupImportedSPIGroups(importedModule, spis);
       for (auto spiName : spis)
         out << "@_spi(" << spiName << ") ";
     }

lib/Sema/TypeCheckAccess.cpp

@@ -1467,14 +1467,25 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
 
   // If there's an exportability problem with \p typeDecl, get its origin kind.
   static DisallowedOriginKind getDisallowedOriginKind(
-      const TypeDecl *typeDecl, const SourceFile &SF, const Decl *context) {
+      const TypeDecl *typeDecl, const SourceFile &SF, const Decl *context,
+      DowngradeToWarning &downgradeToWarning) {
+    downgradeToWarning = DowngradeToWarning::No;
     ModuleDecl *M = typeDecl->getModuleContext();
-    if (SF.isImportedImplementationOnly(M))
+    if (SF.isImportedImplementationOnly(M)) {
+      // Temporarily downgrade implementation-only exportability in SPI to
+      // a warning.
+      if (context->isSPI())
+        downgradeToWarning = DowngradeToWarning::Yes;
+
+      // Implementation-only imported, cannot be reexported.
       return DisallowedOriginKind::ImplementationOnly;
-    else if (typeDecl->isSPI() && !context->isSPI())
+    } else if (typeDecl->isSPI() && !context->isSPI()) {
+      // SPI can only be exported in SPI.
       return context->getModuleContext() == M ?
         DisallowedOriginKind::SPILocal :
         DisallowedOriginKind::SPIImported;
+    }
+
     return DisallowedOriginKind::None;
   };
 
@@ -1494,9 +1505,10 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
       const_cast<TypeRepr *>(typeRepr)->walk(TypeReprIdentFinder(
           [&](const ComponentIdentTypeRepr *component) {
         TypeDecl *typeDecl = component->getBoundDecl();
-        auto originKind = getDisallowedOriginKind(typeDecl, SF, context);
+        auto downgradeToWarning = DowngradeToWarning::No;
+        auto originKind = getDisallowedOriginKind(typeDecl, SF, context, downgradeToWarning);
         if (originKind != DisallowedOriginKind::None) {
-          diagnoser.diagnoseType(typeDecl, component, originKind);
+          diagnoser.diagnoseType(typeDecl, component, originKind, downgradeToWarning);
           foundAnyIssues = true;
         }
 
@@ -1524,9 +1536,10 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
         : SF(SF), context(context), diagnoser(diagnoser) {}
 
       void visitTypeDecl(const TypeDecl *typeDecl) {
-        auto originKind = getDisallowedOriginKind(typeDecl, SF, context);
+        auto downgradeToWarning = DowngradeToWarning::No;
+        auto originKind = getDisallowedOriginKind(typeDecl, SF, context, downgradeToWarning);
         if (originKind != DisallowedOriginKind::None)
-          diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr, originKind);
+          diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr, originKind, downgradeToWarning);
       }
 
       void visitSubstitutionMap(SubstitutionMap subs) {
@@ -1641,9 +1654,13 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
 
     void diagnoseType(const TypeDecl *offendingType,
                       const TypeRepr *complainRepr,
-                      DisallowedOriginKind originKind) const {
+                      DisallowedOriginKind originKind,
+                      DowngradeToWarning downgradeToWarning) const {
       ModuleDecl *M = offendingType->getModuleContext();
-      auto diag = D->diagnose(diag::decl_from_hidden_module,
+      auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes?
+                            diag::decl_from_hidden_module_warn:
+                            diag::decl_from_hidden_module;
+      auto diag = D->diagnose(errorOrWarning,
                               offendingType->getDescriptiveKind(),
                               offendingType->getName(),
                               static_cast<unsigned>(reason), M->getName(),

test/SPI/experimental_spi_imports_swiftinterface.swift

@@ -0,0 +1,26 @@
+/// Test the textual interfaces generated with -experimental-spi-imports.
+
+// RUN: %empty-directory(%t)
+
+/// 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 %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
+
+/// Test the generated swiftinterface.
+// RUN: %target-swift-frontend -typecheck %s -emit-module-interface-path %t/main.swiftinterface -emit-private-module-interface-path %t/main.private.swiftinterface -enable-library-evolution -swift-version 5 -I %t -experimental-spi-imports
+// RUN: %FileCheck -check-prefix=CHECK-PUBLIC %s < %t/main.swiftinterface
+// RUN: %FileCheck -check-prefix=CHECK-PRIVATE %s < %t/main.private.swiftinterface
+
+@_spi(dummy) @_implementationOnly import ExperimentalImported
+// CHECK-PUBLIC-NOT: import ExperimentalImported
+// CHECK-PRIVATE: @_implementationOnly @_spi{{.*}} import ExperimentalImported
+
+@_implementationOnly import IOIImported
+// CHECK-PUBLIC-NOT: IOIImported
+// CHECK-PRIVATE-NOT: IOIImported
+
+@_spi(dummy) import SPIImported
+// CHECK-PUBLIC: {{^}}import SPIImported
+// CHECK-PRIVATE: @_spi{{.*}} import SPIImported

test/SPI/experimental_spi_imports_type_check.swift

@@ -0,0 +1,50 @@
+/// Test the use of implementation-only types with -experimental-spi-imports.
+
+/// Build LibCore an internal module and LibPublic a public module using LibCore.
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -DLIB_CORE %s -module-name LibCore -emit-module-path %t/LibCore.swiftmodule -enable-library-evolution -swift-version 5
+// RUN: %target-swift-frontend -emit-module -DLIB_PUBLIC %s -module-name LibPublic -emit-module-path %t/LibPublic.swiftmodule -I %t -emit-module-interface-path %t/LibPublic.swiftinterface -emit-private-module-interface-path %t/LibPublic.private.swiftinterface -enable-library-evolution -swift-version 5 -experimental-spi-imports
+
+/// Test with the swiftmodule file, the compiler raises an error only when
+/// LibCore isn't loaded by the client.
+// RUN: %target-typecheck-verify-swift -DCLIENT -I %t
+// RUN: %target-swift-frontend -typecheck %s -DCLIENT -DCLIENT_LOAD_CORE -I %t
+
+/// Test with the private swiftinterface file, the compiler raises an error
+/// only when LibCore isn't loaded by the client.
+// RUN: rm %t/LibPublic.swiftmodule
+// RUN: %target-typecheck-verify-swift -DCLIENT -I %t
+// RUN: %target-swift-frontend -typecheck %s -DCLIENT -DCLIENT_LOAD_CORE -I %t
+
+/// Test with the public swiftinterface file, the SPI is unknown.
+// RUN: rm %t/LibPublic.private.swiftinterface
+// RUN: %target-typecheck-verify-swift -DCLIENT -I %t
+// RUN: %target-typecheck-verify-swift -DCLIENT -DCLIENT_LOAD_CORE -I %t
+
+#if LIB_CORE
+
+public struct CoreStruct {
+  public init() {}
+  public func coreMethod() {}
+}
+
+
+#elseif LIB_PUBLIC
+
+@_spi(dummy) @_implementationOnly import LibCore
+
+@_spi(A) public func SPIFunc() -> CoreStruct { return CoreStruct() }
+
+
+#elseif CLIENT
+
+@_spi(A) import LibPublic
+
+#if CLIENT_LOAD_CORE
+import LibCore
+#endif
+
+let x = SPIFunc() // expected-error {{cannot find 'SPIFunc' in scope}}
+x.coreMethod()
+
+#endif

test/SPI/implementation_only_spi_import_exposability.swift

@@ -26,8 +26,8 @@ public protocol IOIProtocol {}
 
 @_spi(A) @_implementationOnly import Lib
 
-@_spi(B) public func leakSPIStruct(_ a: SPIStruct) -> SPIStruct { fatalError() } // expected-error 2 {{cannot use struct 'SPIStruct' here; 'Lib' has been imported as implementation-only}}
-@_spi(B) public func leakIOIStruct(_ a: IOIStruct) -> IOIStruct { fatalError() } // expected-error 2 {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}}
+@_spi(B) public func leakSPIStruct(_ a: SPIStruct) -> SPIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'SPIStruct' in SPI; 'Lib' has been imported as implementation-only}}
+@_spi(B) public func leakIOIStruct(_ a: IOIStruct) -> IOIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'IOIStruct' in SPI; 'Lib' has been imported as implementation-only}}
 
 public struct PublicStruct : IOIProtocol, SPIProtocol { // expected-error {{cannot use protocol 'IOIProtocol' here; 'Lib' has been imported as implementation-only}}
 // expected-error @-1 {{cannot use protocol 'SPIProtocol' here; 'Lib' has been imported as implementation-only}}
@@ -43,4 +43,9 @@ public struct PublicStruct : IOIProtocol, SPIProtocol { // expected-error {{cann
   }
 }
 
+@_spi(B)
+public struct LocalSPIStruct : IOIProtocol, SPIProtocol { // expected-warning {{cannot use protocol 'IOIProtocol' in SPI; 'Lib' has been imported as implementation-only}}
+// expected-warning @-1 {{cannot use protocol 'SPIProtocol' in SPI; 'Lib' has been imported as implementation-only}}
+}
+
 #endif