Skip to content

[4.0] [Serialization] Make requirement signature conformance loading lazy. #11661

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

Conversation

jrose-apple
Copy link
Contributor

@jrose-apple jrose-apple commented Aug 28, 2017

  • Explanation: In certain cases, deserialization of protocol conformances that involved inherited protocols would result in circular dependencies, which meant a compiler crash. By delaying some of the work of deserialization, we avoid those issues.
  • Scope: Affects non-WMO builds of modules that contain inheriting protocols and a conforming type with a member that references back to one of the protocols.
  • Issue: SR-5191 / rdar://problem/33804554
  • Reviewed by: @DougGregor
  • Risk: Medium-low. We originally didn't cherry-pick this because delaying deserialization can occasionally have ripple effects, but it's been living in master for a while with no obvious problems, and it clearly does fix some issues.
  • Testing: Added compiler regression test.

While the original commit was expected to have no effective impact, it
turned out to actually solve some circularity issues after all
involving protocols that inherit from other protocols. It should be a
mild compile-time win as well in cases where the requirement signature
is not needed at all.

Test in subsequent commit.

https://bugs.swift.org/browse/SR-5191 / rdar://problem/33804554

(cherry picked from commit 897effe)
…1685)

This was fixed by 897effe, which I had originally thought would be a
no-functionality-change commit because it just made things lazier.
Turns out requirement signature deserialization can result in
circularity with sufficiently cross-referential conformances.

This isn't exactly a reduced test case because it still depends on
subclassing NSObject, which probably means there are hidden
dependencies on conforming to standard library protocols. But it's
better than nothing.

https://bugs.swift.org/browse/SR-5191
@jrose-apple jrose-apple force-pushed the 4.0-lazy-signature-deserialization branch from 1b6f446 to b38500b Compare August 29, 2017 23:53
@jrose-apple
Copy link
Contributor Author

@DougGregor, mind re-reviewing this since it's been a while since the original commit went in?

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b38500b

@jrose-apple
Copy link
Contributor Author

It looks like everything passed and then Jenkins choked right at the end. But compute is cheap during the off season, so…

@swift-ci Please test macOS

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.

LGTM

@jrose-apple
Copy link
Contributor Author

Approved by Ted via email.

@jrose-apple jrose-apple merged commit 31c6cac into swiftlang:swift-4.0-branch Aug 31, 2017
@jrose-apple jrose-apple deleted the 4.0-lazy-signature-deserialization branch August 31, 2017 18:36
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