Disable implementationOnly Foundation import for resource accessor

[neonichu]

This was a change that landed in #5728 but it has the unintended consequence of generating unfixable warnings for packages which do import Foundation. We can probably solve that with import scanning to decide between the two import types, but given where we are in the 5.8 schedule, this seems to be something better done for a future manifest version.

See also #5991

[neonichu]

@swift-ci please smoke test

[neonichu]

If we decide to merge this, we'll also need a 5.8 cherry-pick, of course.

[MaxDesiatov]

We can probably solve that with import scanning to decide between the two import types

Can you elaborate on this? How exactly would this work?

[neonichu]

Can you elaborate on this? How exactly would this work?

I was thinking we do an import scan on all files of any modules that contains resources and if we find imports of Foundation, we would generate code with a regular import instead of the implementation-only one.

[MaxDesiatov]

Is there some specific tooling already for this type of scanning, or do you think that a simple textual search without even parsing source files would be sufficient?

[neonichu]

Is there some specific tooling already for this type of scanning, or do you think that a simple textual search without even parsing source files would be sufficient?

Yah, we have SwiftcImportScanner which does this by shelling out to the compiler.

[xwu]

Yah, we have SwiftcImportScanner which does this by shelling out to the compiler.

We may need to augment the compiler so as to distinguish @_implementationOnly imports from regular ones; otherwise, we would be reproducing the same problem but flipping the polarity (in other words, emitting a non-@_implementationOnly import that causes an unsilenceable warning for packages that only import Foundation as an implementation-only package).

Also (just a brain dump here) this will need to be kept in sync with future Foundation modularization—a user may down the line import one of the future packages like FoundationEssentials and the invocation of the scanner here will need to know what Bundle is a part of.

One alternative which I anticipate will be less high-maintenance would be an augmentation of @_implementationOnly spelled something like @_implementationOnly(overridable) which tells the compiler that this “access level” (recall Jordan once pitched this as an “internal” import) can be superseded.

[neonichu]

otherwise, we would be reproducing the same problem but flipping the polarity

That is true, but also a pre-existing problem that as far as I know hasn't been reported so far. Because of that, I think it would be ok to ignore for now.

I think for 5.8, the most sensible thing is to go back to the 5.7 behavior and revisit for 5.9+ in some other form. Other thoughts?

[xwu]

Agree, probably best to roll back for now.

[neonichu]

@swift-ci please smoke test