[ClangImporter] Make conformance loading lazier. #10707
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
|
@DougGregor: after discussing this with @ematejska we decided the story with a flag didn't make enough sense—either we're comfortable with this for 4.0 or we're not. I'm personally a lot happier than I thought I would be because the lazy mechanism is already there; if I had had to add that for the first time I'd be a lot more apprehensive. |
DougGregor
approved these changes
Jun 29, 2017
|
Failing a number of internal tests. Stay tuned. |
Previously, the importer queued up conformances to complete once it was done importing the current batch of declarations. However, if there was a serialized Swift module that extended an imported type to add a conformance in exactly the wrong way, the importer could end up asking for that conformance later---even before the reference to the imported type was resolved. This led to a crash in the deserializer "while reading conformance for type X". Instead of this "pending actions" queue, we can just use the mechanisms already in place for lazily loading conformances. That way they'll get filled out on demand, which is better all around anyway. This does mean putting the requirement signature into the "lazy" part of the conformance, though. This does as a side effect mean that /all/ of the witnesses for the imported conformance may be opaque---that is, they will never be devirtualized to a particular implementation. However, they previously would have referred to methods implemented in Objective-C anyway, which are always dispatched with objc_msgSend. So this should have no practical effect. rdar://problem/32346184
jrose-apple
force-pushed
the
conformance-procrastination
branch
from
3cd0f13 to
4c18bda
Compare
|
It looks like those are configuration issues on our end, not related to this pull request. Previous full-testing passed. @swift-ci Please smoke test |
The previous commit needed this for the importer, so we might as well take advantage of it in deserialization as well. No expected change in behavior.
jrose-apple
force-pushed
the
conformance-procrastination
branch
from
4c18bda to
897effe
Compare
|
*sigh* Left in some conflict markers. @swift-ci Please smoke test |
|
@DougGregor, do you think we should take the serialization change for 4.0 as well, or just the importer work? |
|
…wrong PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the importer queued up conformances to complete once it was done importing the current batch of declarations. However, if there was a serialized Swift module that extended an imported type to add a conformance in exactly the wrong way, the importer could end up asking for that conformance later—even before the reference to the imported type was resolved. This led to a crash in the deserializer "while reading conformance for type X".
Instead of this "pending actions" queue, we can just use the mechanisms already in place for lazily loading conformances. That way they'll get filled out on demand, which is better all around anyway. This does mean putting the requirement signature into the "lazy" part of the conformance, though.
This does as a side effect mean that all of the witnesses for the imported conformance may be opaque—that is, they will never be devirtualized to a particular implementation. However, they previously would have referred to methods implemented in Objective-C anyway, which are always dispatched with objc_msgSend. So this should have no practical effect.
There's also a second commit that makes deserialization of Swift conformances a little lazier as well, now that we have that ability. This isn't critical to this fix and we could easily leave it for later.
rdar://problem/32346184