SE-0409: Access-level modifiers on import declarations

strictlyswift · 2023-09-16T15:41:41.797Z

I share this concern. The meaning of "public import" in other languages seems quite different from what is being proposed here. I do wonder if a better alternative is available (bike shedding alert!) - how about shifting the access modifier to the end and adding "export" to make it clearer about what the modifier pertains to :
import Dependency export public
import Foo.Bar export private

allevato · 2023-09-16T15:53:35.922Z

jrose:

I don’t think there are that many times where I’ve wanted not only names restricted to a particular file, but keeping those types from escaping in internal API as well.

I can definitely think of times I would use this. File-scoped imports would be helpful when you're writing a wrapper around a component in another module and you want a guarantee that that module won't accidentally leak out of that file—for example, a Swift wrapper around a lower-level C library or system framework.

taylorswift · 2023-09-16T16:35:32.602Z

but the module can leak, if it declares conformances, the conformances will leak, if it declares operator lexemes, the lexemes will leak.

allevato · 2023-09-16T16:40:37.459Z

taylorswift:

but the module can leak, if it declares conformances, the conformances will leak, if it declares operator lexemes, the lexemes will leak.

IMO, those sound like additional areas for improvement that are orthogonal to the step in the right direction that this proposal would still provide.

taylorswift · 2023-09-16T16:44:50.821Z

but do we want these things to vary based on the imports at the top of the file? i don’t know if i would want import statements to change the precedence or associativity of ><, or suddenly cause String to conform (or stop conforming) to Error…

dmt · 2023-09-16T17:00:51.758Z

import declarations are only valid at file scope. So as long as Swift doesn't allow them to be nested in name-resolution-scope-defining entities, private access level on import is (at least) useless. IMO only fileprivate should be accepted for now. (or private in sense of fileprivate, wording doesn't matter a lot).
Consider a situation if some day Swift will support import decls in narrower scopes.
```
class Foo {
  fileprivate import Module1
  private import Module2

  fileprivate init(_ value: Module1.SomeType) {...}

  fileprivate func f1() -> Module1.SomeType {...}
  private func f2() -> Module2.SomeType {...}
}

private func useF1(foo: Foo) {
  Foo(foo.f1())
} 
```
Here it makes sense to distinguish fileprivate from private, as Module2 types shouldn't escape the scope of Foo, while Module1 types can.
allevato:

File-scoped imports would be helpful when you're writing a wrapper around a component in another module and you want a guarantee that that module won't accidentally leak out of that file—for example, a Swift wrapper around a lower-level C library or system framework.

While I agree fileprivate imports should be supported, I see that someone can argue with this reason by saying: "One can always decompose the "wrapper part" into a third separate module, with internal imports of low level C library, and thus provide a sealed isolation.".
The reason I think fileprivate imports should be supported is access level parity between import decls and other decls.

allevato · 2023-09-16T17:11:13.086Z

dmt:

While I agree fileprivate imports should be supported, I see that someone can argue with this reason by saying: "One can always decompose the "wrapper part" into a third separate module, with internal imports of low level C library, and thus provide a sealed isolation.".

I don't think that argument wouldn't hold much weight because telling someone to move a type out to a completely different API boundary would be an extreme and often inappropriate solution. If the wrapper type is meant to be a public part of the module, it can't just be moved to a different module without re-exporting it, and we don't want to encourage re-exporting as the solution for this much simpler problem.

dmt · 2023-09-16T17:20:59.218Z

allevato:

to move a type out to a completely different API boundary would be an extreme and often inappropriate solution

Well, I wouldn't call it inappropriate, but agree with "extreme". This is the solution we have right now on hands without additional complexity in the language/compiler. And this is quite rare case when one actually has to achieve this level of isolation. So amortised level of inconvenience due to the lack of fileprivate imports will be low.

taylorswift · 2023-09-16T17:21:13.388Z

i think that if you are trying to insulate parts of a wrapper module from the thing it's wrapping, that's an early indicator that the wrapper module has gotten too large and should be vertically split into two modules.

in this situation, re-exporting the wrapper type is the correct approach. there is a world of difference between re-exporting an entire module's namespace and re-exporting a single type.

allevato · 2023-09-16T17:40:12.769Z

I simply don't agree with that. The solution you're proposing provides that library author (and their clients) no significant benefit while causing actual harm by making their build graph and build process more complex. If the original module is going to re-export the thing from the new module, why is that better than just putting it in that module?

You're also assuming that nothing in the hypothetical wrapping file is using anything from the rest of the module. If it is, now the author has to slice up their module even more to make your proposed solution work. That's a lot of overhead when someone just wants to create a small intra-module boundary that's easy to reason about.

taylorswift · 2023-09-16T18:32:57.103Z

i think that ideally, for every module in your project, you should always have an answer to the question: what does this module do?

if you have a module named Bike, and your answer to the previous question is: “this module is a wrapper around CBike”, then it shouldn’t be a problem for any of the code in Bike to be aware of CBike and its definitions - the purpose of Bike is to interface with CBike.

now, if it turns out you’ve got a bunch of “library API” in Bike that only interacts with a handful of definitions in Bike, and doesn’t care at all about CBike, then that’s an indicator that maybe you shouldn’t have Bike and CBike, maybe you should have Bike, BikeShims, and CBike.

now, there are tons of awful reasons we are forced to “slice up” modules in swift today:

inability to publish more than one type within a module with the same name
inability to nest protocols within a namespace

but to me, organizing a project's build graph into clearly delimited layers is not one of them.

xymus · 2023-09-19T15:44:17.642Z

Karl:

I think it would be useful to allow @usableFromInline to be attached to these internal imports

jrose:

How does @usableFromInline fit into this?

We didn't previously consider @usableFromInline on imports but it's reasonable if there's an interest. Nothing should really be blocking it and it will only add one more condition preventing hiding a dependency.

8675309 · 2023-09-20T02:01:01.738Z

Will there be a compiler diagnostic for when an import is unnecessarily public, according to the APIs exposed in the module?

amt · 2023-09-20T15:02:17.531Z

xymus:

Nickolas_Pohilets:

Does this work with imported individual entities or only entire modules? Can I do public import struct Foo.Bar?

The access-level is applied to the whole module, not the individual declaration.

When combined with module-level imports, this could result in surprising code (tested with the experimental flag in Xcode):

private import AppKit                 // (1)
public import class AppKit.NSColor    // (2)

public func f(_ c: NSColor) { }       // (3)
public func g(_ v: NSView) { }        // (4) (!)

If lines 1 & 2 are both permitted, I'd expect line 4 to be prohibited, just based on the text of the code, but it's not. Once default imports are internal, this will be even easier to trip over, I think. Should this be handled, either by erroring on line 4 or by just disallowing the mismatched imports for now?

xymus · 2023-09-20T17:17:38.808Z

8675309:

Will there be a compiler diagnostic for when an import is unnecessarily public, according to the APIs exposed in the module?

This is something I'm currently looking into. My plan would be to warn on any import marked as public or package if they could be internal or lower. It would apply by default only in Swift 6 mode as I don't want to force people to add internal everywhere just for them to remove it a few months later when it becomes the default. I don't plan on reporting internal imports that could be private/fileprivate by default either, for the use case of a single module app one could still use the default imports without having to worry about access levels.

amt:

Should this be handled, either by erroring on line 4 or by just disallowing the mismatched imports for now?

I think it should at least warn on the inconsistent imports. We should report different imports of the same module from the same file if they have different access levels. Currently the compiler only takes into account the most public import access level.

Tino · 2023-09-22T10:45:15.465Z

Paul_Cantrell:

I’d naïvely expect this to show up in a package manifest, not in source code: a library exposing its dependencies to clients is module-level information, so it would make sense for it to appear in Package.swift. I didn’t see any discuss of this in the proposal. (Apologies if I missed it.) What’s the rationale?

Imo this question is really worth an official answer — which, afaics, did not happen yet.
I don't see any significant benefit in the option of using different levels in different files, which has the downside that the actual choice is hidden in the source instead of being declared in a central place.

In larger codebases, a full text search is not that convenient, and it is hard to track which modules are actually exported.

Paul_Cantrell · 2023-09-22T14:57:16.827Z

There was a partial answer here:

xymus:

I think it would be a good improvement for the package manifest to have a way to declare which dependencies should be public or hidden. The compiler could then report any mismatch. It may be more relevant once the package manager supports library-evolution. Until then, in packages this feature is mostly relevant for the per file type-checking.

I do think it would be worth just a smidge of lookahead to think about how module-level and package-level export declarations should/would interact with this feature.

For example: it would be reasonable to argue that package manifests MUST (in the RFC 2119 sense) explicitly declare public exports in order for them to be declared public in individual files. Is that in fact desirable? If so, what’s the migration path? Does the migration path get complicated if this SE is accepted first?

xymus · 2023-09-23T18:39:40.714Z

Tino:

In larger codebases, a full text search is not that convenient, and it is hard to track which modules are actually exported.

Paul_Cantrell:

I do think it would be worth just a smidge of lookahead to think about how module-level and package-level export declarations should/would interact with this feature.

It would be interesting to have a general discussion on how to organize many modules in a package. The proposal for the package access-level modifier and discussion touched a similar problem for big packages that may need to be broken down into layers or groups. For this proposal goals, the solution I would lean towards is to declare the layer of each module in the manifest.

There's partial support for something similar in the compiler aligned with the @_spi experimental feature. Each module can be marked as either API or SPI. The compiler then raises an error when an API module imports publicly an SPI module or a private clang module.

I think we could use something similar in packages where we'd mark some modules as being visible to clients and some to be package-internal only. Similarly for dependencies that could be marked as exported or for package-internal use only. For backwards compatibility, modules could default to the visible layer, and optionally be moved to the package-internal layer. The compiler can then enforce how modules are imported in sources from the intent declared in the manifest.

John_McCall · 2023-10-09T18:04:11.768Z

SE-0409 has been accepted with modifications.

John_McCall · 2023-10-09T18:04:18.279Z