[Second Review] SE-0364: Warning for Retroactive Conformances of External Types

Evolution Proposal Reviews
1

Hello, Swift community.

The review of SE-0364: Warning for Retroactive Conformances of External Types ran from July 13th to 27th. The language workgroup accepted in principle that a warning should be added for such conformances, but returned the proposal for revision to explore what the mechanism of silencing the warning should be.

In particular, we wrote:

The proposed mechanism for silencing the warning is to fully-qualify both the type and protocol names in the conformance:

extension SomeModule.Type: SomeOtherModule.Protocol { ... }

This mechanism has the desirable quality that it does not require any new language features; this is a valid conformance in Swift 5.7, and would continue to be a valid conformance in the future. However, during the review period several commenters noted that they would prefer a more heavyweight indication that a conformance is retroactive. In returning for revision, we would like to consider the following alternative mechanism for silencing the warning:

extension Type: @retroactive Protocol { ... }

There are several considerations that might make this a desirable alternative:

  • it is visual distinct, and easy to search for.
  • there is existing support for @unchecked Sendable, so we are not blazing new syntactic or conceptual ground.
  • although all conformances are treated the same by the runtime at present, the information that a conformance is retroactive is available to the runtime. Thus, a future version might give retroactive conformances different semantics in casts in an attempt to resolve some of the issues that are raised in the thread for this proposal. In light of this, giving such conformances a visibly distinct (and searchable) source-level syntax may be appropriate.

Second Review

After some delay, the proposal has been returned for a second round of review, from today until May 9th. That review is limited to the new @retroactive attribute.

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 email. When contacting the review manager directly, please keep the proposal link at the top of the message and put "SE-0364" in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • 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/main/process.md

Thank you,

Steve Canon
Review Manager

10 Likes
2

Perhaps I am missing something, but it seems to me that problems would only arise if both the type and the protocol are from resilient (ABI-stable) libraries, at least one of which gets dynamically linked.

If at least one of the type and/or the protocol is from a non-resilient library (or both are statically linked), why would we want a warning?

3

It's not harmless for these conformances to show up in apps – if the type in question adds this conformance in a future build of e.g. macOS, then the one owned by the framework itself and the one in your project will fight at runtime, and this behavior is undefined.

I think it's worth this attribute to at least signal that the conformance is retroactive in source, and potentially steer apps away from using them. As before, this change includes a fix-it to add the attributes in source.

4

To be clear, this scenario requires that the type be defined in an OS library (or at least, an ABI-stable library that is dynamically linked to the app), and also that the protocol is defined in a similar library (because otherwise, the OS library could never add such a conformance).

Is there any sort of danger if at least one of the protocol or the type does not come from such a library?

5

Right, I think Nevin makes a good point: either we care about all retroactive conformances (including where both the type and the protocol come from non-resilient dependencies), or we only care about conformances that could change out from under us. The former is still interesting from an API evolution / semver perspective, but this proposal had been limited to the latter before, and that only matters in the case where both the type and protocol are resilient.

I will note that there’s still a potential danger case here: a conformance for a subclass (direct or indirect) of a resilient class to a resilient protocol. That’s not exactly “retroactive”, but it’s just as much a problem. So maybe “retroactive” isn’t the right spelling after all.

EDIT: but read strictly, that means that every subclass of NSObject has to say conforming to any OS protocol is “dangerous”, and that’s technically true, but feels like noise.

8 Likes
6

Can an attribute like @retroactive be implemented as a standard library macro with a private hook into the compiler? This would allow Xcode (or other IDEs) to show quick help and navigate to documentation for the attribute in a way that it currently doesn't support for built-in attributes like @resultBuilder and @propertyWrapper, but does support for @ViewBuilder and @Binding (and, I assume, for macros when they ship).

7

Can we make this an error instead of a warning?

Do I understand correctly that @retroactive or equivalent will just silence the warning, and bad things™ would still happen when OS introduces its own conformance?

8

I wrote a bit about why I'd oppose making this an error here SE-0364: Warning for Retroactive Conformances of External Types - #53 by rex-remind

Notable that I see this most often either when (a) using a test framework requires use of a trait (Rust's version of a protocol) and source and tests are in different modules or (b) using a library that requires a trait but the library's trait does not conform to common used types like uuid or does not conform one of its own types because the authors didn't find it necessary but devs may find it useful. I've even seen similar conflicts occur in code multiple teams own because of how they originally chose to organize crate dependencies on internal code even though we own the code (speaking of Rust still fyi). This generally results in adding a newtype which adds noise and harms grepability and consistency over re-waging the "right" dependency structure debates.

Question for the audience: does Swift's use of @testable help avoid (a) in any way?

9

In terms of the review: I think fully-qualified naming is explicit enough, though I prefer the @retroactive attribute given the bullet point about carrying the information which may allow different semantics in the future. This is a sensitive area with large potential knock-on effects if it needs to be further revisited and adjusted, having that at runtime may prove very useful for more nuanced adjustments. I had considered how that may effect code churn, yet realistically most retroactive conformances are not using fully-qualified names currently I'm betting so there's similar churn either way.

10

@testable is IMO broken by design - it requires building the thing under test in a mode where all internal symbols are exported, so either you're not testing what you ship or you're shipping something suboptimal to facilitate testing. Anything which would rely on it would therefore also be a bad idea.

11

Making something a warning while adding an attribute which does nothing but silence the warning feels somewhat silly to me. Since Swift doesn't have a way to turn off specific warnings entirely, the reason to make something a warning rather than an error is to allow for someone to look at the warning and decide that living with the warning is better than changing the code to silence the warning. When silencing the warning is as trivial as it is here, it feels like it could just be an error with a fix-it suggesting to add @retroactive.

For migrational purposes I suppose it'd need to be a warning in Swift 5 mode and only promoted to an error in Swift 6. Maybe it's just not worth bothering with that? It not being an error feels silly but it's definitely not a big deal.

2 Likes
12

Don't entirely agree that use of @testable is in general an all or nothing scenario for what equates to bad or good, however on separate terms I do agree that not having to rely on @testable is better than having to rely on it as a compromise to what's relevant to this proposal and what you're proposing.

13

the reason to make something a warning rather than an error is to allow for someone to look at the warning and decide that living with the warning is better than changing the code to silence the warning

I don't know if this necessarily aligns with the compiler's use of warnings. My understanding is that the compiler produces warnings when it detects that a bit of code may have unintended consequences or incorrect behavior, and certain classes of warnings provide a means of silencing as a way for the developer to assert they know more than the compiler can know.

I don't think it's a goal for Swift to provide warnings that developers just live with.

2 Likes
14

we only care about conformances that could change out from under us.

There's an argument to be made that this should be relaxed from "same module" to "same package", would that be a satisfying change to this? When this was originally written, package access didn't exist as a concept, but I would be fine loosening this slightly.

7 Likes
15

Given that it has warnings, that clearly is a goal. The only difference between a silenceable warning and a silenceable error is that eliminating the warning is optional, and so the only reason to make something a warning instead of an error is to give developers the option to not fix it right away.

16

I don't know if I agree with that. The difference between a warning or an error, in my experience, is provable harm. Warnings show things that may be harmful, errors prevent things that are known to be harmful in all cases.

1 Like
17

Most errors are syntactic in nature, and have nothing to do with harm. Writing let a: Int = "thirty three" isn't harmful, it's just wrong.

18

Hi all,

I'm glad for this review. I prefer the attribute to fully-qualified names, because it traces directly to the issue (and could be used later to identify causes). I believe the explicitness helps precisely because problems would present is such an occult manner.

But right now retroactive is a niche term of art, from the perspective of a new or even experienced Swift programmer. In the Swift language guide, it's only used wrt extensions. In this forum, it might be well understood, but even here it's ambiguous, as people distinguish e.g., which retroactive conformances are bad. And the proposed warning says nothing about "retroactive".

So retroactive a reasonable attribute name if the documentation explains the situation (and the warning mentions the attribute), though as others have mentioned some retroactive conformances are fine.

It'd be great to have a name that indicates the underlying issue to a naive reader, but I can't offer a great alternative. @duellingForPublicGlobal? @knownUniqueConformance? @retroactivelyUnique?

19

I agree that this doesn't fit the usual characteristics that would justify making it an error. However, I do think that many developers are going to want to treat this particular diagnostic as an error and ban the practice in their code. For better or worse, for many projects warnings do not carry much weight and may go unnoticed for a long time (or forever). Not having the option to ban implicit retroactive conformances seems like a miss to me, and for that reason I'm in favor of this being an error in Swift 6. Developers who want to ban it now could enable and upcoming feature for this behavior. Given that the release valve of applying the attribute exists, the cost of it being an error seems low to me outside of concerns about additional source breaks some projects may encounter when updating language modes.

Also, the more we can find ways to suppress the diagnostic for the safe cases, I think the more justified making it an error becomes. For example, I think the idea suggested above that conformances in the same package as the type or protocol should be exempt seems like a great compromise that would eliminate one significant source of annoyance.

I think this is good feedback. Although I'm now used to referring to these conformances as "retroactive" it has never felt to me like an intuitive term for the concept. There might be a name that more directly describes what it is we are trying to assert about the conformance. It's tough to articulate what that assertion is concisely, though. I don't think the word "unique" does the job because conformances defined in the same module as the conforming type are also meant to be unique; the difference is that the compiler is more confident in that scenario that it can prove that conflicting conformances won't be introduced. Perhaps this goes too far, but to me these seem like "unsafe" conformances because they cannot be proven to be unique at compile time.

20

I'm sorry, I mistakenly thought the proposal was only about resilient types. I had paged it out, saw Nevin's initial reply, and loaded an incorrectly cached older version of the idea. Yes, I agree that this is useful regarding types or protocols in other packages even if those packages are bundled with the executable (statically-linked, copied into the app bundle, whatever), because the upstream package can always add the same conformance.

I also agree that exempting modules within the same package would be reasonable, but I feel like there are a whole host of source-evolution-related things that would benefit from taking "same package" into account, and this one isn't necessarily special enough to have it diverge.