Skip to content

[Serialization] Mark decls that can never be cross-referenced #17223

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

Conversation

jrose-apple
Copy link
Contributor

This allows us to filter them out in cases that would otherwise be ambiguous. The particular prompting situation looks a lot like the test case: a protocol, plus a forward-declared class with the same name. (Normally we ignore forward-declared classes, but SourceKit's module interface generation feature makes dummy ClassDecls for them instead.)

SR-4851, plus possibly other SourceKit crashes in ModuleFile::readConformance.

This allows us to filter them out in cases that would otherwise be
ambiguous. The particular prompting situation looks a lot like the
test case: a protocol, plus a forward-declared class with the same
name. (Normally we ignore forward-declared classes, but SourceKit's
module interface generation feature makes dummy ClassDecls for them
instead.)

https://bugs.swift.org/browse/SR-4851
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

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.

The approach is reasonable for dodging deserialization bugs. Adding an attribute for this feels really heavyweight to solve the problem (vs. a bit), but I guess you did this so we don’t pollute the Decl itself with this one-off thing?

@jrose-apple
Copy link
Contributor Author

That was the idea, yeah. If you'd prefer a bit I can switch to that, but since most decls won't have it I didn't want to claim the space forever.

@jrose-apple jrose-apple merged commit 05d0a2d into swiftlang:master Jun 18, 2018
@jrose-apple jrose-apple deleted the overly-forward branch June 18, 2018 17:50
jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jun 18, 2018
…ang#17223)

This allows us to filter them out in cases that would otherwise be
ambiguous. The particular prompting situation looks a lot like the
test case: a protocol, plus a forward-declared class with the same
name. (Normally we ignore forward-declared classes, but SourceKit's
module interface generation feature makes dummy ClassDecls for them
instead.)

https://bugs.swift.org/browse/SR-4851
(cherry picked from commit 05d0a2d)
jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jun 18, 2018
…ang#17223)

This allows us to filter them out in cases that would otherwise be
ambiguous. The particular prompting situation looks a lot like the
test case: a protocol, plus a forward-declared class with the same
name. (Normally we ignore forward-declared classes, but SourceKit's
module interface generation feature makes dummy ClassDecls for them
instead.)

https://bugs.swift.org/browse/SR-4851
(cherry picked from commit 05d0a2d)
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.

2 participants