[Pre-Pitch] Import access control: a modest proposal

Speaking for myself here, not the Core Team, but I really like the direction of this proposal. As others have noted, the public-ness of the current import declaration goes against Swift's general principle of leaving maximum freedom for library authors by default.

I also consider a change like this to be important for our ongoing work on improving build times: today's import semantics introduce extraneous dependencies into the build graph that make it harder to optimize builds when many modules are present. This proposal helps limit those dependencies to ones that matter.

I consider both of these to be bugs and would like to fix them in the same cleanup. However, they are significantly less important than the main proposal here.

I agree that Swift 6 is the right timing for such a change. @Joe_Groff 's suggestion to phase this out over two major releases feels too slow to me, especially given that Swift 5 -> 6 will be a least a 3-year gap.

Doug

25 Likes

Problems like this where a binary module and its client both link different versions of the same library, but the compiler doesn't detect this because the binary module's import is not visible to its client, will be more likely to happen because more imports will be internal.

Are there mitigations for this? Can we expand on this problem please?

1 Like

I'm very glad to see some progress in that direction, because currently we are struggling with our build time and module visibility mess in project. To give some perspective, we currently have 500+ swift and 800+ clang modules as dependencies of root application module, and it takes forever to compile.

Regardless of proposal itself, I like the use of existing keywords and changing the default to more isolate one. I like the idea about open import instead of @exported import as it uses familiar keyword, although with a little different meaning.

I would also like if we had some way (compiler warning? separate tool?) to check that specific modules are public or not, as it very useful to check that build system's dependencies and actual code are in sync.

1 Like

We've migrated our project to use @_implementationOnly imports, and got about 30% faster clean build of "root" modules, and about 15-20% faster incremental builds.

It's not very much, but that's done without changing any code, only adding @_implementationOnly attribute, where it's possible. We certainly have a lot of "leaked" modules, that are supposed to be imported privately but exposed accidentally, and a lot of modules are publicly imported just because their types are used in internal/private properties.

I’m very much in favor of this proposal. It makes perfect sense, and it matches the default level of access control perfectly.

I’m also in favor of fileprivate import: I think people would be a lot less worried about using dependencies if they could get compiler assurances that they wouldn’t accidentally end up getting used in unexpected places. I imagine it’d make type inference considerably easier as well.

Ideally, it should be impossible to break clients of a module by changing non-public implementation details, including dependencies. SemVer is built with that assumption, after all.

On a related note, I assume that the most restrictive access control wins?

  • Module B publicly imports Module A
  • Module C internally imports Module B
  • Module D imports Module C

Under that scenario:

  • Module B would have the public imports and declarations of Module A
  • Module C would have the public imports and declarations of Modules A and B
  • Module D would have the public imports of Module C
2 Likes

I’ve been thinking about the whole import reform saga – now approaching its fourth year of idling in the forums – in light of the recent stability kerfuffle. If we aren’t likely to see movement on this soon, would it be reasonable to pitch de-underscoring @_exported and @_implementationOnly in the interim? Keeping them “unsupported” because we’re waiting to fulfill Jordan’s vision of a comprehensive rethink of imports is a clear example of letting the perfect be the enemy of the good.

7 Likes

IMO, @_implementationOnly definitely isn't ready for prime-time yet (which is why it's good that it's still underscored). Today, a type can't declare even a private stored property using a type from an @_implementationOnly-imported module (I think because the size of the property contributes to the layout of the containing type, at least for non-class types?). That needs to be resolved, because that's something that we should be able to do (even in non-resilient modules), and forcing everyone to use an Any and casting it back out isn't a great workaround.

With Swift's focus on source compatibility, our opportunities to do things "right" are somewhat more limited. There's no reason to rush the current implementation of the feature out and give it the official stamp of approval when you can already use that implementation today (edit: at your own risk) with the underscores.

2 Likes

I won’t pursue the point further here, but this position is not consistent with core team guidance.

2 Likes

I still think that unfinished features like that should require compiler arguments to use going forward. The current state of affairs allows libraries to use them without the end-user’s awareness, which has led to it being unnervingly common in production code.

The underscores only help if you audit your entire dependency tree for them.

Let's please not derail this thread about imports with the meta-discussion from the thread that was locked; I edited my post to mention that doing so is at one's own risk, and I regret mentioning it at all—I was only trying to highlight the point that dropping the underscore changes nothing about the readiness of the feature as it's implemented today. That's what folks should be focused on, not whether the name is "forbidden" or not.

If the feature was 100% polished and "production-ready", then it would be trivial to introduce a new version without the underscore and start using that in the next language version; someone from the community could propose that. The fact that this isn't being done is strong evidence that the feature is still missing important pieces before the feature can "graduate".

If you're reluctant to use a feature like @_implementationOnly in your own code, the reason shouldn't be because there's an underscore in front of its name. The reason should be because the feature is still missing important requirements. Dropping the underscore from the name isn't a remedy for that.

5 Likes

This is a very important point and one that I had forgotten about. Modules that enable library evolution vend non-@frozen structs and enums in a manner that abstracts away the size/alignment of those structs and enums and how they are copied and destroyed. For modules that do not enable library evolution, we expect to know the layout of all structs and enums precisely. So the compiler still knows about private and internal members in clients of the module, even thought client code isn't able to access them.

For @_implementationOnly imports to work completely, we need to introduce a way to communicate the precise layout information of structs and enums without exposing the private or internal fields and types.

Doug

12 Likes

Sorry for derailing the thread, you’re completely correct.

Clarification

I didn’t mean to imply that you should simply call an unstable feature stable and be done with it. Rather, I genuinely meant that future unstable features should not be usable without compiler flags (which is the case for some recent work), rather than just an underscore.

Returning to the proposal: when I import a module, the behavior I expect is basically acting as if the public contents of the module were copied wholesale into the current module, aside from non-CMO inlining, naming, etc. (I’m conflating public and open here, since the distinction isn’t really relevant.)

If I publicly imported a module, I’d expect the access level of all its public declarations to be capped at public. If I internally imported it, I’d expect the access level of public declarations to be capped at internal. This is already established as the guiding principle of access levels.

With that desired behavior in mind, how would the proposed implementation diverge? You can have private properties with private types in public structures right now. How is that different?

That’s exactly what I’m doing, though.

I don’t think it’s obviously problematic for @implementationOnly to be stabilized while it’s still overly constraining. Lifting constraints is non-breaking, and a stable attribute doesn’t need to be universally useful. If it currently allows a strict subset of what the desired internal import does, then it can be widened with more of that feature set, and become a deprecated-with-a-warning alternative spelling of internal import in future.

(That said, I’m personally more interested in stabilizing @exported, which would still be around in Becca’s version of import reform anyway.)

There’s never any guarantee that a future direction will happen, so any stabilized feature does need to stand on its own at a suitable “resting place” for the language that can be justifiable for an indefinite period of time.

This isn’t to say that there can’t be any limits to a feature, of course. Rather, since those limits become then exposed themselves as part of the user-facing rules of the language (for instance, we all know about and have had to deal with “Self or associatedtype requirements”), we have to consider what those contours are that would make for a good user experience for the time being, which could span multiple language versions. When the implemented parts of a nascent feature are much less than its intended final shape, it’s reasonable to conclude that the best user experience comes from temporarily drawing that line at zero.

It’s obviously a judgment call (like all other calls about whether a feature is shippable), but I think the message being conveyed is that @_implementationOnly hasn’t reached such a point from the perspective of the implementers to be exposed to any extent.

3 Likes

Some personal thoughts:

  • I’m totally on adding open import as the stabilized version of @_exported import, to make the proposal complete and cover the most popular import access control options.
  • What is the behavior of public import struct SomeModule.SomeStruct? Which of the access levels can be applied to import struct/enum/class/actor? We should definitely make this clear to align the import behavior.
1 Like

Hi guys!
What's the current state of this proposal?

At Meta, we found @_implementationOnly one of the most promising Swift Build speed optimisations.

Using a compiler-based codemod we automatically converted one of our apps (that heavily uses Swift) to use @_implementationOnly. Even with those aforementioned limitations, we observe quite a significant improvement of incremental and clean build time.

Total clean build speed has decreased by -14%(P70) with compilation time -50%(P70).
Incremental build speed is almost twice as faster as before! -45%(P70) of total build speed.
(Compilation time on the critical path for incremental builds has decreased by -70%(P70))

cc @beccadax @xymus

1 Like

Don’t know if this was proposed already.

I think more often than not I would like to declare at the module/package level that a dependency is "implementation-only", not on the individual import statements.

1 Like

I think those two are different.

at the module/package level

here you effectively define what search path will be passed to the compiler and where it will look for modules.

on the individual import statements

while here, you prevent the compiler from looking for transitive modules that are not needed for the compilation.

Not sure the optimisation could be implemented with only the former without the latter.

Im not really arguing against @_implementationOnly.

i just think it would be useful to declare that a dependency is "implementation only"

I guess that would be more of a SwiftPackageManager feature.

Something like this in Package.swift for instance

.target(
    name: "PublicLibrary",
    dependencies: [
        .implementationOnly("MyInternalLibrary")
    ]
),

That way i can declare that i dont want users of my library to depend on that module being available just by depending on my library.

It would give me the freedom to remove that library without breaking any code.

Maybe im misunderstanding a bit what @_implementationOnly currently would do.

Am i correct in assuming that if i forget to add @_implementationOnly to a single import statement it would in fact make it no longer "implementation-only" and consumers of my library can then actually import that module.

This is already formally illegal. Clients are not to assume anything about transitive dependencies. If the module is not listed in the product definition, it is an implementation detail and should not be imported directly. (Indirect use granted through @_exported import is fine).

It is only an accident of the file system that the compiler accepts such imports on some platforms. It is a hole that will be plugged. I think an implementation even exists already, although I do not remember if it is in main yet.

So you already have the freedom to remove that library without breaking any legal code.

Yes, but forgetting is difficult because you get a compiler warning letting you know your imports are inconsistent.

2 Likes