apple / swift-crypto

Open-source implementation of a substantial portion of the API of Apple CryptoKit suitable for use on Linux platforms.
https://apple.github.io/swift-crypto
Apache License 2.0
1.47k stars 166 forks source link

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

Open 2horse9sun opened 1 week ago

2horse9sun commented 1 week ago

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: https://github.com/swiftlang/swift/issues/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 commented 1 week ago

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?