Merge pull request #61011 from xymus/spi-only-exportability

xymus · web-flow · commit 7b9d4e645cc0 · 2022-09-09T11:56:24.000-07:00
[Sema] Check exportability of @_spiOnly imported decls
diff --git a/docs/Testing.md b/docs/Testing.md
@@ -344,6 +344,12 @@ code for the target that is not the build machine:
   swiftinterface to the given path and passing additional default flags
   appropriate for resilient frameworks.
 
+* ``%target-swift-emit-module-interfaces(`` *swift interface path*,
+  *swift private interface path* ``)`` *other arguments*:
+  run ``swift-frontend`` for the target, emitting both swiftinterfaces
+  to the given paths and passing additional default flags appropriate for
+  resilient frameworks.
+
 * ``%target-swift-typecheck-module-from-interface(`` *swift interface path*
   ``)`` *other arguments*: run ``swift-frontend`` for the target, verifying
   the swiftinterface at the given path and passing additional default flags
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2952,6 +2952,7 @@ ERROR(decl_from_hidden_module,none,
       "%select{%3 has been imported as implementation-only|"
       "it is an SPI imported from %3|"
       "it is SPI|"
+      "%3 was imported for SPI only|"
       "%3 was not imported by this file}4",
       (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 WARNING(decl_from_hidden_module_warn,none,
@@ -2960,7 +2961,7 @@ WARNING(decl_from_hidden_module_warn,none,
       "in an extension with public or '@usableFromInline' members|"
       "in an extension with conditional conformances}2; "
       "%select{%3 has been imported as implementation-only|"
-      "<<ERROR>>|<<ERROR>>|"
+      "<<ERROR>>|<<ERROR>>|<<ERROR>>|"
       "%3 was not imported by this file}4",
       (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 ERROR(typealias_desugars_to_type_from_hidden_module,none,
@@ -2972,6 +2973,7 @@ ERROR(typealias_desugars_to_type_from_hidden_module,none,
       "because %select{%4 has been imported as implementation-only|"
       "it is an SPI imported from %4|"
       "<<ERROR>>|"
+      "%4 was imported for SPI only|"
       "%4 was not imported by this file}5",
       (DeclName, StringRef, StringRef, unsigned, Identifier, unsigned))
 ERROR(conformance_from_implementation_only_module,none,
@@ -2982,6 +2984,7 @@ ERROR(conformance_from_implementation_only_module,none,
       "%select{%3 has been imported as implementation-only|"
       "the conformance is declared as SPI in %3|"
       "the conformance is declared as SPI|"
+      "%3 was imported for SPI only|"
       "%3 was not imported by this file}4",
       (Type, Identifier, unsigned, Identifier, unsigned))
 NOTE(assoc_conformance_from_implementation_only_module,none,
@@ -5892,12 +5895,13 @@ ERROR(inlinable_decl_ref_from_hidden_module,
       "because %select{%3 was imported implementation-only|"
       "it is an SPI imported from %3|"
       "it is SPI|"
+      "%3 was imported for SPI only|"
       "%3 was not imported by this file}4",
       (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 
 WARNING(inlinable_decl_ref_from_hidden_module_warn,
       none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
-      "because %select{<<ERROR>>|<<ERROR>>|<<ERROR>>|"
+      "because %select{<<ERROR>>|<<ERROR>>|<<ERROR>>|<<ERROR>>|"
       "%3 was not imported by this file}4"
       "; this is an error in Swift 6",
       (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
@@ -5907,6 +5911,7 @@ ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
       "because %select{%4 has been imported as implementation-only|"
       "it is an SPI imported from %4|"
       "<<ERROR>>|"
+      "%4 was imported for SPI only|"
       "%4 was not imported by this file}5",
       (DeclName, StringRef, StringRef, unsigned, Identifier, unsigned))
 
diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h
@@ -26,12 +26,20 @@ namespace swift {
 class PersistentParserState;
 
 /// Kind of import affecting how a decl can be reexported.
+///
+/// This is sorted in order of priority in case the same module is imported
+/// differently. e.g. a normal import (None) offers more visibility than
+/// an @_spiOnly import, which offers more visibility than an
+/// @_implementationOnly import. The logic of \c getRestrictedImportKind relies
+/// on the order of this enum.
+///
 /// This is a subset of \c DisallowedOriginKind.
 ///
 /// \sa getRestrictedImportKind
 enum class RestrictedImportKind {
-  ImplementationOnly,
   Implicit,
+  ImplementationOnly,
+  SPIOnly,
   None // No restriction, i.e. the module is imported publicly.
 };
 
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -2529,21 +2529,25 @@ RestrictedImportKind SourceFile::getRestrictedImportKind(const ModuleDecl *modul
   auto &imports = getASTContext().getImportCache();
   RestrictedImportKind importKind = RestrictedImportKind::Implicit;
 
+  // Workaround for the cases where the bridging header isn't properly
+  // imported implicitly.
   if (module->getName().str() == CLANG_HEADER_MODULE_NAME)
     return RestrictedImportKind::None;
 
   // Look at the imports of this source file.
   for (auto &desc : *Imports) {
-    // Ignore implementation-only imports.
     if (desc.options.contains(ImportFlags::ImplementationOnly)) {
-      if (imports.isImportedBy(module, desc.module.importedModule))
+      if (importKind < RestrictedImportKind::ImplementationOnly &&
+          imports.isImportedBy(module, desc.module.importedModule))
         importKind = RestrictedImportKind::ImplementationOnly;
-      continue;
     }
-
-    // If the module is imported publicly, it's not imported
-    // implementation-only.
-    if (imports.isImportedBy(module, desc.module.importedModule))
+    else if (desc.options.contains(ImportFlags::SPIOnly)) {
+      if (importKind < RestrictedImportKind::SPIOnly &&
+          imports.isImportedBy(module, desc.module.importedModule))
+        importKind = RestrictedImportKind::SPIOnly;
+    }
+    // If the module is imported publicly, there's no restriction.
+    else if (imports.isImportedBy(module, desc.module.importedModule))
       return RestrictedImportKind::None;
   }
 
diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp
@@ -1548,9 +1548,14 @@ swift::getDisallowedOriginKind(const Decl *decl,
   if (howImported != RestrictedImportKind::None) {
     // Temporarily downgrade implementation-only exportability in SPI to
     // a warning.
-    if (where.isSPI())
+    if (where.isSPI() &&
+        where.getFragileFunctionKind().kind == FragileFunctionKind::None &&
+        !SF->getASTContext().LangOpts.EnableSPIOnlyImports)
       downgradeToWarning = DowngradeToWarning::Yes;
 
+    if (where.isSPI() && howImported == RestrictedImportKind::SPIOnly)
+      return DisallowedOriginKind::None;
+
     // Before Swift 6, implicit imports were not reported unless an
     // implementation-only import was also present. Downgrade to a warning
     // just in this case.
@@ -1599,9 +1604,16 @@ swift::getDisallowedOriginKind(const Decl *decl,
     }
 
     // Restrictively imported, cannot be reexported.
-    if (howImported == RestrictedImportKind::Implicit)
+    switch (howImported) {
+    case RestrictedImportKind::Implicit:
       return DisallowedOriginKind::ImplicitlyImported;
-    return DisallowedOriginKind::ImplementationOnly;
+    case RestrictedImportKind::SPIOnly:
+      return DisallowedOriginKind::SPIOnly;
+    case RestrictedImportKind::ImplementationOnly:
+      return DisallowedOriginKind::ImplementationOnly;
+    default:
+      llvm_unreachable("RestrictedImportKind isn't handled");
+    }
   } else if ((decl->isSPI() || decl->isAvailableAsSPI()) && !where.isSPI()) {
     if (decl->isAvailableAsSPI() && !decl->isSPI()) {
       // Allowing unavailable context to use @_spi_available decls.
diff --git a/lib/Sema/TypeCheckAccess.h b/lib/Sema/TypeCheckAccess.h
@@ -44,6 +44,7 @@ enum class DisallowedOriginKind : uint8_t {
   ImplementationOnly,
   SPIImported,
   SPILocal,
+  SPIOnly,
   ImplicitlyImported,
   None
 };
diff --git a/test/SPI/report-ioi-in-spi.swift b/test/SPI/report-ioi-in-spi.swift
@@ -0,0 +1,60 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
+// RUN:   -module-name Lib -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -swift-version 5
+
+/// Use of IOI types in SPI signatures is an error with -experimental-spi-only-imports
+// RUN: %target-swift-frontend -emit-module %t/ClientSPIOnlyMode.swift -I %t \
+// RUN:   -swift-version 5 -verify \
+// RUN:   -experimental-spi-only-imports
+
+/// Use of IOI types in SPI signatures is a warning without -experimental-spi-only-imports
+// RUN: %target-swift-frontend -emit-module %t/ClientDefaultMode.swift -I %t \
+// RUN:   -swift-version 5 -verify
+
+/// This is a warning in swiftinterfaces
+// R UN: %target-swift-typecheck-module-from-interface(%t/Client.private.swiftinterface) \
+// R UN:   -I %t -module-name Client
+
+//--- Lib.swift
+
+public struct IOIStruct {
+    public init() {}
+}
+
+//--- ClientSPIOnlyMode.swift
+
+@_implementationOnly import Lib
+
+@_spi(X) public func spiClient(s: IOIStruct) -> IOIStruct { // expected-error 2 {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}}
+    return IOIStruct()
+}
+
+@_spi(X) @inlinable public func inlinableClient(s: IOIStruct) -> IOIStruct { // expected-error 2 {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+    return IOIStruct() // expected-error {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+    // expected-error @-1 {{initializer 'init()' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+}
+
+//--- ClientDefaultMode.swift
+
+@_implementationOnly import Lib
+
+@_spi(X) public func spiClient(s: IOIStruct) -> IOIStruct { // expected-warning 2 {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}}
+    return IOIStruct()
+}
+
+@_spi(X) @inlinable public func inlinableClient(s: IOIStruct) -> IOIStruct { // expected-error 2 {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+    return IOIStruct() // expected-error {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+    // expected-error @-1 {{initializer 'init()' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
+}
+
+//--- Client.private.swiftinterface
+
+// swift-interface-format-version: 1.0
+// swift-compiler-version: Swift version 5.8-dev effective-4.1.50
+// swift-module-flags: -swift-version 4 -module-name Client
+@_implementationOnly import Lib
+
+@_spi(X) public func spiClient() -> IOIStruct { fatalError() }
diff --git a/test/SPI/spi-only-and-library-level.swift b/test/SPI/spi-only-and-library-level.swift
@@ -0,0 +1,42 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
+// RUN:   -module-name Lib -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -swift-version 5
+// RUN: %target-swift-frontend -emit-module %t/APILib.swift -I %t \
+// RUN:   -swift-version 5 -verify \
+// RUN:   -experimental-spi-only-imports \
+// RUN:   -library-level api
+// RUN: %target-swift-frontend -emit-module %t/SPILib.swift -I %t \
+// RUN:   -swift-version 5 -verify \
+// RUN:   -experimental-spi-only-imports \
+// RUN:   -library-level spi
+// RUN: %target-swift-frontend -emit-module %t/OtherLib.swift -I %t \
+// RUN:   -swift-version 5 -verify \
+// RUN:   -experimental-spi-only-imports
+
+//--- Lib.swift
+
+public struct LibStruct {}
+
+//--- APILib.swift
+
+@_spiOnly import Lib
+
+public func publicClient() -> LibStruct { fatalError() } // expected-error {{cannot use struct 'LibStruct' here; 'Lib' was imported for SPI only}}
+@_spi(X) public func spiClient() -> LibStruct { fatalError() }
+
+//--- SPILib.swift
+
+@_spiOnly import Lib
+
+public func publicClient() -> LibStruct { fatalError() }
+@_spi(X) public func spiClient() -> LibStruct { fatalError() }
+
+//--- OtherLib.swift
+
+@_spiOnly import Lib
+
+public func publicClient() -> LibStruct { fatalError() } // expected-error {{cannot use struct 'LibStruct' here; 'Lib' was imported for SPI only}}
+@_spi(X) public func spiClient() -> LibStruct { fatalError() }
diff --git a/test/SPI/spi-only-import-exportability.swift b/test/SPI/spi-only-import-exportability.swift
diff --git a/test/Sema/restricted-imports-priorities.swift b/test/Sema/restricted-imports-priorities.swift
diff --git a/test/lit.cfg b/test/lit.cfg
