[Amendment] SE-0364: Allow same-package conformances

scanon · 2024-05-15T20:08:35.469Z

Hello all --

The review for an amendment to SE-0364: Warning for Retroactive Conformances of External Types begins now and runs through May 22nd, 2024.

This amendment adds the following exception to the list of cases in which the warning is suppressed:

If it is declared in a module that is part of the same package as the conformance,
this is not considered retroactive.

As review manager, I would like to clarify that such a conformance is retroactive; what the authors are proposing is that it would not be produce a warning.

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 emailing the review manager directly, please include "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

AlexisQapa · 2024-05-15T20:46:25.491Z

Could one provides an example where this would be needed ? I guess this could be useful for large repos worked by différents teams but I find it odd, isn’t it bad practice ?

taylorswift · 2024-05-15T20:58:22.290Z

AlexisQapa:

Could one provides an example where this would be needed ? I guess this could be useful for large repos worked by différents teams but I find it odd, isn’t it bad practice ?

it is very useful (and for me, common) to put protocol witnesses in an overlay module, to help keep binary size down and improve compilation speed. otherwise, the witnesses have to go either in the module that defines the type, or the module that defines the protocol, and would introduce unwanted dependencies between the two.

i have reservations about making SwiftPM packages an organizational unit for anything, given that SwiftPM only supports one package per repository, which means in practice a “package” (which may contain hundreds of modules) is far too large of a unit. but i already talked about that during the review of the package access level, and it really doesn’t have much to do with how the compiler supports package, rather it is more an issue with how SwiftPM defines “packages” in its implementation.

tshortli · 2024-05-15T21:17:07.264Z

I'm strongly in favor of this amendment after having some experience building real projects with toolchains that have the diagnostics for SE-0364. Many packages contain a constellation of modules that have different layering requirements. For example a package might contain a low level module that defines some data models and a high level module that contains visualizations for those models:

// MyModels module
public struct Model {
  // ...
}

// MyWidgets module
public import WidgetUI // external dependency

public struct ModelWidget: WidgetUI.Widget {
  // ...
}

Imagine the owners of this package want to add a conformance on a protocol from the external, high-level dependency WidgetUI to the type Model:

extension Model: WidgetUI.Drawable {
  // ...
}

In this package the logical home for this conformance is the MyWidgets module, rather than the MyModels module because MyWidgets is the one with an existing UI dependency. Today, that conformance would have to be marked @retroactive, even though the owners of Model are the ones declaring it. That's not in the spirit of SE-0364 in my opinion, because the owners of the type are exercising their right to declare a conformance; for legitimate reasons, it just happens to belong in a separate module that they also own.

We could just require package owners to use @retroactive in this circumstance. However, the benefit of relaxing the requirements for retroactive conformances in this way is that the remaining conformances that do require @retroactive are more likely to all be problematic. If you want to be able implement a policy like banning @retroactive conformances in your codebase, then this carveout becomes an important release valve for conformances that could not be reasonably defined any other way.

The main risk of this amendment is that it could encourage package owners to accidentally paint themselves into a corner by declaring a retroactive conformance in the wrong choice of module. I think this concern really only applies to ABI stable modules, where you wouldn't be able to reorganize the code later when you realize the mistake. This failure mode doesn't seem particularly likely to happen in practice because if you already own a type then the most obvious place to put conformances involving that type is in the same module. And if you're maintaining ABI, hopefully you're already applying a very high level of scrutiny to public declarations and ensuring that you're committed to supporting them indefinitely.

AdamCmiel · 2024-05-17T19:50:15.825Z

tshortli:

I'm strongly in favor of this amendment after having some experience building real projects with toolchains that have the diagnostics for SE-0364

I'll echo this sentiment. To prepare for Xcode16, I've been (trying to get) building projects with hundreds of modules and am looking into an automated solution to re-introducing the module names to avoid this problem. There is little concern for ABI stability as we don't use module resiliency, and nearly all of these conformances are on types and protocols that we own.

tshortli:

If you want to be able implement a policy like banning @retroactive conformances in your codebase, then this carveout becomes an important release valve for conformances that could not be reasonably defined any other way.

I'll further echo that this is a reasonable goal, to declare retroactive conformance for code outside of your domain, and I'd prefer the marker as an indication that we are doing something more dangerous rather than the module qualifier.

We've had to get into specialized imports to get around same-module-and-type name collisions, and at times had to introduce new source files where that wasn't possible with the given implementation. Because the @retroactive attribute is not backwards-compatible with Xcode 15.3, we're having to introduce these module qualifications to avoid code changes coinciding with the new Xcode and toolchain coming this summer and avoid disabling warnings-as-errors throughout the codebase.

dnadoba · 2024-05-22T13:07:54.080Z

Very much in favor of this amendment!

Most places I have seen warnings about retroactive conformance were perfectly fine and in fact inside the same package. Adding @retroactive has been mostly an annoyance in these cases. It also made it harder to spot real issues where a retroactive conformance was used for types that were not owned by the package.

This amendment will make the boilerplate go away and highlight real retroactive conformances issues that are outside of one's control.

Nobody1707 · 2024-05-22T22:11:07.412Z

This seems like a reasonable and useful amendment. I'm all for it.

scanon · 2024-06-10T18:52:48.140Z

Hi all: in response to a request from the LSG, the authors have updated the amendment. The new design is that same-package retroactive conformances will actually not be retroactive, instead of merely not generating a warning.

Note that conflicting same-package conformances might be detected at compile time, link time, or load time, depending on the specifics.

I am extending the review period from now until June 17.

grynspan · 2024-06-10T18:54:46.972Z

Makes sense to me. Ship it.

ksluder · 2024-06-10T19:12:46.587Z

Is there any actual difference between “not actually retroactive” and “not being warned about being retroactive”?

scanon · 2024-06-10T19:20:48.615Z

Yes!

There is a runtime bit that identifies a conformance as retroactive. I think that it's only used in one part of the runtime currently (@Douglas_Gregor could supply the details), but its use might expand in the future. Perhaps more importantly, it is possible to have overlapping retroactive conformances from separate modules, because the symbols mangle differently--this is not necessarily desirable, rather a statement of fact. It is an error to have overlapping non-retroactive conformances (for same-package-but-not-same-module conformances we will keep the mangling to preserve ABI, but add a canary symbol that lets us diagnose linking or loading two of them together).

cal · 2024-07-01T22:47:16.474Z

This would be a great improvement! At Airbnb, our app has a large number of modules, and we have hundreds of extensions that add protocol conformances where the type and protocol are defined in different internal modules. We also have warnings-as-errors enabled, so have hundreds of new warnings / errors to work through when upgrading to Xcode 16 / Swift 6.0. We were hoping that having all of our internal modules be part of the same "package" would avoid the need to do this. Very happy to see this being considered!

I like the idea of privileging code within the same package with extra functionality, since it's always distributed and compiled as a single unit.

scanon · 2024-07-02T01:06:19.381Z

This proposal has been accepted.

John_McCall · 2024-07-02T06:35:59.357Z