Error when one associated type is constrained to another. #10053
Conversation
|
The actual meat of this patch was written by Slava. I just cleaned it up a little and added tests. I'm not skilled enough in the generic signature builder or the iterative decl checker to know the best way to fix things up here without spending a good deal more time. |
|
@swift-ci Please smoke test |
|
Also, @JohnEstropia, I thought you'd like to know that this wasn't doing what you wanted it to in Swift 3.1. A typealias would probably work if you don't need compatibility with Swift 3.0. (We discovered this problem thanks to CoreStore's presence in the source compat suite.) |
|
LGTM! As for the Swift 3 compatibility story, I don't have any strong opinions. Leaving it as an error might be best, because clearly we're not doing (or able to do) what the user meant here. |
(...is constrained to be a subtype of another) Previously the compiler would just mark the entry in the inheritance clause invalid and move on without emitting any errors; in certain circumstances in no-asserts builds this could actually lead to everything working "correctly" if all conforming types happened to pick the same concrete type for both associated types. In Swift 4 this can actually be enforced with a same-type requirement, which will guarantee that the two associated types are the same even in generic contexts. This fix avoids assertions and crashes, but the diagnostic is still incorrect, and in the simple case of the inheritance clause it's redundant. Doing something better and possibly even downgrading it to a warning in Swift 3 mode is tracked by rdar://problem/32409449. Initial patch by Slava, fixed up by me.
jrose-apple
force-pushed
the
assock-assosh
branch
from
8588dcb
to
e4668cc
Compare
|
Even if we don't do anything for Swift 3 it would be nice to clean up the error so it actually said the right thing. :-) @swift-ci Please smoke test |
@jrose-apple I'll refactor this part of the library as I feel it's fighting the type system anyway. Thanks for the heads-up! |
(...is constrained to be a subtype of another)
Previously the compiler would just mark the entry in the inheritance clause invalid and move on without emitting any errors; in certain circumstances in no-asserts builds this could actually lead to everything working "correctly" if all conforming types happened to pick the same concrete type for both associated types. In Swift 4 this can actually be enforced with a same-type requirement, which will guarantee that the two associated types are the same even in generic contexts.
This fix avoids assertions and crashes, but the diagnostic is still incorrect, and in the simple case of the inheritance clause it's redundant. Doing something better and possibly even downgrading it to a warning in Swift 3 mode is tracked by SR-4981 / rdar://problem/32409449.