Skip to content

[Serialization] Only read the underlying type of an opaque type if inlinable #64991

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 2 commits into from
Apr 7, 2023

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Apr 6, 2023

The underlying type of an opaque type is defined by the function body, as such its knowledge needs to be spread in a similar manner. Clients of a resilient module using opaque types don't need access to the underlying type unless it's used for an inlinable function. Plus, the underlying type can reference internal details which can lead to crashes when deserialization safety is enabled, and in theory when they reference implementation-only dependencies. To realign this behavior, let's only read the underlying type when the opaque type is on an inlinable function.

rdar://105128784


This follows on my previous attempts to limit deserialization failures related to opaque types in #62980 and #63479. Hopefully this approach will prevent any issues even sooner.

@xymus xymus requested review from bnbarham and angela-laar April 6, 2023 23:25
@xymus xymus force-pushed the serial-opaque-types branch from 0ad3da8 to cf47453 Compare April 6, 2023 23:25
@xymus
Copy link
Contributor Author

xymus commented Apr 6, 2023

@swift-ci Please smoke test

…nlinable

Clients of a resilient module using opaque types don't need access to
the underlying type unless it's used in an inlinable context. Plus, the
underlying type can reference internal details which can lead to crashes
when they reference implementation-only dependencies. To clean up this
behavior, let's only serialize the underlying type if used by an
inlinable function.

rdar://105128784
@xymus xymus force-pushed the serial-opaque-types branch from cf47453 to 8ba60da Compare April 7, 2023 14:50
@xymus
Copy link
Contributor Author

xymus commented Apr 7, 2023

@swift-ci Please smoke test

@xymus xymus changed the title [Serialization] Only write the underlying type of an opaque type if inlinable [Serialization] Only read the underlying type of an opaque type if inlinable Apr 7, 2023
@xymus
Copy link
Contributor Author

xymus commented Apr 7, 2023

Moved the logic from the writer to the reader side and fixed availability in the new test. I still have to investigate the correct fix for SILOptimizer/specialize_opaque_result_types2.sil.

When reading a swiftmodule that's part of the main module the compiler
should have access to all internal details. In that case, read the
underlying type to opaque types.

This was caught by `SILOptimizer/specialize_opaque_result_types2.sil`
which has a merge-module like behavior reading it sib files as input.
@xymus
Copy link
Contributor Author

xymus commented Apr 7, 2023

@swift-ci Please smoke 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.

1 participant