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
[SR-88] Remove old mirrors #42710
Comments
Thanks, Austin! |
Comment by Austin Zheng (JIRA) A cursory search in Xcode seems to indicate that _UTF8ViewMirror isn't declared anywhere in the codebase, although it should be a nested type within String. I'll keep looking later. |
|
Comment by Austin Zheng (JIRA) Ah, thanks. I'll be sure to check the gyb templates the next time this happens. |
Comment by Austin Zheng (JIRA) I'm planning to modify the .gyb and .py files related to generating _Mirror boilerplate. If you'd rather me not let me know. |
Comment by Austin Zheng (JIRA) I have a question. I notice that _Reflectable encompasses both CustomReflectable and CustomPlaygroundQuickLookable's functionality. It looks like there are a couple of _MirrorTypes that exist only to provide quicklook functionality. Can I assume that, quicklook behavior aside, a _MirrorType conformer that is hardcoded to report zero children can be replaced by the default runtime mirror (e.g. the type who the _FoobarMirrorType belongs to doesn't need to conform to CustomReflectable)? If you don't know the answer I'll go through Reflection.mm and try to piece out an answer. Posting this just in case Dmitri or Joe knows off the top of their head. |
Comment by Austin Zheng (JIRA) Final question for tonight: is there an explicit replacement for _MirrorType's "summary" property? As-is, the current Mirror type provides no analogous property, and in most cases the existing custom mirrors just return the object's description. (I need to know this in order to properly update the unit tests.) Thanks! |
Comment by Austin Zheng (JIRA) Still working on this. Hope to have a PR by end of weekend. |
Thanks, Austin! |
Comment by Austin Zheng (JIRA) Finished with the conversion parts of 1 and 2. I need to update/get existing reflection-related tests to run correctly, and increase test coverage. |
Comment by Austin Zheng (JIRA) I'll need a few more days. Manual testing indicate mirrors seem to be working correctly. Still converting unit tests (lots of .summary), fixing bugs, writing new ones. |
Comment by Austin Zheng (JIRA) I haven't dropped this. Still debugging issues with my implementation. |
Comment by Austin Zheng (JIRA) I have most, but not all the tests passing. There's a leak with the generic singleton mirror test (and a few others) I need to troubleshoot. |
Comment by Austin Zheng (JIRA) I narrowed down the cause of the failing unit tests to the following, linked issue (which can be reproduced without using mirrors, reflection, or anything specific to the stdlib). |
Thank you! Could you bring up these issues on the mailing list, CC'ing @jckarter? |
Comment by Austin Zheng (JIRA) PR here: #838 Happy 2016! I understand it's quite big for a PR, but many of the changes are straightforward. I'm happy to break it up if you'd prefer. Preferably a few people can try reviewing it to see if the code conforms to expectations etc. |
Comment by Austin Zheng (JIRA) I have some time this weekend; I'll get in an updated PR in the next few days per comments on the GitHub thread. |
Comment by Austin Zheng (JIRA) I assume #3 is going to be deferred until Swift 3.x. |
Is this resolved, or no longer valid? If either, this should be closed. |
Additional Detail from JIRA
md5: 084e23cc744db09ed5ef4c17b5e5fe0f
is blocked by:
Issue Description:
We need to remove old mirror API: the
_Reflectable
protocol and all related underscored APIs.Plan:
1. All old mirrors in the standard library and SDK overlays should be migrated to the new
CustomReflectable
APIs. All mirrors that didn't have tests and are affected by this conversion should get tests.2.
dump()
function should be ported to use the new mirrors.3. The new mirror API should be migrated off the old
_reflect()
API. The issue is that_reflect()
function and its implementation details depend on the old mirrors. We need to design new runtime entry points that make sense for the new mirrors implementation. This part requires writing a proposal, because the runtime API is going to be stable.4. Remove old underscored mirrors. Remove old runtime entry points.
The text was updated successfully, but these errors were encountered: