For Swift 5.10, mark Message/Enum _protobuf_nameMap nonisolated(unsafe)
#1564
Conversation
…fe)` To cut down on the generate code, only conditional the decls, and declare a new static function do the the actual construction.
| p.print("\(visibility)static let _protobuf_nameMap: \(namer.swiftProtobufModulePrefix)_NameMap = [") | ||
| p.print(""" | ||
| #if swift(>=5.10) | ||
| \(visibility)static nonisolated(unsafe) let _protobuf_nameMap: \(namer.swiftProtobufModulePrefix)_NameMap = _makeNameMap() |
There was a problem hiding this comment.
The fact that these might be public makes me a touch nervous. Why are these public again?
There was a problem hiding this comment.
I agree. Since they are public we cannot use nonisolated(unsafe) here since anybody could grab those; however, from looking at _NameMap this appears to be an immutable struct and the only reason that it isn't Sendable right now is due to the storage of a few UnsafeRawBufferPointer. We really should make the underlying type Sendable here by either coming up with an implementation that is Sendable by default or by double-checking that this is indeed immutable and then adding an @unchecked Sendable to the type.
There was a problem hiding this comment.
I think revisiting the impl given how String has changed since the Swift 3 days (which is where I believe @tbkka _NameMap work goes back to) is likely a good idea, that's why I opened #1561.
If we push ahead with that in the coming weeks, this would likely just be sorter lived.
As for why they are public, it's because they are an api point that the Runtime needs to be able to invoke, and I don't believe we ever found a better way to handle that – i.e. - generated protocol conformance that the core runtime can call things on.
There was a problem hiding this comment.
It's unfortunate that the need for these to satisfy a protocol witness forces them to be public. Ideally these conformances would be on a type that didn't need to be public, though I understand why they are.
There was a problem hiding this comment.
Until then I would recommend using @unchecked Sendable on the type instead of nonisolated(unsafe) let on the global
There was a problem hiding this comment.
Until then I would recommend using
@unchecked Sendableon the type instead ofnonisolated(unsafe) leton the global
Is that likely to have to ripple through the support types also? i.e. - Name, InternPool?
I'm doing this somewhat blind (need to do Xcode updates to locally test), so this PR might have to sit a little while until so I can look at switching it over.
I think this is the rest of the issues in apple#1560 This is the different approach discussed in apple#1564.
|
Closing this as I believe #1577 is the desired direction now. |
I think this is the rest of the issues in apple#1560 This is the different approach discussed in apple#1564.
I think this is the rest of the issues in apple#1560 This is the different approach discussed in apple#1564.
I think this is the rest of the issues in apple#1560 This is the different approach discussed in apple#1564.
I think this is the rest of the issues in #1560