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

Correct—this change would be at compile-time. You’d need a newer compiler at your desk, but not a newer device in your users’ hands.

Probably, but I don’t think we’d actually want the compiler to infer public import any more than we’d want it to infer public class or public struct when you used a type in a public API. Your dependencies are an externally visible aspect of your module; explicitly controlling that aspect is a feature, not a bug.

Huh, that could actually be useful. I think it might be better as a separate proposal, though.

I am very much thinking about solutions to this problem, but it’s a separate issue from what we’re discussing in this thread.

One of the reasons I think we can get away with this change is that in app targets, you almost never need to write public import.* If Swift 6 mode warns about un-annotated imports, the proposal changes from “some imports, mostly in libraries, need to be marked with an additional keyword” to “every import in every file needs to be marked with an additional keyword”, and given how long it took to get from Swift 5 to Swift 6, waiting until Swift 7 to finish the transition could mean we spend several years in that state. I think that might tip this change from a minor inconvenience to a major hassle.

* It actually just occurred to me that even in app targets, retroactively conforming types you don’t own to protocols you don’t own would require a public import of both modules. This is arguably a feature.

My back-of-the-envelope hypothesis is that more than half of imports in libraries will need to become public imports, but very few imports in executables or test suites will need to change. This is not based on any data; I should probably instrument the compiler in a branch (e.g. repurpose the existing @_implementationOnly checking to emit a remark on normal imports that would need to become public) and then build the Source Compatibility Suite to get some numbers.

This is a really good question that I haven’t even thought about.

This is true. I guess my mental model is that <access-level> import M doesn’t mean “make this import visible at <access-level>; it means “import public declarations from M into this file as though they have <access-level> or less”. That is, (absent @exported) import is always inherently a per-file thing, and the access control keyword is being transitively applied to what’s being imported. (Although that gets a little weird with open.)

Incidentally, I learned recently that @hamishknight at least partially fixed the operator visibility logic last year, but there’s some compatibility logic to retain the old behavior and a feature flag to disable it.

8 Likes

The slight difference here is if you made a public struct that wasn't a part of your API, it may still be useful on its own as a type to consumers of the module. An unused public import, however, has no value except to increase build times for consumers of the module (I am considering re-exporting a non-value as the consuming module should explicitly import what it needs).

I am personally against making the default different between Swift versions. I didn't see why internal should be the default anymore than public. Why not keep public the default and break the cycle of "adding new pedantic breaking changes every couple years, resulting in evermore developer headache"

To be clear, we are suggest a change based on the project’s language version, which can be incremented by the developer on their own terms, not on the compiler version. There are already far more involved changes slated for the Swift 6 language mode to enforce concurrency safety, and IMO correcting the default import visibility should be relatively minor in impact.

20 Likes

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