[SR-12026] Swift 5.2 regression: requires the types 'Self.Error' and 'Never' be equivalent

[tonyarnold]

Previous ID	SR-12026
Radar	rdar://58466230
Original Reporter	@tonyarnold
Type	Bug

Environment

macOS 10.15.2

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 345088af56cb65145c075e6937a3ae29

Issue Description:

I'm seeing a regression when compiling a project that I use under the Swift 5.2 compiler included with Swift 5.2 Snapshot 2020-01-13 (a):

/Users/tonyarnold/Developer/Repositories/ReactiveKit/Sources/Connectable.swift:118:93: error: 'Self.LoadingValue' requires the types 'Self.Error' and 'Never' be equivalent
    public func replayValues(limit: Int = Int.max) -> ConnectableSignal<Signal<LoadingState<LoadingValue, LoadingError>, Error>> {
                                                                                            ^
/Users/tonyarnold/Developer/Repositories/ReactiveKit/Sources/LoadingSignal.swift:160:1: note: requirement specified as 'Self.Error' == 'Never' [with Self = Self]
extension SignalProtocol where Element: LoadingStateProtocol, Error == Never {
^
/Users/tonyarnold/Developer/Repositories/ReactiveKit/Sources/Connectable.swift:128:80: error: 'Self.LoadingValue' requires the types 'Self.Error' and 'Never' be equivalent
    public func shareReplayValues(limit: Int = Int.max) -> Signal<LoadingState<LoadingValue, LoadingError>, Error> {
                                                                               ^
/Users/tonyarnold/Developer/Repositories/ReactiveKit/Sources/LoadingSignal.swift:160:1: note: requirement specified as 'Self.Error' == 'Never' [with Self = Self]
extension SignalProtocol where Element: LoadingStateProtocol, Error == Never {
^

The project is ReactiveKit, and the latest tagged release (https://github.com/DeclarativeHub/ReactiveKit/releases/tag/v3.15.6) exhibits this behaviour.

The code compiles without issue under Xcode 11.3.1/Swift 5.1.3.

[slavapestov]

Reduced source break from Robert:

Reduced the source break

struct Observer<Element, Error> {}

protocol LoadingStateProtocol {

  associatedtype LoadingValue
  associatedtype LoadingError: Error
}

protocol SignalProtocol {
  associatedtype Element
  associatedtype Error: Swift.Error
}

extension SignalProtocol where Element: LoadingStateProtocol, Error == Never {

  typealias LoadingValue = Element.LoadingValue
  typealias LoadingError = Element.LoadingError

}

extension SignalProtocol where Element: LoadingStateProtocol {
  func replayValues(_ : Observer<LoadingValue, LoadingError>) {
    fatalError()
  }
}

Swift 5.1 was accepting the code but interpreting it incorrectly. The test case in the radar defines two protocols, followed by two protocol extensions of the second protocol.

Note that these type aliases were defined in the first protocol extension:

extension SignalProtocol where Element: LoadingStateProtocol, Error == Never {

  typealias LoadingValue = Element.LoadingValue
  typealias LoadingError = Element.LoadingError

}

However due to a bug in 5.1, effectively they behaved as if they were unconstrained, and the type checker always preferred them in name lookup. So the second protocol extension:

extension SignalProtocol where Element: LoadingStateProtocol {
  func replayValues(_ : Observer<LoadingValue, LoadingError>) {
    fatalError()
  }
}

Actually behaved as if it were written like this:

extension SignalProtocol where Element: LoadingStateProtocol {
  func replayValues(_ : Observer<Element.LoadingValue, Element.LoadingError>) {
    fatalError()
  }
}

That is, even though the second extension did not constrain 'Error == Never’, referencing LoadingValue and LoadingError from inside its body would find the typealiases defined in the first extension, and not the associated types defined in the protocol.

If you want to keep the 5.1 behavior going forward, you can change the second extension to replace ‘LoadingValue’ and ‘LoadingError’ with ‘Element.LoadingValue’ and ‘Element.LoadingError’, respectively. Alternatively, you can add “Error == Never” to the where clause of the second extension.

What changed is that 5.2 is now more careful about ensuring that requirements in ‘where’ clauses are correctly satisfied. I still need to figure out if its possible to accept this code (by having name lookup locate the associated type rather than the type alias with the same name). However that would still be a semantic change from the 5.1 behavior.

[beccadax]

@swift-ci create