Skip to content

Serialization: Fix deserializing opaque types details for computed properties and subscripts #69546

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
Nov 2, 2023

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Oct 31, 2023

A client shouldn't know about the underlying type of an opaque type unless it can see the body of the naming decl. Attempting to read it can lead to accessing a hidden dependency and a compiler crash.

This was protected by a check specific to function decls but var decls and subscripts were not handled. To support them we have to move this logic to the writer side where we have access to the full AbstractStorageDecl and write in the swifmodule whether the underlying type should be visible outside of the module.

rdar://117607906

@xymus
Copy link
Contributor Author

xymus commented Oct 31, 2023

@swift-ci Please smoke test

@xymus xymus changed the title Serialization: Fix deserializing opaque types details for computed properties and subscript Serialization: Fix deserializing opaque types details for computed properties and subscripts Oct 31, 2023
@xymus xymus force-pushed the deserialization-opaque-var branch 2 times, most recently from 1329055 to 7cb9e5a Compare October 31, 2023 23:16
@xymus xymus force-pushed the deserialization-opaque-var branch 3 times, most recently from e727b65 to cda347e Compare November 1, 2023 18:03
@xymus
Copy link
Contributor Author

xymus commented Nov 1, 2023

@swift-ci Please smoke test

@xymus
Copy link
Contributor Author

xymus commented Nov 1, 2023

I significantly changed this PR to move the core logic from the reader side to the writer side. The accessors were not always available at deserialization, which lead to unexpected results. Now we determine if the underlying type should be exported at serialization, write it as a bit in the swiftmodule, and use it to decide if we should read the underlying information at deserialization.

…s and subscripts

A client shouldn't know about the underlying type of an opaque type
unless it can see the body of the naming decl. Attempting to read it can
lead to accessing a hidden dependency and a compiler crash.

This was protected by a check specific to function decls but var decls
and subscripts were not handled. To support them we have to move this
logic to the writer side where we have access to the full
AbstractStorageDecl and write in the swifmodule whether the underlying
type should be visible outside of the module.

rdar://117607906
@xymus xymus force-pushed the deserialization-opaque-var branch from cda347e to f2d1627 Compare November 1, 2023 23:04
@xymus
Copy link
Contributor Author

xymus commented Nov 1, 2023

Extracted the test to two new files where one is restricted to macOS hosts.

@swift-ci Please smoke test

@xymus xymus merged commit c10dae9 into swiftlang:main Nov 2, 2023
@xymus xymus deleted the deserialization-opaque-var branch November 2, 2023 15:43
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.

2 participants