I agree there is room for improvement over the all-or-nothing approach to including metadata we have now, but I have a number of problems with this approach that make it a -1 for me.
(whoops, bit of a wall of text, sorry )
Let's start with the motivation section, it mentions the two underlying problems that the proposal ultimately seeks to address:
First: Metadata "may simplify reverse-engineering". In the (admittedly limited) amount of reverse-engineering Darwin apps I've done, I mostly oriented myself by symbols, so by mangled function names. I guess looking at metadata might help a little but I don't think the bit of security-by-obscurity provided by omitting it is a deal breaker or will deter many reverse-engineering attempts.
Second: Metadata "unnecessarily increases the binary size". Based on anecdotal evidence by @benpious and me back in the pitch thread the size increase seems falls somewhere between 5-10% of the binary size. The 20-40mb for Instagram (245mb download size) mentioned in this thread are a bit bigger than that, but still in the same ballpark. I think it would be good if the proposal gave a more informed picture of potential space savings in the motivation section.
So, I don't really agree with the reverse-engineering part of the motivation. Binary size on the other hand is a bit of a problem area for Swift.
Logging / String Interpolation
First of all, the proposal mentions
dump as having altered behaviour for types that are not marked
Reflectable, but I do wanna point out that string interpolation is affected as well (
String(reflecting:) too, but that is obvious).
Now, I log a lot of stuff in my apps/server code, both in development and production (with dynamically configured log levels). With the changes in this proposal, every time I want to include some value in a log message, I would need to remember to check whether that type is already marked
Reflectable, has a custom string representation, or add the conformance if required.
If the type in question is not from my code, but from some dependency that neither marked the type
Reflectable nor provided a custom string representation, I'm out of luck completely, and need to add some custom description computed variable that I use when I log that type.
So far, I'm only complaining about annoyances for me. Maybe most developers don't work that way. But there is one group, like @glessard alluded to, that in my experience uses logs a lot: learners.
When I see developers new to a language or framework struggle with something, they almost always start logging a bunch of stuff. How you log things is often among the first things you learn about a new language. I distinctly remember that when I started learning and adopting Swift, the ability to get useful descriptions of values without any extra effort was one of my favourite features. Maybe number one. Seeing how much I'm writing here it might still be :D
Requiring developers to know about reflection metadata, what it is used for, and to mark their types
Reflectable for something as simple as logging a value is imo a violation of progressive disclosure for learners, and quite the annoying bit of ceremony for everyone else.
The proposal states multiple times that in opt-in-mode, in Swift 6, use of reflection API would cause compile-time errors:
For modules that have the metadata disabled, but are consumers of reflectable API, the compiler will emit the error enforcing the guarantee.
[...] code with types that has not been audited to conform to the Reflectable protocol will fail to compile if used with APIs that consume the reflection metadata.
These sentences fail to take into account this earlier part of the proposal: "We intentionally do not propose adding a Reflectable constraint on Mirror type", as well as the implication that
print etc. won't require
This means that, when moving an existing Swift 5 codebase to Swift 6, where that codebase uses
Mirror, some abstraction over it,
print and friends, string interpolation or some other package reading the metadata directly, nothing will fail to compile (unless you use them through dependencies that you updated and that were already changed to require
Reflectable), but things will misbehave at runtime.
You might not even notice right away. Maybe this only materializes a month later while debugging some problem that only occurs in production. Sure would be nice to know what exactly is happening, but all you see in your logs is "SomeErrorWithVeryInformativeProperties()" and now you need to go add a comma, a "
Reflectable", push a new build of the app and wait for the error to occur again before you can continue debugging. This could reasonably be called data loss.
I don't think such a silent change in behaviour is acceptable, even for a major language version. If
print and string interpolation required either
Custom(Debug)?StringConvertible, the only silent changes left would be custom code reading the metadata and types that don't get reflected directly but as a child of another type. That still doesn't sound ideal but much better than the proposed version.
I think the drawbacks of this proposal in its current form a too big to accept. I think this is amplified by the fact that they have more impact on learners, while the proposals wins mostly (only? I don't really care about the 1-2mb it might save for our apps) benefit large code bases with large development teams, which should be well equipped with experienced developers and person-hours to figure out what tradeoff they want wrt. including metadata.
The proposed escape hatch of passing
-enable-full-reflection-metadata doesn't solve the progressive disclosure problem (and to my taste skirts the line to introducing a dialect anyway).
Imo, this proposal would be much improved by keeping the default as implicitly conforming all types to
Reflectable, by requiring proper parameter conformances for the types and functions mentioned above, and by making opt-in mode either a flag, or maybe possibly by using a
@noReflection attribute on types (and/or individual fields?) instead.