Skip to content

Conversation

@maxovtsin
Copy link
Contributor

@maxovtsin maxovtsin commented Oct 6, 2020

A prototype for a swift evolution proposal that implements opt-in reflection metadata.

Proposal - swiftlang/swift-evolution#1203
Discussion - https://forums.swift.org/t/pitch-2-opt-in-reflection-metadata/41696

@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from 9ef4a50 to b369f74 Compare October 6, 2020 10:39
@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from 17e147b to bd4f59d Compare October 27, 2020 16:00
@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from bd4f59d to 30f709e Compare November 10, 2020 21:11
maxovtsin added a commit to maxovtsin/swift-evolution that referenced this pull request Nov 10, 2020
@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch 3 times, most recently from ee8e2c9 to 3feb0d9 Compare June 10, 2022 13:39
@maxovtsin maxovtsin changed the title [prototype] Opt-In reflection metadata Swift Opt-In Reflection metadata Jun 10, 2022
@maxovtsin maxovtsin marked this pull request as ready for review June 15, 2022 12:10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protocols should never emit field descriptors because they aren't types by themselves and never have fields. It looks like an oversight that these get emitted in the first place and hopefully nothing will break if we stop emitting them because they're always empty.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Azoy points out that it isn't even possible to find these field descriptors from the protocol descriptor, so yeah, they're probably unused.

Copy link
Contributor Author

@maxovtsin maxovtsin Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that those symbols are used by the debugger? I tried to stop emitting field descriptor for protocols, but several tests started failing (like this one - reflect(object: TestGeneric(D() as P))). Not sure how to check what exactly uses it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protocol "field descriptors" encode whether the protocol is ObjC or not.

@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from 3feb0d9 to 49f17b8 Compare July 13, 2022 16:04
@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from 49f17b8 to e1a220d Compare October 5, 2022 17:13
@maxovtsin maxovtsin marked this pull request as draft October 5, 2022 17:14
@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch 4 times, most recently from 6fbbc69 to 6f148e5 Compare October 17, 2022 07:37
@maxovtsin maxovtsin marked this pull request as ready for review October 17, 2022 12:52
@maxovtsin maxovtsin requested a review from jckarter November 9, 2022 16:59
@mikeash
Copy link
Contributor

mikeash commented Nov 15, 2022

This is really neat! I like how you handle conditional casting to the marker protocol. Does this handle casting to protocol compositions including Reflectable, like as? P & Q & Reflectable? I'm not quite sure what sort of code the compiler would emit for that.

@maxovtsin
Copy link
Contributor Author

This is really neat! I like how you handle conditional casting to the marker protocol. Does this handle casting to protocol compositions including Reflectable, like as? P & Q & Reflectable? I'm not quite sure what sort of code the compiler would emit for that.

Good catch, thanks!
(Fixed by allowing casts to protocol composition types with Reflectable)

@hoangatuan
Copy link

Almost 4 years for this MR..
I think this is really cool and can reduce app size a lot

@maxovtsin maxovtsin force-pushed the opt-in-reflection-metadata branch from db7b541 to bb0263f Compare January 2, 2024 16:37
@slavapestov
Copy link
Contributor

I think instead of adding a new layout constraint (which as you saw requires a lot of new plumbing) this could instead use a marker protocol?

@ahoppen ahoppen removed their request for review January 8, 2024 21:54
@maxovtsin
Copy link
Contributor Author

@slavapestov I used a marker protocol in the first iteration of the implementation to make the logic simpler, but the first review feedback made it clear that marker protocols shouldn't affect runtime behaviour therefore it was suggested to create a new non-protocol generic constraint similar to AnyObject, which requires to handle many cases in the code. 🤷

https://forums.swift.org/t/returned-for-revision-se-0379-opt-in-reflection-metadata/62390

@slavapestov
Copy link
Contributor

In the year since that was written, Copyable, Escapable and BitwiseCopyable were added as marker protocols. I understand the frustration here in going back and forth, but I really think modeling these kinds of relations as conformances makes the most sense, and in hindsight I sometimes wish AnyObject didn't exist as a special case either. CC @jckarter

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.

5 participants