Skip to content

Update reflection on 4.0 to match code in master #9242

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

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented May 3, 2017

  • Description:

    • This is a series of patches which improve remote reflection:
      • Format version 3 brings a new way of recording the superclass of a class directly, instead of a fake "super" associated type on an AnyObject conformance.
      • Support for subclass existentials, which are enabled by default in the 4.0 branch.
      • Fixes for rdar://problem/31661662, rdar://problem/31668126, rdar://problem/30398155 and rdar://problem/31961386, which are some of the top crashes we've been seeing here.
      • To make the above changes work, I had to cherry pick a couple of the mangling-related patches from the AnyObject removal, because they fixed some problems with subclass existential mangling also, in particular getting a mangled name from runtime metadata for a subclass existential type, and printing such a mangling as a human-readable string. While the AnyObject removal was intimately connected to subclass existentials, note that these changes do not introduce the new primitive AnyObject type; this continues to be a protocol in the 4.0 branch.
  • Scope of the issue: the top crashes have been coming up a lot, and subclass existential support in reflection is important because this is a new feature we just introduced.

  • Origination: The crash with private protocols was introduced by the RemoteAST work done in Swift 3.1, so it is a regression from Swift 3.0 to 3.1. The support for subclass existentials is needed because subclass existentials were introduced in 4.0.

  • Risk: Very low risk for anything other than reflection. Medium risk for reflection since there is a lot of new code; I would have liked to get it in two days ago, but was hitting CI issues and I'm hoping it should not be a problem to get it in now.

  • Tested: New tests added for the new features and bug fixes.

  • Reviewed by: @bitjammer and @rjmccall

@slavapestov slavapestov force-pushed the reflection-syncup-4.0 branch from d64ef31 to 9150a9b Compare May 3, 2017 23:09
slavapestov added 10 commits May 4, 2017 16:05
…ld type does not demangle

Better to fail instead of producing an invalid layout.
…ed type of AnyObject

The "superclass as associated type" modeling was put in to
maintain backward compatibility.

We just bumped the version number because of new mangling so
we may as well fix this sillyness too.
…otocol member

Fixes <rdar://problem/31661662>, <rdar://problem/31668126>,
and probably many other crashes with the same backtrace.
I don't have a test case, but this will address the
crashes in <rdar://problem/30398155> and
<rdar://problem/31961386>.
@slavapestov slavapestov force-pushed the reflection-syncup-4.0 branch from 9150a9b to 1494564 Compare May 4, 2017 23:11
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@bitjammer @rjmccall Can you please review this for 4.0?

@bitjammer
Copy link
Contributor

I'm out and about right now but will take a look when I get home tonight.

@rjmccall
Copy link
Contributor

rjmccall commented May 5, 2017

LGTM.

@ematejska ematejska merged commit e530413 into swiftlang:swift-4.0-branch May 5, 2017
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.

4 participants