Skip to content

[Deserialization] Fix error when typealias required by protocol refers to type in @_implementationOnly module #37011

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 23, 2021

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 22, 2021

In the added test case, the typealias refers to the HiddenStruct type in the private module, which is imported as @_implementationOnly. Because the import is @_implementationOnly, during deserialization, we don’t import the private module and hence any reference to the HiddenStruct type fails. In the common deserialization code path, this causes us to skip over the typealias member. However, when creating the protocol conformance, we assume that we can resolve the type to which the typealias refers and thus we are crashing.

If LangOpts.EnableDeserializationRecovery is set to true, we should do our best to recover from such failures so this patch makes the deserialization failure handling more graceful and resolve the right-hand side of the typealias as an ErrorType.

Fixes rdar://72891807

…s to type in @_implementationOnly module

In the added test case, the `typealias` refers to the `HiddenStruct` type in the private module, which is imported as `@_implementationOnly`. Because the import is `@_implementationOnly`, during deserialization, we don’t import the private module and hence any reference to the `HiddenStruct` type fails. In the common deserialization code path, this causes us to skip over the `typealias` member. However, when creating the protocol conformance, we assume that we can resolve the type to which the `typealias` refers and thus we are crashing.

If `LangOpts.EnableDeserializationRecovery` is set to `true`, we should do our best to recover from such failures so this patch makes the deserialization failure handling more graceful and resolve the right-hand side of the `typealias` as an `ErrorType`.

Fixes rdar://72891807
@ahoppen ahoppen requested a review from akyrtzi April 22, 2021 15:02
@ahoppen
Copy link
Member Author

ahoppen commented Apr 22, 2021

@swift-ci Please test

@akyrtzi akyrtzi requested a review from xymus April 22, 2021 23:06
@akyrtzi
Copy link
Contributor

akyrtzi commented Apr 22, 2021

if LangOpts.EnableDeserializationRecovery is set to true, we should do our best to recover from such failures

What are the conditions that this is true, could you clarify?

@akyrtzi
Copy link
Contributor

akyrtzi commented Apr 22, 2021

@bnbarham clarified this is 'true' by default.

Copy link
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

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

This looks good, thank you!

@ahoppen
Copy link
Member Author

ahoppen commented Apr 23, 2021

@swift-ci Please test Windows

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