Skip to content

IRGen: Outlining thunks don't need to receive fixed-size metadata #15972

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 18, 2018

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Apr 17, 2018

There is a general problem where if a public fixed-layout type contains internal or private fields, we have trouble manipulating values of the type from outside the module. This patch doesn't address the issue, but fixes one specific case where we don't need the metadata anyway.

@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ce3df8e95026c37f37a429109883502d5a71a920

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ce3df8e95026c37f37a429109883502d5a71a920

@slavapestov slavapestov force-pushed the fix-outlined-thunks branch from ce3df8e to a2c6f37 Compare April 17, 2018 07:07
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ce3df8e95026c37f37a429109883502d5a71a920

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ce3df8e95026c37f37a429109883502d5a71a920

@slavapestov slavapestov changed the title foo IRGen: Outlining thunks don't need to receive fixed-size metadata Apr 17, 2018
@slavapestov slavapestov requested a review from DougGregor April 17, 2018 07:13
@slavapestov
Copy link
Contributor Author

@DougGregor I'm not going to bother adding the test case unless the source compat suite passes, but it fixes that MyPrivate/OtherPrivate example you showed me.

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This works on my example and appears to work in the larger project I reduced it from (see rdar://problem/39470607), and it looks safe...

@slavapestov slavapestov force-pushed the fix-outlined-thunks branch from a2c6f37 to 9fb0248 Compare April 17, 2018 21:45
They're not used inside the thunk so it's a waste passing them in, and
if they involve private types from a different translation unit we will
get a linking error from referencing the metadata accessor function.

Fixes the test case in <rdar://problem/39470607>, but the more general
problem remains.
@slavapestov slavapestov force-pushed the fix-outlined-thunks branch from 9fb0248 to 39a65ab Compare April 17, 2018 23:11
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov merged commit 8a43338 into swiftlang:master Apr 18, 2018
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