Availability and backward deployment for variadic generics [5.9] #65926
Conversation
|
swiftlang/swift-syntax#1664 |
slavapestov
force-pushed
the
availability-and-backdeploy-variadics-5.9
branch
from
430f84c to
91a3971
Compare
|
swiftlang/swift-syntax#1664 |
|
|
||
| SWIFT_RUNTIME_EXPORT SWIFT_CC(swift) | ||
| const Metadata * const * | ||
| swift_allocateMetadataPack(const Metadata * const *ptr, size_t count) { |
There was a problem hiding this comment.
I don't think the uniquing here will work correctly when the Swift runtime in use has swift_allocateMetadataPack (which uses a runtime-internal MetadataPacks cache) and the program was compiled for an earlier deployment target (so this version of swift_allocateMetadataPack will be linked in), because some callers will go through the runtime version and some will go through the compatibility version.
With back-deployed concurrency, I ended up creating swift_getFunctionTypeMetadataGlobalActorBackDeploy in the back-deployment shim. IRGen generates a call to that when building for a back-deployment target, and that function does a dlsym to determine whether swift_getFunctionTypeMetadataGlobalActor is available in the active Swift runtime... if it is, we use it so everyone agrees on the underlying cache. I think you'll need to take a similar approach here.
There was a problem hiding this comment.
Doug and I discussed this offline. Doug is 100% correct in that a binary built against an older deployment target will link against the stub and call the entry points in the stub even on a newer runtime, so we cannot guarantee uniqueness of packs in this scenario. However, pack uniqueness is only a space optimization and not required for correctness; when comparing metadata keys in generic type instantiation we already do a deep equality of pack elements, because we have to deal with on-stack packs. It would still be nice to eek out a tiny bit of memory savings though, so I will address this issue in a follow-up PR. Thanks Doug!
There was a problem hiding this comment.
We've determined that my concern not a correctness problem, but that we might lose out on some uniquing in a back-deployed confirmation and therefore allocate more memory than needed. I'm okay with landing this pull request as-is and we can improve the memory footprint for this configuration as a follow-up.
|
@swift-ci Please test |
swift_allocate{Metadata,WitnessTable}Pack()entry points.