1

Hi everyone. The review of SE-0379: Opt-In Reflection Metadata, begins now and runs through December 14, 2022.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager by direct message in the Swift forums.

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at https://github.com/apple/swift-evolution/blob/master/process.md

Thank you for helping improve Swift!

Joe Groff
Review Manager

11 Likes
2

I have a couple of questions that are more to do with the way it's currently written than the content of the proposal itself.

The second one isn't safe because many APIs are black boxes if the guess is wrong, an app might behave not as expected at runtime.

Is there a word missing here?

Conformance to Reflectable should be only allowed at type declarations level, to avoid confusing behavior, when a developer adds conformance on an imported from another module type that doesn't have reflection enabled.

Would this follow the same rules as Sendable, where you can use an extension for this but only in limited cases? I can imagine code of the form

struct MyStruct { ... }

#if swift(>=5.8) // or whenever it lands
extension MyStruct: Reflectable {}
#endif

being desirable for some libraries.

1 Like
3

The goal of ensuring that the data exists when needed is good and this achieves that. However I wonder if the implied change of behaviour for the output of print() wouldn't end up being source-breaking in practice. It might be preferable if the strip-all-but-Reflectable behaviour remained gated by a flag even in Swift 6 mode. This would still improve the availability of reflection metadata, providing the option to remove more of it than before, while being less breaking for those who don't need that change.

Writing clarification question:
The paragraph entitled "Stdlib behavior changes" appears to explain why no changes in current stdlib behaviour are proposed. Given that, would it be clearer to simply title the paragraph "No stdlib behavior changes"?

I also initiated PR #1857 with typo fixes and phrasing suggestions.

Guillaume Lessard

1 Like
4

Is there a word missing here?

Let me try to rephrase this sentence.

Would this follow the same rules as Sendable, where you can use an extension for this but only in limited cases? I can imagine code of the form

I considered following the same approach as Sendable does, but I wasn't able to find an actual use case for that.

For the sake of consistency, I don't mind following the Sendable behavior and allowing conformance to Reflectable on extensions if they are placed in the same source file as the type declaration. PR

1 Like
5

However I wonder if the implied change of behaviour for the output of print() wouldn't end up being source-breaking in practice. It might be preferable if the strip-all-but-Reflectable behaviour remained gated by a flag even in Swift 6 mode.

This was raised several times that developers shouldn't rely on debugging output, for example in a recent decision on SE-369.

The language workgroup is keen to re-emphasize the point made in the proposal that, in line with the nature of debugging output, users should not rely on the best-effort result to be consistent either across platforms or across time on any specific platform

The paragraph entitled "Stdlib behavior changes" appears to explain why no changes in current stdlib behaviour are proposed. Given that, would it be clearer to simply title the paragraph "No stdlib behavior changes"?

This makes sense, fixed in this PR

Thanks for fixing typos!

6

I'm not sure we should equate the output of print() in release mode to "debugging output". I'm sure that many command-line tools would see a behaviour change from this proposal. It seems that stripping information to serve the needs of enormous GUI apps, as seems to be the underlying objective of this proposal, should not be done to the detriment of the small tools written by everyday developers. We want Swift to be useful for all uses cases, not just big apps.

7

This is a valid framing, but it’s not just GUI apps. Anybody distributing their program in any capacity has a reason to care about code size, including Docker containers on server, closed-source libraries on iOS (and eventually Windows?), anyone using a build farm (which, admittedly, is not well-supported right now), and experiments with using Swift in resource-constrained environments, such as embedded or wasm. On the flip side, you have command-line tools using print on non-CustomStringConvertible types to get something human-readable, not even reliably parseable. My impression of the relative size of these aggregate groups is opposite of what your framing implies with “enormous” and “everyday”.

I personally think tying this to a language version is the right compromise. Swift 5 mode is unlikely to go away for a long, long time.

3 Likes
8

A thought: should the runtime warn, once per type, if someone attempts to print a type without the appropriate metadata? It’s not much of a signal but it might help, similar to how NSKeyedArchiver warns if you archive a class without a stable runtime name.

4 Likes
9

I think this proposal solves a legitimate issue for apps that want to strip the reflection metadata by introducing the Reflectable protocol that library author's can use to ensure that reflection metadata is available at runtime for their APIs. What confuses me is that the proposal solves this issue, but then wants to completely flip our defaults for Swift 6. I agree with Guillaume here that this is a very large behavior difference and might actually be a large source break for some programs.

Just to display the behavioral difference of print that will come with this proposal in Swift 6:

struct Dog {
    let name: String
    let age: Int
}

let sparky = Dog(name: "Sparky", age: 2)

// Debug: "Dog(name: "Sparky", age: 2)"
// Release: "Dog()"
print(sparky)

but print doesn't have to output to stdout, you can write applications that may look like:

var dogString = ""
print(sparky, to: &dogString)

// "Dog(name: "Sparky", age: 2)"
print(dogString)

Granted this use case may be less common than the former, but the documentation for this feature doesn't say that this format stable or unstable. Given that this output hasn't been changed for a long time (unless you explicitly change it yourself by adding a CustomStringConvertible conformance), I think this would break a lot of code.

I agree, but they already have control over their code size by stripping this info themselves which is what prompt this proposal because there's no way for APIs to designate that they require this information at runtime (which this proposal solves beautifully).

10

I’ve been thinking why this is sticking with me, and ultimately I think it comes down to wanting good defaults. If we think 80% of projects benefit from this, it would stink if 80% of projects had to add an extra config line to their build commands, package manifest, or Xcode project. On the other hand, if we think only 20% of projects benefit from this, that’s much less motivating, and the benefit to being “batteries-included” for unconfigured projects is more important.

4 Likes
11

I’m really excited for this! Are there any estimates of the expected binary-size reductions?

12

How will the output of String(describing:), print(_:), debugPrint(_:) and dump(_:) for structs, enums and classes without reflection metadata and without CustomString/DebugConvertible look like?

e.g.

struct Foo: Reflectable {
    enum Bar {
        case a
        case b(Int)
        case c(Baz)
    }
    struct Baz {
        var d: String
    }
    var bar: Bar
}
print(Foo(bar: .a))
print(Foo(bar: .b(1)))
print(Foo(bar: .c(Baz(d: "Hello")))

Note that only Foo conforms to Reflectable

I'm a bit worried about custom Error types. Have you considered letting Error inherit from Reflectable?
But even then I'm a bit concerned that one might forget to mark all types stored in the custom errors with Reflectable as well (e.g. the Foo type above). The new defaults in Swift 6 reduce the available information drastically if we are not careful and mark all types used as stored properties. Only at runtime if you print/log such an error you will notice that some information is not available to you.

Maybe we could have something that requires all stored property types to inherit from Reflectable as well (similar to what Sendable does) or let them atomically inherit for nested types.

In SwiftNIO we often wrap our enum based errors in a struct to make it possible to add more cases without breaking API. e.g.

Error inherits from Sendable and all non-public types are automatically Sendable if conformance can be auto synthesised. Therefore no change was required and NIOHTTPObjectAggregatorError automatically conforms to Sendable. If any stored propertiy would not conform to Sendable we would get a compile time warning or error in Swift 6.

On example of such a custom error type which sadly can't conform to Sendable (full explanation why is in the source code at the very bottom) is our generic VerificationError.

The compiler made us aware of this issue through a warning at compile time but also gave us a way to silence the warning through @unchecked Sendable.

1 Like
13

I would think that a lot of the projects that would be marred by this change in the reflection metadata defaults would be ones that have little to no visibility. Personal projects and learner's projects among others. Projects that need the code size reduction will have the tools to achieve it, regardless of whether the default is flipped. However, others who don't need that would then have to change their code (and add extra ceremony) to retain the convenience they currently have.

14

I haven't tested using the actual implementation, but I imagine it'll print something like the following:

Foo(bar: output.Foo.Bar)
Foo(bar: output.Foo.Bar)
Foo(bar: output.Foo.Bar)
1 Like
15

Just to display the behavioral difference of print that will come with this proposal in Swift 6:

struct Dog {
    let name: String
    let age: Int
}

let sparky = Dog(name: "Sparky", age: 2)

// Debug: "Dog(name: "Sparky", age: 2)"
// Release: "Dog()"
print(sparky)

This example isn't quite right because in Swift 6 if a type doesn't conform to Reflectable, reflection metadata emitted for debugging wouldn't be accessible through Nominal Type Descriptor. So it would look like this, even if the debugger has access to reflection metadata:

// Debug: "Dog()"
// Release: "Dog()"

But if you wanted to emphasize the difference between language versions, then yes, it would look like this:

// Swift 5: "Dog(name: "Sparky", age: 2)"
// Swift 6: "Dog()"

I agree with Jordan that the current default behavior may not benefit most developers who just want their apps to be safe and small.

I can't come up with many cases when code like this would be useful for real-life applications.

var dogString = ""
print(sparky, to: &dogString)

In my opinion, the internal state of variables should be used only for debugging, and no logic should depend on it. (I may be wrong, so any examples are welcome)

Types' names are slightly different cases, but they are available without reflection. For instance, In the iOS world API like String(describing:) might be used to register UICollectionViewCell class with runtime, but I don't think that the absence of reflection will break this code.

Those reflection-consuming APIs do not provide any guarantees about their output and make the best effort to do their job. If a developer chooses to depend on their output, they implicitly take risks that the output may change. (More information might be printed, the output may change, etc)

I think the main point here is that some percentage of code might be broken in the short term, but in a long term, developers will gain an opportunity to make sure that this code will never break because of the absence of reflection. And Swift 6 might be the best option to achieve this because developers will need to migrate their codebases to this major version of the language anyway.

1 Like
16

This is quite difficult to say, but accordingly to our rough estimations if the major part of Instagram was written in Swift, safe stripping of reflection metadata would bring ~20-40Mb of binary size reduction.

1 Like
17

Projects that need the code size reduction will have the tools to achieve it, regardless of whether the default is flipped.

I see defaulting to opt-in mode as a trade-off between short-term convenience and long-term efficiency of the language.
Yes, if full reflection is enabled by default in Swift 6, it won't break some code that depends on it for some reason, but the majority of apps that don't want this, won't get any benefits from this feature.
And we will miss this opportunity to do this with a little blood. (Major language version migration, runtime warnings)

So the idea of enabling Opt-In mode by default comes from the understanding that Swift should be safe and efficient by default if it is possible to achieve.

I would think that a lot of the projects that would be marred by this change in the reflection metadata defaults would be ones that have little to no visibility. Personal projects and learner's projects among others.

They won't have to change the source code in any way, this is just a matter of adding -enable-full-reflection-metadata flag.

2 Likes
18

Flipping sides for a moment here, “debugging” includes debugging issues in production reported via logs from users (hence why Error was highlighted above, but not the only such case). Existing code might be dumping a struct into a log via default stringification.

(But IMO that code will continue using Swift 5 mode.)

19
  • What is your evaluation of the proposal?

I find some of the wording in this proposal to be misleading and have strong concerns about this proposal.

Changes for debugging

Since Reflection metadata might be used by the debugger, we propose to always keep that metadata if full emission of debugging information is enabled (with -gdwarf-types or -g flags). However, such Reflection metadata won't be accessible through the nominal type descriptor which will avoid inconsistencies in API behavior between Release and Debug modes.

The wording "might" suggests that reflection metadata is optional for the debugger, but that's not the case. The debugger depends on reflection metadata for any operation that involves displaying any variables or function arguments.

On a more technical note, I see two problems with the proposed mechanism of tying the emission of reflection metadata to the presence of the -g flag:

The presence of a debugging options is not supposed to affect code generation (i.e., the contents of the __TEXT segment). This is a requirement the Swift compiler has inherited from the LLVM project.

More importantly though, this suggestion is conflating the notion of an "unoptimized development build versus with assertions versus release build" with debug info. Many software developers build their releases with full debug info, but just don't ship the debug info together with the application. For example, on Darwin the debug info is linked separately from the binary into a .dSYM bundle, which makes this process very natural. Because debug info isn't allowed to affect code generation, this can be done without affecting the performance of the binary.

A proper design for this feature would partition the reflection metadata into a metadata that goes into the binary and metadata that should go into the debug info. The dsymutil utility currently implements an all-or-nothing variant of this that allows all reflection metadata to be copied in the .dSYM bundle together with the debug info, thus allowing for the binary to be linked without the reflection metadata, while still preserving 100% debugability.

To summarize: I believe this proposal should be reworked to allow for all of the reflection metadata to be salvaged for debugging purposes. I am happy to help coming up with a proper design for this.

7 Likes
20

At first glance, I generally support this feature, but I’m a little weirded out that our second ever marker protocol is introducing dynamic casts (which are supposed to not be supported on marker protocols). On the other hand, if we want that kind of runtime check for reflectability, the alternative is to provide it with functions or static methods, and I’m not really convinced that’s any better.

7 Likes