[SR-9753] REGRESSION: Ambiguity involving overloads and generics constrained by Error

[lilyball]

Previous ID	SR-9753
Radar	rdar://problem/47550780
Original Reporter	@lilyball
Type	Bug
Status	Closed
Resolution	Won't Do

Environment

Apple Swift version 5.0 (swiftlang-1001.0.45.7 clang-1001.0.37.7)
Target: x86_64-apple-darwin18.2.0
ABI version: 0.6

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug, 5.0Regression
Assignee	None
Priority	Medium

md5: d9e050beffdbe60baf098d7a78e86171

Issue Description:

I have a generic struct that declares a method, and an extension to this struct that overloads the method in the case where the generic parameter conforms to Error. This has worked fine so far, but the Swift 5 compiler rejects this as an ambiguity error. It seems to be special-casing Error somehow because this doesn't happen with other protocols. This compiler error happens even in Swift 4 mode (though I don't believe it should happen in Swift 5 mode either).

This error means Xcode 10.2 cannot compile https://github.com/lilyball/Tomorrowland.

struct Foo<E> {
    func bar(with: Foo<E>) {}
}
extension Foo where E: Error {
    func bar(with: Foo<Error>) {}
}
let foo = Foo<Error>()
foo.bar(with: Foo<Error>())

This yields

unnamed.swift:10:1: error: ambiguous use of 'bar(with:)'
foo.bar(with: Foo<Error>())
^
unnamed.swift:2:10: note: found this candidate
    func bar(with: Foo<E>) {}
         ^
unnamed.swift:6:10: note: found this candidate
    func bar(with: Foo<Error>) {}
         ^

[belkadan]

The new thing that happened is that Error-the-existential now conforms to Error-the-protocol. Hm.

[lilyball]

This sample code works if I change the overload to func bar(with: Foo<E>). Unfortunately I can't make that change in the main library.

I'm not actually sure why the normal overload resolution rules don't handle this case; the overload is more specific on the E parameter, it just doesn't use that parameter in the method itself (but of course it is used on the receiver).

It's entirely possible that the right solution here is to stop making Error-the-existential conform to Error-the-protocol in Swift 4 mode. Not having this code work in Swift 5 is kind of a shame, but it's something I can work around. I just don't want the existing code to break because Xcode was upgraded.

[DougGregor]

If we stop making Error-the-existential conform to Error-the-protocol in Swift < 5 mode, the new `Result` type will be mostly unusable outside of Swift 5.0 mode, which is a bit unfortunate. It'll force people to either not use `Result` in APIs, or push their clients to Swift 5 mode. I think that's probably worse overall than accepting the source-compatibility regression.

For reference, I found a small workaround:

struct Foo<E> {
    func bar(with: Foo<E>) {}
}

#if compiler(<5.0)
extension Foo where E: Error {
    func bar(with: Foo<Error>) {}
}
#endif

let foo = Foo<Error>()
foo.bar(with: Foo<Error>()) 

[lilyball]

Your workaround to "overloads in this scenario are ambiguous" is apparently "don't overload". The fix I'm actually planning on applying is just changing the name of the overload, because all the call-sites are statically known and it's a private function so I don't have API compatibility issues. But I still am concerned about existing clients not being able to upgrade their compiler without upgrading the library (especially if they're on an older version and would have to deal with breaking changes).

More generally, I'm also concerned about unexpected behavioral changes resulting from Error-the-existential conforming to Error-the-protocol, in cases where I've overloaded an API for E: Error (like the sample code, except this time the method actually uses E instead of Error so it's not ambiguous). I'm reasonably sure that after fixing this compiler error, Tomorrowland will still have cases where methods resolve to an overload that they didn't used to when using {Promise<_,Error>, though I don't know yet if there are any observable behavioral changes from this (the overloads should just cast the error object to Error but otherwise be the same as the original).

[lilyball]

Ok I checked, the other resolutions in Tomorrowland that are affected by this appear to be benign.

[DougGregor]

Okay, I think this is a case where we've made a source-incompatible change that we cannot meaningfully back out (due to the self-conformance of Swift.Error introduced in https://github.com/apple/swift-evolution/blob/master/proposals/0235-add-result.md.,)), and changing the overload-resolution rules here would be a significant undertaking with more fallout. It's not necessarily a bad idea, but should be done within a larger context.

Given those, I'm going to close this as "won't fix" with my apologies that you'll have to adapt your code.