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

Remove inlinable from init() to support library evolution on linux #294

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

2horse9sun
Copy link

Motivation:

I'm currently working on a swift package project which depends on the swift-crypto library. The package is distributed to other teams and built on multiple platforms (macos + linux). Most importantly, the package is required to enable swift library evolution.

However, after the library evolution is enabled, the linux build failed with following error:

error: emit-module command failed with exit code 1 (use -v to see invocation)
/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/Ed25519_boring.swift:90:27: error: 'self' used before 'self.init' call or assignment to 'self'
 88 |         @inlinable
 89 |         init<D: ContiguousBytes>(rawRepresentation: D) throws {
 90 |             self.keyBytes = try rawRepresentation.withUnsafeBytes { keyBytesPtr in
    |                           `- error: 'self' used before 'self.init' call or assignment to 'self'
 91 |                 guard keyBytesPtr.count == 32 else {
 92 |                     throw CryptoKitError.incorrectKeySize

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/Ed25519_boring.swift:96:9: error: 'self.init' isn't called on all paths before returning from initializer
 94 |                 return Array(keyBytesPtr)
 95 |             }
 96 |         }
    |         `- error: 'self.init' isn't called on all paths before returning from initializer
 97 | 
 98 |         init(_ keyBytes: [UInt8]) {

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/NISTCurvesKeys_boring.swift:109:18: error: 'self' used before 'self.init' call or assignment to 'self'
107 |     @inlinable
108 |     init(wrapping key: BoringSSLECPublicKeyWrapper<Curve>) {
109 |         self.key = key
    |                  `- error: 'self' used before 'self.init' call or assignment to 'self'
110 |     }
111 | 

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/NISTCurvesKeys_boring.swift:110:5: error: 'self.init' isn't called on all paths before returning from initializer
108 |     init(wrapping key: BoringSSLECPublicKeyWrapper<Curve>) {
109 |         self.key = key
110 |     }
    |     `- error: 'self.init' isn't called on all paths before returning from initializer
111 | 
112 |     @inlinable

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/X25519Keys_boring.swift:32:27: error: 'self' used before 'self.init' call or assignment to 'self'
 30 |         @inlinable
 31 |         init<D: ContiguousBytes>(rawRepresentation: D) throws {
 32 |             self.keyBytes = try rawRepresentation.withUnsafeBytes { dataPtr in
    |                           `- error: 'self' used before 'self.init' call or assignment to 'self'
 33 |                 guard dataPtr.count == Curve25519.KeyAgreement.keySizeBytes else {
 34 |                     throw CryptoKitError.incorrectKeySize

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/X25519Keys_boring.swift:39:9: error: 'self.init' isn't called on all paths before returning from initializer
 37 |                 return Array(dataPtr)
 38 |             }
 39 |         }
    |         `- error: 'self.init' isn't called on all paths before returning from initializer
 40 | 
 41 |         @usableFromInline
/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/NISTCurvesKeys_boring.swift:109:18: error: 'self' used before 'self.init' call or assignment to 'self'
107 |     @inlinable
108 |     init(wrapping key: BoringSSLECPublicKeyWrapper<Curve>) {
109 |         self.key = key
    |                  `- error: 'self' used before 'self.init' call or assignment to 'self'
110 |     }
111 | 

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/NISTCurvesKeys_boring.swift:110:5: error: 'self.init' isn't called on all paths before returning from initializer
108 |     init(wrapping key: BoringSSLECPublicKeyWrapper<Curve>) {
109 |         self.key = key
110 |     }
    |     `- error: 'self.init' isn't called on all paths before returning from initializer
111 | 
112 |     @inlinable

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/X25519Keys_boring.swift:32:27: error: 'self' used before 'self.init' call or assignment to 'self'
 30 |         @inlinable
 31 |         init<D: ContiguousBytes>(rawRepresentation: D) throws {
 32 |             self.keyBytes = try rawRepresentation.withUnsafeBytes { dataPtr in
    |                           `- error: 'self' used before 'self.init' call or assignment to 'self'
 33 |                 guard dataPtr.count == Curve25519.KeyAgreement.keySizeBytes else {
 34 |                     throw CryptoKitError.incorrectKeySize

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/X25519Keys_boring.swift:39:9: error: 'self.init' isn't called on all paths before returning from initializer
 37 |                 return Array(dataPtr)
 38 |             }
 39 |         }
    |         `- error: 'self.init' isn't called on all paths before returning from initializer
 40 | 
 41 |         @usableFromInline

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/Ed25519_boring.swift:90:27: error: 'self' used before 'self.init' call or assignment to 'self'
 88 |         @inlinable
 89 |         init<D: ContiguousBytes>(rawRepresentation: D) throws {
 90 |             self.keyBytes = try rawRepresentation.withUnsafeBytes { keyBytesPtr in
    |                           `- error: 'self' used before 'self.init' call or assignment to 'self'
 91 |                 guard keyBytesPtr.count == 32 else {
 92 |                     throw CryptoKitError.incorrectKeySize

/tmp/MyLibrary/.build/checkouts/swift-crypto/Sources/Crypto/Keys/EC/BoringSSL/Ed25519_boring.swift:96:9: error: 'self.init' isn't called on all paths before returning from initializer
 94 |                 return Array(keyBytesPtr)
 95 |             }
 96 |         }
    |         `- error: 'self.init' isn't called on all paths before returning from initializer
 97 | 
 98 |         init(_ keyBytes: [UInt8]) {

The above error can be easily reproduced by:

  1. Create a new swift package project on a Linux OS.
  2. Add swift-crypto to the target dependency
  3. Import Crypto somewhere in the source code
  4. Run this command to build with library evolution: swift build -c release -Xswiftc -emit-module-interface -Xswiftc -enable-library-evolution

Modifications:

After some investigation on this error, I found a useful github issue that may be relevant: swiftlang/swift#62507

As shown in the issue discussion:

it is not safe to have an inlinable public initializer that initializes storage directly in a non-@Frozen struct

Therefore, I just remove "inlinable" from init() of three structs.

Result:

After the modification, it successfully builds on Linux. (At least fix my problem :)

But feel free to give more suggestions on how to support library evolution in the long term instead of just removing these 'inlinable'. Somehow I feel like it's not elegant enough.

@Lukasa Lukasa added the semver/patch No public API change. label Nov 18, 2024
@Lukasa
Copy link
Collaborator

Lukasa commented Nov 18, 2024

Thanks for this ticket!

I'm a little bit surprised to hear you're compiling with library evolution mode on Linux. Swift's ABI is not stable on Linux, and so there is no promise that library evolution mode does anything at all. Can you elaborate a bit on what you're trying to achieve?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants