swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.29k stars 1.14k forks source link

Use internal import instead of implementationOnly import #5109

Closed parkera closed 1 month ago

parkera commented 1 month ago

Use internal import CoreFoundation instead of @_implementationOnly import CoreFoundation. Resolves #5108

parkera commented 1 month ago

@swift-ci test

parkera commented 1 month ago
/home/build-user/swift-corelibs-foundation/Sources/plutil/main.swift:13:8: error: missing required module 'CoreFoundation'
 11 | import SwiftFoundation
 12 | #elseif canImport(Glibc)
 13 | import Foundation
    |        `- error: missing required module 'CoreFoundation'
 14 | import Glibc
 15 | #elseif canImport(Musl)

Looks like we have something else to sort out here...

clackary commented 1 month ago

Can I also make the request to get this cherrypicked to 6.0 (and even 6.0.2) if possible?

parkera commented 1 month ago

cc @xymus

jmschonfeld commented 1 month ago

I had the same concern as @compnerd - unfortunately @_implementationOnly and internal import are not equivalent for non-resilient modules like Foundation. It sounds like @_implementationOnly fully hid the module which caused the NSLock subclassing bug, but changing it to internal import will cause the module to become visible meaning that we need to install it into the toolchain (which we don't on Windows). It sounds like the right decision is to make this an internal import to fix the subclassing bug, but that likely means we'll need to start installing the CoreFoundation module on Windows - is there a way we can do this while still preventing clients from writing import CoreFoundation in their sources? I'm thinking something like the -allowable-client flag for swift modules but for this clang module

parkera commented 1 month ago

Closing in favor of #5122