Skip to content

Conversation

jmschonfeld
Copy link
Contributor

We currently use _typeByName to "up call" from types in FoundationEssentials to types in FoundationInternationalization (mainly for finding ICU-based versions of types defined in the essentials module). Now that swift-foundation exists in the toolchain, _typeByName is conflating the two versions of FoundationInternationalization when building a copy locally resulting in test crashes and failures. Instead, this transitions to the use of @_dynamicReplacement in order to swap out the up-call function at runtime for an implementation that returns the resulting type. Since the replacement implementation is statically type checked at compile time, it statically references the correct version of the type avoiding the failures.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

}
#endif

private struct UIDNAHookICU: UIDNAHook {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did I misread it or was UIDNAHookICU changed from final class to a struct 👀? Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the only reason it was a final class was so that _typeByName would work (it can only lookup classes). I changed it to a struct to be more in-line with how we typically write swift types now that a class isn't required

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks that makes total sense.

@@ -158,17 +158,16 @@ package protocol UIDNAHook {
static func decode(_ host: some StringProtocol) -> String?
}

dynamic package func _uidnaHook() -> UIDNAHook.Type? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:
From the implementation PR it looks like dynamic replacement would work with properties too. So if we want to keep this as a read-only var I suppose we can keep doing that. I'm fine with this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah I could do that too, I'm fine with either - happy to change it if you have a preference for the variable spelling

}
#endif

private struct UIDNAHookICU: UIDNAHook {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks that makes total sense.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants