Skip to content

Restore Prior Behavior for NSCopying Lookup in Subscript Type Mapping #42275

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2022

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Apr 9, 2022

Bear with me, this is going to get weird.

Swift has to import -[NSDictionary objectForKeyedSubscript:] with a special NSCopying
bound that otherwise does not appear in source. This involves the clang importer
looking very specifically for this selector, on this type, in Foundation. The old
behavior was to try to run a type lookup for NSCopying then bail if that failed.
With the introduction of explicit existentials, the failure path changed to check
if the existential itself is NULL. That's currently an impossible condition. What instead
happens is that if your SDK is broken we submit a NULL NSCopying primitive to the
ExistentialType constructor and crash in +asserts compilers.

Quite how you wind up in this position in real
world setups is a mystery - if the Foundation module were somehow corrupt, or the module
cache returned an incoherent type entry lacking a definition for NSCopying we could crash
here. In fact, you can replicate this crash by removing the definition for NSCopying in the
mock SDK and leaving behind a forward declaration!

Since this condition is not representative of an expected setup, restoring the old behavior
sans test is probably the right way to go.

rdar://91062370

@CodaFi CodaFi requested a review from hborla April 9, 2022 00:42
@CodaFi
Copy link
Contributor Author

CodaFi commented Apr 9, 2022

@swift-ci smoke test

Bear with me, this is going to get weird.

Swift has to import -[NSDictionary objectForKeyedSubscript:] with a special NSCopying
bound that otherwise does not appear in source. This involves the clang importer
looking very specifically for this selector, on this type, in Foundation. The old
behavior was to try to run a type lookup for NSCopying then bail if that failed.
With the introduction of explicit existentials, the failure path changed to check
if the existential itself is NULL. That's currently an impossible condition. What instead
happens is that if your SDK is broken we submit a NULL NSCopying primitive to the
ExistentialType constructor and crash in +asserts compilers.

Quite how you wind up in this position in real
world setups is a mystery - if the Foundation module were somehow corrupt, or the module
cache returned an incoherent type entry lacking a definition for NSCopying we could crash
here. In fact, you can replicate this crash by removing the definition for NSCopying in the
mock SDK and leaving behind a forward declaration!

Since this condition is not representative of an expected setup, restoring the old behavior
sans test is probably the right way to go.

rdar://91062370
@CodaFi CodaFi force-pushed the copy-constructor branch from 4fff1cb to a22f57b Compare April 9, 2022 00:50
@CodaFi
Copy link
Contributor Author

CodaFi commented Apr 9, 2022

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 64e9d0b into swiftlang:main Apr 9, 2022
@CodaFi CodaFi deleted the copy-constructor branch April 9, 2022 18:21
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