SE-0364: Warning for Retroactive Conformances of External Types

Hi all --

Harlan has updated the proposal and implementation to apply to all modules, not just resilient
libraries. It also updates the warning text to more clearly explain what the consequences are of such a construct.

As the review conversation to this point has been minimal and positive, I am not planning to extend the review period at this time; it will run through the 27th as before. But if this change generates significant unexpected discussion, we may extend the review period to accommodate that.

The actual changes to the proposal are available here.

8 Likes

I think this is quite a large change, and it is not clear to me that it is what we want.

The ability to provide retroactive conformances is something I greatly appreciate about Swift. Introducing noisy warnings for it would be detrimental.

I think a more narrowly-tailored approach is warranted. Perhaps a warning should only arise when the type and protocol are both declared in resilient modules, and either they are from the same module, or one of their modules “encapsulates” the other.

Or perhaps nothing should be done at all.

8 Likes

Even in the best (non-resilient) case, retroactive conformances are a potential source-compatibility pitfall, since there's no great way to resolve the conflict that arises if/when the library which provides the type/protocol decides to vend the conformance first-class (unless the conformance is so trivial that there's absolutely no concern about different implementations).

Since the proposal provides a relatively lightweight silencing mechanism, an unrestricted warning seems appropriate to me.

4 Likes

(speaking as myself, not as review manager)

I share these concerns to a certain extent, but:

  • While these conformances are "common", they are not pervasive. Most projects can keep them well isolated in source.
  • There's a trivial escape hatch to silence the warnings while clarifying intent.

Given this, I think the benefits outweigh the drawbacks.


(wearing my review manager hat)

It would be great to see examples of projects where you feel the burden imposed by these new warnings would be unacceptable.

4 Likes

In addition to that, the implementation includes a fix-it to automatically add these qualifications.

3 Likes

I almost don’t want the fix-it, so people are forced to go figure out whose types they’re messing with, but it’s probably better to have it.

4 Likes

I could be wrong but wouldn't that potentially result in a completely unresolvable warning in the following scenario?

// Let's say we know for sure that Bar is in fact "Sendable", but the library owner did not yet mark it as such.
final class Bar { }


// Now, inside my module...

struct Foo: Sendable {
    
    // Either this will warn, because Bar isn't marked Sendable.
    var bar: Bar?
}

// Or this will warn, because I don't own Bar.
extension Bar: Sendable { }

Maybe there should be a special carve-out here for Sendable, or marker protocols in general?

The proposal allows for a silencing mechanism by module qualifying the types in question so that you could write:

extension BarModule.Bar: Swift.Sendable { }

without warning.

3 Likes

The correct way to handle this is for the client to use @preconcurrency import TheModule, which will disable Sendability errors for types in that module.

6 Likes

That said, @Syre’s point about marker protocols (which seems extendable to Error and other non-marker protocols without required members) is well taken in that the stated harms in this proposal are inapplicable, and it may be worth subsetting this out. Likewise @unchecked conformances generally.

2 Likes

Error and Sendable are still not particularly valid because even though they have no members, it’s still invalid to duplicate their conformances.

2 Likes

What sort of invalidity occurs when a type conforms to Error twice?

extension Foundation.Date: Swift.Identifiable {
  // ...
}

Does this workaround solve the undefined behavior problem?
If not, it has the disadvantage of hiding the existence of problematic code.
I think attributes are better.

example:

@suppress(warn-unsupport-retroactive-conformance)
extension Date: Identifiable {
  // ...
}
2 Likes

While there may not be observed invalid behavior, it is still a misuse of the runtime protocol conformance facilities, and is therefore unsupported.

The workaround does not solve the undefined behavior problem. I would also like to see a warning suppression system, but it's outside of the scope of this proposal.

Right, sorry I thought you were proposing that even that escape hatch not be provided, so I wanted to highlight this example as a possible issue with that.

I think this might still be kind of dissatisfactory.

For example, let's say I only know for sure that one particular type in the outside module is "Sendable" but it wasn't marked as such. I still want the helpful Sendable warnings for the rest of the types in that module, so I think I would rather retroactively conform the one type I know to be Sendable than import the whole module as @preconcurrency.

That is a bug on the framework. If the type is truly Sendable, the owners of that framework need to mark it as such.

Rest assured, I have no plans to remove the escape hatch from this proposal, so for this specific case if you still need to retroactively conform this type (which is invalid once the framework does introduce the conformance), the escape hatch is still there.

1 Like

I am not requesting a rich suppression system with this proposal.
For now, a simple attribute like @unsupportRetroactiveConformance (or @unsafe-) would be also fine.
It is important that the unsafe code be explicit.

@harlanhaskins Without weighing in on the subject myself, it would be useful to write a paragraph or two exploring the tradeoff between:

  • this approach
  • at attribute that silences the warning
  • an attribute that silences warnings more generally

as that seems to be missing from the "alternatives considered" section. Assuming that the proposal is eventually accepted, it would be appropriate to append the rationale there as part of the acceptance.

3 Likes