Skip to content
New issue

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

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

Already on GitHub? # to your account

NIOConcurrencyHelpers: make LLP64 friendly #1402

Merged
merged 3 commits into from
Feb 19, 2020
Merged

Conversation

compnerd
Copy link
Contributor

On LLP64 platforms, Int is mapped to intptr_t and UInt is mapped
to uintptr_t. Use the newly minted CNIOAtomics operations to
support these semantics.

[One line description of your change]

Motivation:

[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]

Modifications:

[Describe the modifications you've done.]

Result:

[After your change, what will change.]

@swift-server-bot
Copy link

Can one of the admins verify this patch?

9 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@Lukasa
Copy link
Contributor

Lukasa commented Feb 18, 2020

Presumably based on #1401.

@Lukasa Lukasa requested a review from weissi February 18, 2020 09:07
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Feb 18, 2020
@Lukasa Lukasa added this to the 2.15.0 milestone Feb 18, 2020
@@ -400,6 +423,7 @@ extension UInt: AtomicPrimitive {
public static let atomic_load = catmc_atomic_unsigned_long_load
public static let atomic_store = catmc_atomic_unsigned_long_store
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the old-school atomics are deprecated, is it worth making this change? I don't see any value in adding new API surface to types we don't want people to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I ran into build errors without this change. I can try to drop it and get the errors if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind, I think I would. 😄 :

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:132:1: error: type 'Int' does not conform to protocol 'NIOAtomicPrimitive'
extension Int: NIOAtomicPrimitive {
^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:134:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32) -> Void'
    public static let nio_atomic_create_with_existing_storage = catmc_nio_atomic_long_create_with_existing_storage
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:135:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32, Int32) -> Bool'
    public static let nio_atomic_compare_and_exchange         = catmc_nio_atomic_long_compare_and_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:136:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32) -> Int32'
    public static let nio_atomic_add                          = catmc_nio_atomic_long_add
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:137:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32) -> Int32'
    public static let nio_atomic_sub                          = catmc_nio_atomic_long_sub
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:138:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32) -> Int32'
    public static let nio_atomic_exchange                     = catmc_nio_atomic_long_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:139:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>) -> Int32'
    public static let nio_atomic_load                         = catmc_nio_atomic_long_load
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:140:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int32) -> Void'
    public static let nio_atomic_store                        = catmc_nio_atomic_long_store
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:24:16: note: protocol requires property 'nio_atomic_create_with_existing_storage' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int) -> Void' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int) -> ()'); do you want to add a stub?
    static var nio_atomic_create_with_existing_storage: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Void { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:25:16: note: protocol requires property 'nio_atomic_compare_and_exchange' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int, Int) -> Bool' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int, Int) -> Bool'); do you want to add a stub?
    static var nio_atomic_compare_and_exchange: (UnsafeMutablePointer<AtomicWrapper>, Self, Self) -> Bool { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:26:16: note: protocol requires property 'nio_atomic_add' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int) -> Int' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int) -> Int'); do you want to add a stub?
    static var nio_atomic_add: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:27:16: note: protocol requires property 'nio_atomic_sub' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int) -> Int' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int) -> Int'); do you want to add a stub?
    static var nio_atomic_sub: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:28:16: note: protocol requires property 'nio_atomic_exchange' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int) -> Int' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int) -> Int'); do you want to add a stub?
    static var nio_atomic_exchange: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:29:16: note: protocol requires property 'nio_atomic_load' with type '(UnsafeMutablePointer<Int.AtomicWrapper>) -> Int' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>) -> Int'); do you want to add a stub?      static var nio_atomic_load: (UnsafeMutablePointer<AtomicWrapper>) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:30:16: note: protocol requires property 'nio_atomic_store' with type '(UnsafeMutablePointer<Int.AtomicWrapper>, Int) -> Void' (aka '(UnsafeMutablePointer<catmc_nio_atomic_long>, Int) -> ()'); do you want to add a stub?
    static var nio_atomic_store: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Void { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:143:1: error: type 'UInt' does not conform to protocol 'NIOAtomicPrimitive'
extension UInt: NIOAtomicPrimitive {
^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:145:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32) -> Void'
    public static let nio_atomic_create_with_existing_storage = catmc_nio_atomic_unsigned_long_create_with_existing_storage
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:146:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32, UInt32) -> Bool'
    public static let nio_atomic_compare_and_exchange         = catmc_nio_atomic_unsigned_long_compare_and_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:147:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32) -> UInt32'
    public static let nio_atomic_add                          = catmc_nio_atomic_unsigned_long_add
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:148:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32) -> UInt32'
    public static let nio_atomic_sub                          = catmc_nio_atomic_unsigned_long_sub
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:149:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32) -> UInt32'
    public static let nio_atomic_exchange                     = catmc_nio_atomic_unsigned_long_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:150:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>) -> UInt32'
    public static let nio_atomic_load                         = catmc_nio_atomic_unsigned_long_load
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:151:23: note: candidate has non-matching type '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt32) -> Void'
    public static let nio_atomic_store                        = catmc_nio_atomic_unsigned_long_store
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:24:16: note: protocol requires property 'nio_atomic_create_with_existing_storage' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt) -> Void' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt) -> ()'); do you want to add a stub?
    static var nio_atomic_create_with_existing_storage: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Void { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:25:16: note: protocol requires property 'nio_atomic_compare_and_exchange' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt, UInt) -> Bool' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt, UInt) -> Bool'); do you want to add a stub?
    static var nio_atomic_compare_and_exchange: (UnsafeMutablePointer<AtomicWrapper>, Self, Self) -> Bool { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:26:16: note: protocol requires property 'nio_atomic_add' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt) -> UInt' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt) -> UInt'); do you want to add a stub?
    static var nio_atomic_add: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:27:16: note: protocol requires property 'nio_atomic_sub' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt) -> UInt' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt) -> UInt'); do you want to add a stub?
    static var nio_atomic_sub: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:28:16: note: protocol requires property 'nio_atomic_exchange' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt) -> UInt' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt) -> UInt'); do you want to add a stub?
    static var nio_atomic_exchange: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:29:16: note: protocol requires property 'nio_atomic_load' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>) -> UInt' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>) -> UInt'); do you want to add a stub?
    static var nio_atomic_load: (UnsafeMutablePointer<AtomicWrapper>) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\NIOAtomic.swift:30:16: note: protocol requires property 'nio_atomic_store' with type '(UnsafeMutablePointer<UInt.AtomicWrapper>, UInt) -> Void' (aka '(UnsafeMutablePointer<catmc_nio_atomic_unsigned_long>, UInt) -> ()'); do you want to add a stub?
    static var nio_atomic_store: (UnsafeMutablePointer<AtomicWrapper>, Self) -> Void { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:388:1: error: type 'Int' does not conform to protocol 'AtomicPrimitive'extension Int: AtomicPrimitive {
^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:389:23: note: candidate has non-matching type '(Int32) -> OpaquePointer'
    public static let atomic_create               = catmc_atomic_long_create
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:391:23: note: candidate has non-matching type '(OpaquePointer, Int32, Int32) -> Bool'
    public static let atomic_compare_and_exchange = catmc_atomic_long_compare_and_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:392:23: note: candidate has non-matching type '(OpaquePointer, Int32) -> Int32'
    public static let atomic_add                  = catmc_atomic_long_add
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:393:23: note: candidate has non-matching type '(OpaquePointer, Int32) -> Int32'
    public static let atomic_sub                  = catmc_atomic_long_sub
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:394:23: note: candidate has non-matching type '(OpaquePointer, Int32) -> Int32'
    public static let atomic_exchange             = catmc_atomic_long_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:395:23: note: candidate has non-matching type '(OpaquePointer) -> Int32'
    public static let atomic_load                 = catmc_atomic_long_load
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:396:23: note: candidate has non-matching type '(OpaquePointer, Int32) -> Void'
    public static let atomic_store                = catmc_atomic_long_store
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:279:16: note: protocol requires property 'atomic_create' with type '(Int) -> OpaquePointer'; do you want to add a stub?
    static var atomic_create: (Self) -> OpaquePointer { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:281:16: note: protocol requires property 'atomic_compare_and_exchange' with type '(OpaquePointer, Int, Int) -> Bool'; do you want to add a stub?
    static var atomic_compare_and_exchange: (OpaquePointer, Self, Self) -> Bool { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:282:16: note: protocol requires property 'atomic_add' with type '(OpaquePointer, Int) -> Int'; do you want to add a stub?
    static var atomic_add: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:283:16: note: protocol requires property 'atomic_sub' with type '(OpaquePointer, Int) -> Int'; do you want to add a stub?
    static var atomic_sub: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:284:16: note: protocol requires property 'atomic_exchange' with type '(OpaquePointer, Int) -> Int'; do you want to add a stub?
    static var atomic_exchange: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:285:16: note: protocol requires property 'atomic_load' with type '(OpaquePointer) -> Int'; do you want to add a stub?
    static var atomic_load: (OpaquePointer) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:286:16: note: protocol requires property 'atomic_store' with type '(OpaquePointer, Int) -> Void'; do you want to add a stub?
    static var atomic_store: (OpaquePointer, Self) -> Void { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:399:1: error: type 'UInt' does not conform to protocol 'AtomicPrimitive'
extension UInt: AtomicPrimitive {
^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:400:23: note: candidate has non-matching type '(UInt32) -> OpaquePointer'
    public static let atomic_create               = catmc_atomic_unsigned_long_create
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:402:23: note: candidate has non-matching type '(OpaquePointer, UInt32, UInt32) -> Bool'
    public static let atomic_compare_and_exchange = catmc_atomic_unsigned_long_compare_and_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:403:23: note: candidate has non-matching type '(OpaquePointer, UInt32) -> UInt32'
    public static let atomic_add                  = catmc_atomic_unsigned_long_add
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:404:23: note: candidate has non-matching type '(OpaquePointer, UInt32) -> UInt32'
    public static let atomic_sub                  = catmc_atomic_unsigned_long_sub
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:405:23: note: candidate has non-matching type '(OpaquePointer, UInt32) -> UInt32'
    public static let atomic_exchange             = catmc_atomic_unsigned_long_exchange
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:406:23: note: candidate has non-matching type '(OpaquePointer) -> UInt32'
    public static let atomic_load                 = catmc_atomic_unsigned_long_load
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:407:23: note: candidate has non-matching type '(OpaquePointer, UInt32) -> Void'
    public static let atomic_store                = catmc_atomic_unsigned_long_store
                      ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:279:16: note: protocol requires property 'atomic_create' with type '(UInt) -> OpaquePointer'; do you want to add a stub?
    static var atomic_create: (Self) -> OpaquePointer { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:281:16: note: protocol requires property 'atomic_compare_and_exchange' with type '(OpaquePointer, UInt, UInt) -> Bool'; do you want to add a stub?
    static var atomic_compare_and_exchange: (OpaquePointer, Self, Self) -> Bool { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:282:16: note: protocol requires property 'atomic_add' with type '(OpaquePointer, UInt) -> UInt'; do you want to add a stub?
    static var atomic_add: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:283:16: note: protocol requires property 'atomic_sub' with type '(OpaquePointer, UInt) -> UInt'; do you want to add a stub?
    static var atomic_sub: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:284:16: note: protocol requires property 'atomic_exchange' with type '(OpaquePointer, UInt) -> UInt'; do you want to add a stub?
    static var atomic_exchange: (OpaquePointer, Self) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:285:16: note: protocol requires property 'atomic_load' with type '(OpaquePointer) -> UInt'; do you want to add a stub?
    static var atomic_load: (OpaquePointer) -> Self { get }
               ^
S:\SourceCache\swift-nio\Sources\NIOConcurrencyHelpers\atomics.swift:286:16: note: protocol requires property 'atomic_store' with type '(OpaquePointer, UInt) -> Void'; do you want to add a stub?
    static var atomic_store: (OpaquePointer, Self) -> Void { get }
               ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. We could work around it, but frankly I think that's more trouble than it's worth.

On LLP64 platforms, `Int` is mapped to `intptr_t` and `UInt` is mapped
to `uintptr_t`.  Use the newly minted `CNIOAtomics` operations to
support these semantics.
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@weissi weissi merged commit 2c555d1 into apple:master Feb 19, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants