Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected @_spi visibility for public function returning C++ type #61472

Closed
GeorgeLyon opened this issue Oct 6, 2022 · 7 comments
Closed

Unexpected @_spi visibility for public function returning C++ type #61472

GeorgeLyon opened this issue Oct 6, 2022 · 7 comments
Assignees
Labels
attributes Feature: Declaration and type attributes bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift compiler The Swift compiler itself multiple modules Flag: An issue whose reproduction requires multiple modules serialization Area → compiler: Serialization & deserialization @_spi Feature → attributes: the @_spi attribute swift 5.8 unexpected error Bug: Unexpected error

Comments

@GeorgeLyon
Copy link

Describe the bug
Commit demonstrating bug: https://github.com/GeorgeLyon/circt/tree/23831ec861f3874cbdc733c30404ce74b892a8a9

I have a C++ CMake target circt/lib/Bindings/Swift/Bridge which add functionality not yet bridged by C++ interop (namely, calling new MLIRContext()). This target uses the C++ type MLIRContext, from a dependent module (MLIRIR) with import_as_ref and retain/release:immortal annotations. I also have a Swift CMake target (CIRCTSwift) at circt/lib/Bindings/Swift/Module which uses C++ interop to depend on Bridge. Finally, I have a test target test/Bindings/Swift which imports CIRCTSwift as a Swift library and attempts use its API.

In CIRCTSwift, there are two public functions: func fourtyTwo() -> Int and func createMLIRContext() -> mlir.MLIRContext, the main difference between them being that mlir.MLIRContext is a C++ imported with import_as_ref. Attempting to compile this project produces the following error when calling createMLIRContext, but no similar error is produced when only calling fourtyTwo:

[build] /workspaces/circt-workspace/circt/test/Bindings/Swift/Bindings.swift:19:15: error: 'createMLIRContext' is inaccessible due to '@_spi' protection level
[build] let context = createMLIRContext()
[build]               ^
[build] /workspaces/circt-workspace/circt/lib/Bindings/Swift/Module/MLIRContext.swift:8:13: note: 'createMLIRContext()' declared here
[build] public func createMLIRContext() -> mlir.MLIRContext {
[build]             ^

Steps To Reproduce
The easiest way to reproduce this behavior is to

  1. Open up the project in VSCode with Docker running.
  2. Execute the VSCode command "Rebuild and reopen this project in a container".
  3. Once the container is opened, execute the "CMake: Configure" command.
  4. Build the circt-swift-bindings-test target
    ...

Expected behavior
The API is callable

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please fill out the following information)
This uses Ubuntu 22.04 and the Swift 5.8 snapshot at https://download.swift.org/development/ubuntu2204/swift-DEVELOPMENT-SNAPSHOT-2022-10-03-a/swift-DEVELOPMENT-SNAPSHOT-2022-10-03-a-ubuntu22.04.tar.gz

CMake settings can befound in .decontainer/VSCode/settings.json

@GeorgeLyon GeorgeLyon added the bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. label Oct 6, 2022
@GeorgeLyon
Copy link
Author

@egorzhdan @hyp

@hyp hyp added the c++ interop Feature: Interoperability with C++ label Oct 6, 2022
@hyp
Copy link
Contributor

hyp commented Oct 7, 2022

Assigning to @zoecarver as he knows what the issue is and will work on fixing it with @beccadax .

@GeorgeLyon
Copy link
Author

@zoecarver thinks this bug is the result of a bug related to the order modules are loaded. If I'm understanding correctly, because Clang modules are loaded later in the pipeline there is code that gets confused and thinks all of that API is @_spi (because what else can it be?!). The current workaround is to call a function from the C++ module before calling any API which depends on those types to ensure that the module is loaded early enough for the first pass to pick it up.

@zoecarver
Copy link
Contributor

Yep, pretty much!

In more depth: Swift goes to lookup function swiftFunction() -> Namespace.Record. It finds the function in the serialized module. It goes to deserialize the return type. It hasn't loaded the clang module yet [1], so it cannot find the return type. It fails to deserialize the function, and reports that it cannot find the function. Swift then looks for the function in other places, which has the side effect of loading the clang modules (and populating the bridging header's lookup table, among other things). It's a swift function, so it can't find it in the clang modules, and still reports that there are no lookup results.

Then, to produce a nice error, Swift does another lookup, this time ignoring the access specifiers. This time the clang modules have been loaded though, so it does find the function in the serialized module! The Swift compiler then says, "this is a public decl, I didn't find anything the first time, but did find something when ignoring access specifiers, so it must be SPI." That's why you get that error.

[1] Normally, what would happen here, is Swift would see that it hasn't loaded the clang module that's be referenced and it would go load that module. However, namespaces are not owned by a module, so they go in the bridging header module (__ObjC). There's nothing to load there (that lookup table gets filled in as a side effect of loading other modules), so we fail to lookup the type.

hyp added a commit to hyp/swift that referenced this issue Oct 14, 2022
furby-tm added a commit to wabiverse/SwiftUSD that referenced this issue Nov 20, 2023
* There is/was a bug which unexpectedly threw an error when using
  C++ types in Swift, which caused the Swift compiler to declare the
  type as inaccessible due to '@_spi' protection level, even though
  nothing is marked with a '@_spi' protection level.
* The issue for reference can be found on GitHub for @apple/swift:
  swiftlang/swift#61472
* It is marked as completed, however, it appears this is still a current
  bug, reproducable (only for some types?) when defining typealiases
  in Swift to CXX types which "hop" the namespace so-to-speak.
* Example: public typealias SomeType = cxxnamespace.CxxType,
  attempting to use SomeType from a swift module that imports the
  module defining the typealias triggers "inaccessible due to '@_spi'
  protection level".
@furby-tm
Copy link

furby-tm commented Nov 20, 2023

Sorry to add noise on a potentially fixed issue; however, I was able to reproduce this issue when attempting to "hop" a c++ namespace from a swift namespaced typealias.

Note

It is inconsistent and seemingly random, with which C++ type it would deem unexpectedly inaccessible from a given swift namespace. Though, pretty consistently it seemed to happen most frequently from a struct that was defined entirely in a header file, implementation included.

Example:

// SwiftPMPackageA/Sources/ModuleA/ModuleA.swift

import SomeCxxLib

// a module defining this typealias
public typealias SomeCxxType = __a_verylong__cxxnamespace.SomeCxxType

// and this typealias, namespaced in swift by CxxTypes
public struct CxxTypes
{
  public typealias CxxType = SomeCxxType
}
// SwiftPMPackageB includes SwiftPMPackageA as a package dependency,
// and adds the SwiftPMPackageA.ModuleA target as a dependency of
// SwiftPMPackageB.ModuleB using the following:
// .product(name: "ModuleA", package: "SwiftPMPackageA")

// SwiftPMPackageB/Sources/ModuleB/ModuleB.swift
import ModuleA

// would cause SomeCxxType to produce error: 'SomeCxxType' is inaccessible due to '@_spi' protection level
SomeCxxType.doSomething()

// and would also cause CxxTypes to produce error: type 'CxxTypes' has no member 'CxxType'
CxxTypes.CxxType.doSomething()

Swift version:

swift-driver version: 1.87.2 Apple Swift version 5.9 (swiftlang-5.9.2.1.6 clang-1500.1.0.1.1)
Target: arm64-apple-macosx14.0

I believe what @zoecarver said is spot on, and possibly it is the case that additional "hops" to search for this Namespace.Record can unexpectedly produce this underlying bug, introducing the possibility of such edge cases. Though, perhaps it is more of an intended feature in compiler optimization to keep this search for a Namespace.Record relatively low?

@masters3d
Copy link
Contributor

@egorzhdan @zoecarver could we reopen this for type alias issues stated above? Or would you prefer a new issue filed?

@furby-tm
Copy link

furby-tm commented Jan 15, 2024

I can confirm this bug is still prevalent when using type aliases in the latest release though I have not yet tried this with a prerelease version of swift. To reproduce this issue specifically, you should be able to do so when creating swift type aliases to a C++ namespace (swift enum) in particular, and the most crucial part is to then attempt to use this typealias from a SwiftPM package that depends on the package which declares said swift typealias to a C++ namespace, it is in this consuming package you should receive the following error:

error: "MySwiftTypealiasToCXXNamespace" is inaccessible due to '@_spi' protection level.

Oddly enough, in the example above the only symbol Swift does allow you to use without error is:

__a_verylong__cxxnamespace.SomeCxxType.doSomething()

Which basically means it is for some reason regarding a Swift library that extends a C++ library as inaccessible due to '@_spi' protection level while only making the underlying C++ library from which it imports as public, which if anything, should probably be the opposite of this effect, (I can see many C++ libraries getting extended for Swift, with Swift types like String instead of std.string or UnsafeMutablePointer for example, providing safe Swift libraries and APIs of the more dangerous 'raw C++' libraries from which they wrap).

I know the Swift team is very busy, and I can additionally create an example project to demonstrate this bug if it can help in any way, thank you!

@AnthonyLatsis AnthonyLatsis added swift 5.8 c++ to swift Feature → c++ interop: c++ to swift attributes Feature: Declaration and type attributes unexpected error Bug: Unexpected error @_spi Feature → attributes: the @_spi attribute compiler The Swift compiler itself serialization Area → compiler: Serialization & deserialization multiple modules Flag: An issue whose reproduction requires multiple modules labels Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attributes Feature: Declaration and type attributes bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift compiler The Swift compiler itself multiple modules Flag: An issue whose reproduction requires multiple modules serialization Area → compiler: Serialization & deserialization @_spi Feature → attributes: the @_spi attribute swift 5.8 unexpected error Bug: Unexpected error
Projects
None yet
Development

No branches or pull requests

6 participants