-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add an "addressable for dependencies" flag to value witness flags. #82323
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
Add an "addressable for dependencies" flag to value witness flags. #82323
Conversation
@swift-ci Please test |
a439a52
to
16a919e
Compare
@swift-ci Please test |
16a919e
to
4c5c8e7
Compare
@swift-ci Please test |
This may be useful for type layout of borrow fields in the future, should we decide that addressable-for-dependencies borrows should always be represented by a pointer. rdar://153650278
4c5c8e7
to
7e99e84
Compare
@swift-ci Please test |
@swift-ci Please smoke test Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's the story here for existing value witness tables? |
@swift-ci Please test Linux |
@rjmccall It seems to me that the borrow representation is likely to require new runtime support, so it's likely to be deployment target limited (though hopefully we can still support back deploying static uses that don't need runtime layout). However, there should also at least be a limited universe of |
Is there any problem here for a new OS running existing binaries that haven't been recompiled with the new mode? Do we need a way to recognize that case? |
@rjmccall Thinking about it for a couple hours… existing binaries wouldn't directly make use of new runtime features, but they could call in to the new OS's libraries, and the new OS could be using new language features that interpret this new bit. The setting of the bit could be different from what a new compiler would generate from the new type; however, code in the new OS can't possibly have static knowledge of the old binary it's running with, so it doesn't seem possible for there to be a static/dynamic layout disagreement. And since the old binary was built in a world where So it seems like old binaries would be OK with a new OS, though I could be missing something. Do you have any counterexamples? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. I have some concerns about that impact with existing non-OS libraries, but maybe we can address that with documentation. At any rate, I agree it's better to have this information than not.
The code that's here LGTM, but there's a runtime aspect of this that should at least be tested, and you need to make sure that the preset value witnesses in the runtime have the right values (and that we don't use them for types that disagree).
I was hoping to tackle the runtime updates separately (and not for the 6.2 branch, since I wanted to minimize risk there). As long as we start emitting binaries so that the bit is set properly, it seems to me we should be able to update the runtime later. |
This may be useful for type layout of borrow fields in the future, should we decide that addressable-for-dependencies borrows should always be represented by a pointer. rdar://153650278