search

swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.72k stars 1.34k forks source link

Fix more `InternalImportsByDefault` errors #7905

Closed tshortli closed 1 month ago

tshortli commented 1 month ago

SwiftPM doesn't build with all toolchains when the InternalImportsByDefault upcoming feature is properly enabled.

Motivation:

With the upcoming feature InternalImportsByDefault enabled, some versions of the Swift compiler diagnose this pattern:

import protocol _Concurrency.Actor

// error: package protocol cannot refine an internal protocol
package protocol Proto: Actor {
  // ...
}

Modifications:

Address these diagnostics by using correct access levels for direct imports of _Concurrency.

Result:

SwiftPM builds with all relevant Swift compilers with InternalImportsByDefault enabled.

tshortli commented 1 month ago

This is a follow on to https://github.com/swiftlang/swift-package-manager/pull/7895.

MaxDesiatov commented 1 month ago

@swift-ci test windows

MaxDesiatov commented 1 month ago

IIRC _Concurrency is not imported implicitly when building with CMake, that's why those imports were added in the first place

tshortli commented 1 month ago

IIRC _Concurrency is not imported implicitly when building with CMake, that's why those imports were added in the first place

I see, I wonder if that is due to -disable-implicit-concurrency-module-import getting passed down in CMake builds for some reason? It's hard for me to imagine a reason why that flag would be needed by anything in SwiftPM, maybe it's a mistake?

tshortli commented 1 month ago

@swift-ci please test

tshortli commented 1 month ago

Yeah, the flag seems to be intentional:

> rg disable-implicit-concurrency-module-import
Utilities/bootstrap
671:        swiftpm_args += ["-Xswiftc", "-Xfrontend", "-Xswiftc", "-disable-implicit-concurrency-module-import"]

Sources/swift-bootstrap/main.swift
221:            "-Xfrontend", "-disable-implicit-concurrency-module-import",

Maybe that dates from the days before _Concurrency was available ubiquitously? In any case, I won't question it for now. I've put the imports back and tried to thread the needle differently.

bnbarham commented 1 month ago

Yeah, the flag seems to be intentional:

Can probably remove. It is indeed from before _Concurrency was always available. I recently removed it from the manifest build themselves, I just missed that bootstrap was also doing it.

tshortli commented 1 month ago

@swift-ci please test Windows