Skip to content

[Deserialization] Allow lookup into decls that don't have an interface type yet #27410

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 1, 2019
Merged

[Deserialization] Allow lookup into decls that don't have an interface type yet #27410

merged 1 commit into from
Oct 1, 2019

Conversation

harlanhaskins
Copy link
Contributor

@harlanhaskins harlanhaskins commented Sep 27, 2019 •
edited
Loading

Fixes rdar://55560962 and https://bugs.swift.org/browse/SR-11495

This bug was caused because we'd fail to lookup C in B, because we
hadn't computed its type yet and therefore B is filtered from lookup.
Just remove the filter for interface type.

@harlanhaskins harlanhaskins force-pushed the compute-type-before-setting-superclass branch from ce17c56 to ec3f1e2 Compare September 27, 2019 22:41
@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

@jrose-apple
Copy link
Contributor

Deserialization filters out types that don't have an interface type set

Should we just…not do this part? Is it masking other issues? After all, presumably computeType() will go away soon too.

@harlanhaskins
Copy link
Contributor Author

Well, hopefully once computeType() goes away, calling getInterfaceType() inside the filter will be enough to compute the type on demand. And the only other time we use the interface type in the filter is checking it against the expected type when filtering. I think that might be catching some issues?

@jrose-apple
Copy link
Contributor

I mean, why is there a filter at all?

@harlanhaskins
Copy link
Contributor Author

An interface type filter? It doesn't seem to have an effect if I remove the filter. I'm happy to remove the filter instead.

@harlanhaskins harlanhaskins force-pushed the compute-type-before-setting-superclass branch from ec3f1e2 to 236e258 Compare September 28, 2019 00:13
@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ec3f1e2fa83429a3a5b7cdbc1abc8ae649afaab0

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ec3f1e2fa83429a3a5b7cdbc1abc8ae649afaab0

jrose-apple
jrose-apple reviewed Sep 28, 2019
View reviewed changes
jrose-apple
jrose-apple reviewed Sep 28, 2019
View reviewed changes
@harlanhaskins harlanhaskins force-pushed the compute-type-before-setting-superclass branch from 236e258 to ea6e12f Compare September 30, 2019 17:54
jrose-apple
jrose-apple reviewed Sep 30, 2019
View reviewed changes
@harlanhaskins harlanhaskins force-pushed the compute-type-before-setting-superclass branch from ea6e12f to 8825f10 Compare September 30, 2019 18:23
[Deserialization] Allow lookup into decls that don't have an interfac…
67683d8 
…e type yet

Fixes rdar://55560962 and https://bugs.swift.org/browse/SR-11495

This bug was caused because we'd fail to lookup C in B, because we
hadn't computed its type yet and therefore B is filtered from lookup.
Just remove the filter for interface type.
@harlanhaskins harlanhaskins force-pushed the compute-type-before-setting-superclass branch from 8825f10 to 67683d8 Compare September 30, 2019 18:24
@harlanhaskins harlanhaskins changed the title [Deserialization] Call computeType as soon as possible [Deserialization] Allow lookup into decls that don't have an interface type yet Sep 30, 2019
@harlanhaskins
Copy link
Contributor Author

@swift-ci please smoke test

@harlanhaskins
Copy link
Contributor Author

LLDB test failure seems unrelated.

@swift-ci please test macOS

@swiftlang swiftlang deleted a comment from swift-ci Sep 30, 2019
@harlanhaskins
Copy link
Contributor Author

@swift-ci please test source compatibility

@harlanhaskins harlanhaskins merged commit 2da742e into swiftlang:master Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple jrose-apple approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@harlanhaskins @jrose-apple @swift-ci