SPI support in Swift #29810

xymus · 2020-02-13T17:34:58Z

Add experimental support to define System Programming Interfaces (SPI) in Swift. An SPI is a kind of API targetted at specific clients and that is hidden by default. In practice, a Swift library developer could mark decls as SPI if they are reserved for a closely-related library or client, or if they are experimental and may be modified without warning in the next version of the library.

Definition and importation of SPIs is declared with the @_spi attribute. A public decl marked with @_spi(SPIName) is usable only from within the same module and by clients that imports the module with the compatible @_spi(SPIName) attribute.

In the following example, MyLib defines a function under the SPI named Experimental.

// MyLib
@_spi(Experimental) public func newExperimentalService() {}

The SPI function newExperimentalService is hidden from clients that imports the module normally.

import MyLib

newExperimentalService() // Error: use of unresolved identifier

However, clients that imports MyLib and its SPI Experimental have access to all decls with the attribute @_spi(Experimental) declared in MyLib. This is a way for the library clients to opt-in using the SPI.

@_spi(Experimental) import MyLib

newExperimentalService() // Ok

The developers of a binary framework can restrict access to its SPIs by distributing the new private Swift textual interface file (.private.swiftinterface) only to the authorized clients. This file contains both the public API and all SPIs, it is generated next to the public textual interface (.swiftinterface). The compiler prefers to load the private textual interface and falls back on the public one if the private is not found. Generating the private file is requested by the option -emit-private-module-interface-path.

This is a first partial support for SPI in Swift. A few things still need work, including the diagnostics that is not always aware of the SPI restriction and reports SPI decls as being internal.

We might want to find a better syntax for SPI attributes that can hold a list of names. The current underlying implementation already supports a list of name per attribute but it’s not reflected in the syntax.

xymus · 2020-02-13T17:35:16Z

@swift-ci Please test

swift-ci · 2020-02-13T18:34:51Z

Build failed
Swift Test OS X Platform
Git Sha - 9c62848787173f41ff07aeec86ed7d4fefd1f206

compnerd · 2020-02-13T18:57:53Z

@swift-ci please test Windows platform

xymus · 2020-02-13T20:59:48Z

@swift-ci Please smoke test

xymus · 2020-02-13T21:00:04Z

@swift-ci please test Windows platform

compnerd · 2020-02-13T21:44:03Z

@swift-ci please test Windows platform

xymus · 2020-02-14T01:16:32Z

The Windows test fails on network issues, it passed the first time. @compnerd you can trigger it again if you want.

slavapestov

It looks like SPI is not inherited from the parent context (type, or extension etc). Is this intended? Maybe it would help if the various places that directly checked for an SPI attribute on the decl were modified to evaluate a request instead, and the request could look at the parent context.

slavapestov · 2020-02-18T03:06:21Z

include/swift/AST/SourceFile.h

    StringRef filename;

+    // Names of explicitly imported SPIs.
+    SmallVector<Identifier, 4> spis;


Should this be an arena-allocated ArrayRef, just like the ImportedModuleDesc itself?

Right, it should now be fixed.

slavapestov · 2020-02-18T03:09:06Z

include/swift/AST/DiagnosticsParse.def

@@ -1378,6 +1378,8 @@ ERROR(projection_value_property_not_identifier,none,
 // Access control
 ERROR(attr_access_expected_set,none,
      "expected 'set' as subject of '%0' modifier", (StringRef))
+ERROR(attr_access_expected_spi_name,none,
+      "expected an SPI identifier as subject of the @_spi attribute", ())


We tend to quote attribute names in diagnostics

slavapestov · 2020-02-18T03:10:43Z

lib/Sema/TypeCheckAttr.cpp

+
+  auto VD = dyn_cast<ValueDecl>(D);
+  if (!VD) {
+    diagnose(attr->getLocation(),


Isn't this already handled as part of the definition in Attr.def?

Right, we only need the check for the access level of the decl.

slavapestov · 2020-02-18T03:12:13Z

lib/Sema/TypeCheckAccess.cpp

 void AccessControlCheckerBase::checkTypeAccessImpl(
    Type type, TypeRepr *typeRepr, AccessScope contextAccessScope,
-    const DeclContext *useDC, bool mayBeInferred,
+    const DeclContext *useDC, bool mayBeInferred, bool fromSPI,


Perhaps 'fromSPI' should be an enum?

slavapestov · 2020-02-18T03:16:25Z

lib/AST/Module.cpp

@@ -549,6 +549,11 @@ void ModuleDecl::lookupObjCMethods(
  FORWARD(lookupObjCMethods, (selector, results));
 }

+void ModuleDecl::lookupImportedSPIs(const ModuleDecl *importedModule,


Is the module variant of this called anywhere? Generally, imports in other source files don't affect behavior in a source file. So the decl checker should always be calling this on a SourceFile and not the entire module. Also, FORWARD is O(n) in the number of source files -- this can be slow if you're calling it a lot.

lookupImportedSPIs is called on modules when printing the textual Swift interface and for serialization (including at merge-modules), so it's not called often. I'm not sure we could keep in only on the source file in this case.

However, the SPI decl checks use isImportedAsSPI which is only on source files and it calls only SourceFile:: lookupImportedSPIs.

slavapestov · 2020-02-18T03:16:47Z

lib/Serialization/ModuleFile.cpp

@@ -2531,6 +2531,11 @@ void ModuleFile::lookupObjCMethods(
  }
 }

+void ModuleFile::lookupImportedSPIs(const ModuleDecl *importedModule,
+                                    SmallVectorImpl<Identifier> &spis) const {
+  // TODO


Probably this won't be needed at all, so maybe lookupImportedSPIs should be a method on SourceFile only?

We need this one when printing the private textual interface at the merge-module phase as it needs all the SPI imported from all partial modules.

xymus · 2020-02-18T22:26:44Z

Updated to rebase on master and apply Slava's comments.

xymus · 2020-02-18T22:28:22Z

@swift-ci please smoke test

compnerd · 2020-02-19T00:35:13Z

@swift-ci please clean test Windows platform

nkcsgexi · 2020-02-19T00:25:16Z

lib/AST/Decl.cpp

+          VD->getAttrs().hasAttribute<SPIAccessControlAttr>() &&
+          !useSF->isImportedAsSPI(VD))
+        return false;
+    }


This seems to be a duplication. Could you refactor them to a common function?

nkcsgexi · 2020-02-19T00:28:02Z

lib/AST/Module.cpp

+
+  for (auto attr : targetDecl->getAttrs().getAttributes<SPIAccessControlAttr>())
+    for (auto declSPI : attr->getSPINames())
+      for (auto importedSPI : importedSpis)


hmm, three nested loops.. can we use Set somewhere to avoid iterating?

I plan on moving the logic collecting the SPI groups declared on a decl to a request in order to cover what Slava pointed out with decls inheriting SPI from their parent. With it I can probably add a function for the previous comment too.

nkcsgexi · 2020-02-19T00:31:41Z

include/swift/AST/FileUnit.h

+  /// Find all SPI imported from \p importedModule by this module, collecting
+  /// their identifiers in \p spis.
+  virtual void lookupImportedSPIs(const ModuleDecl *importedModule,
+                                  SmallVectorImpl<Identifier> &spis) const {};


The function name is kind of confusing. Can we change it to something like lookupImportedSPIKinds or lookupImportedSPINotions.

I changed the ambiguous naming for an "SPI name" to "SPI group" instead.

nkcsgexi · 2020-02-19T00:34:20Z

include/swift/AST/SourceFile.h

    StringRef filename;

+    // Names of explicitly imported SPIs.
+    ArrayRef<Identifier> spis;


can we rename this to spiKinds? spis sounds like these are the decl identifiers of decls marked as SPI.

nkcsgexi · 2020-02-19T00:37:35Z

lib/AST/ASTPrinter.cpp

+
+          // Always print SPI decls if `PrintSPIs`.
+          if (options.PrintSPIs &&
+              VD->getAttrs().hasAttribute<SPIAccessControlAttr>())


Why is this check necessary? It should be fine if we always print SPI attributes and they will be skipped (because decls are skipped) if we print non-spi version of the interface.

SPI decls would be skipped because there formal access is considered to be internal here, so we have to force skip them. It’s a bit backwards because of the recent change from the default internal to public, I’ll have to come back to clean this.

nkcsgexi · 2020-02-19T00:39:29Z

lib/Sema/TypeCheckAccess.cpp

  // diag::conformance_from_implementation_only_module.
  enum class Reason : unsigned {
    General,
    ExtensionWithPublicMembers,
    ExtensionWithConditionalConformances
  };
+  enum class HiddenImportKind : unsigned {


uint8_t should be sufficient.

CodaFi · 2020-02-19T01:34:10Z

A request: Please diagnose @_exported+@_spi.

…a module

xymus · 2020-02-19T22:32:39Z

@swift-ci Please smoke test

e-001 · 2021-11-18T23:26:34Z

@xymus, could you please clarify if the following is per-spec? I know IDE behavior is beyond the purview of this work, but I can't find much documentation.

Two Swift packages, one is fetched via source control and another is a local clone. Both are added to an Xcode project via the Xcode/SPM package interface. The local clone will expose and allow for @_spi(Experimental) imports, whereas the remote will not. Both sources are identical on disk. Neither includes private module emit flags. The access level from the consuming code should be the same, right?

IlyaPuchkaTW · 2022-07-20T11:12:02Z

@_spi does not seem to limit visibility of inherited initialisers in classes when protection is added on the initialiser in a super class and subclass does not override it

xymus requested review from slavapestov, beccadax and nkcsgexi February 13, 2020 17:34

xymus force-pushed the spi branch from 9c62848 to 96fbff6 Compare February 13, 2020 20:59

slavapestov reviewed Feb 18, 2020

View reviewed changes

xymus force-pushed the spi branch from 96fbff6 to bd032d9 Compare February 18, 2020 22:24

xymus force-pushed the spi branch from bd032d9 to 780a217 Compare February 18, 2020 22:28

nkcsgexi reviewed Feb 19, 2020

View reviewed changes

xymus added 11 commits February 19, 2020 14:17

[AST] Intro SPI attribute for access control and imports

d5969a9

[Parser] Parse the SPI attribute

3552f80

[AST] Intro service lookupImportedSPIs to find direct SPI imports of …

501f458

…a module

[SPI] Serialize SPI attributes on imports

6a5a858

[Sema] Diagnose the use of SPI on unsupported decls

56880d7

[Sema] Type-check the use and exposability of SPI decls

2e73cb4

[Frontend] Output the private module interface with the SPI info

9013928

[Frontend] Use the private module interface when available

ffa0bda

[Test] SPI access and limits when used locally

b14fdd3

[Test] Test clients of modules defining SPI decls

4d6adef

[Test] Test generated private and public swiftinterface when using @_spi

d4bd722

xymus added 4 commits February 19, 2020 14:17

[Sema] Consider SPI decls public to emit symbols and TBD

d12292c

[Test] Test use of SPI in compiled lib

d4e0336

[Serialization] Register IMPORTED_MODULE_SPIS in the block info block

ac0f444

Use "SPI group" for the name used in an @_spi attribute

0e7029d

xymus force-pushed the spi branch from 780a217 to 0e7029d Compare February 19, 2020 22:21

swiftlang deleted a comment from swift-ci Feb 19, 2020

xymus merged commit b0b927b into swiftlang:master Feb 20, 2020

mludowise-stripe mentioned this pull request Jul 10, 2021

Don't document anything SPI-public realm/jazzy#1263

Closed

xymus deleted the spi branch October 31, 2022 21:03

SPI support in Swift #29810

SPI support in Swift #29810

Uh oh!

Conversation

xymus commented Feb 13, 2020

Uh oh!

xymus commented Feb 13, 2020

Uh oh!

swift-ci commented Feb 13, 2020

Uh oh!

compnerd commented Feb 13, 2020

Uh oh!

xymus commented Feb 13, 2020

Uh oh!

xymus commented Feb 13, 2020

Uh oh!

compnerd commented Feb 13, 2020

Uh oh!

xymus commented Feb 14, 2020

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xymus commented Feb 18, 2020

Uh oh!

xymus commented Feb 18, 2020

Uh oh!

compnerd commented Feb 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodaFi commented Feb 19, 2020

Uh oh!

xymus commented Feb 19, 2020

Uh oh!

e-001 commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IlyaPuchkaTW commented Jul 20, 2022

Uh oh!

Uh oh!

e-001 commented Nov 18, 2021 •

edited

Loading