-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Stdlib] Use associated type where clauses to address various ABI FIXMEs. #8593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
af200e9
to
de94b3a
Compare
@dabrahams , @huonw , and @gribozavr might be interested in this. It's WIP right now because it regresses a compiler crasher |
It's unfortunate that associatedtype _Element pops up in completion results now - but I see it has a FIXME saying it's a hack that should be removed. Any chance the problem is already fixed, or we could fix it pretty easily? |
The problem is "the library needs recursive constraints" |
@DougGregor Are you sure? IIRC, |
Oh! @dabrahams has the right answer... that's nice big cleanup to have
within |
Just gotta stop that one crasher from looping ;) |
Very W00t and Wunderful to see all that red in the diff! Do let me know the moment we can start using it in the unicode-rethink branch! |
I think the crasher Doug is struggling with is fairly invalid code that, even without this PR, currently still crashes, so, you should be able to use (/find bugs in) these where clauses now. |
Yeah, my struggles are entirely with recursively-constructed types now, not with the rest of the feature. That said, minor perturbations of the Sequence/Collection protocols trip over the recursively-constructed types, so when you see the compiler spinning in |
When computing the connected components of within an equivalence class (based on derived same-type requirements), we might need derived-via-concrete constraints to maintain connectedness of the larger graph. Therefore, delay their removal until after we’ve computed connected components.
…requirement sources. When we’re determining what type should be used to key a ProtocolRequirement based on the source’s potential archetype and the target’s potential archetype, use potential archetype identity (==) rather than equivalence-class membership to cover the basis case. This ensures that interesting identity relationships in the enclosing context (e.g., the current GenericSignatureBuilder) don’t cause us to compute incorrect stored types.
…swiftlang#90, swiftlang#91. Address ABI FIXME swiftlang#68 by using same-type constraints directly on an associated type to describe the requirements on the Indices associated type of the Collection protocol. ABI FIXMEs swiftlang#89, swiftlang#90, swiftlang#91 are all in StdlibUnittest, and provoke warnings once swiftlang#68 is fixed, but it's nice to clear them out. Fixes SR-2121.
…onsequences of other FIXMEs. There isn't any work related to SE-0142 associated type where clauses for these particular ABI FIXMEs. Rather, we get all of the constraints we need from Collection and the recursive constraint we cannot yet express, all of which are covered by other ABI FIXMEs.
de94b3a
to
29a15ce
Compare
The infinite recursion is here: https://github.com/apple/swift/blob/master/lib/AST/GenericSignatureBuilder.cpp#L2796 |
This stops after 5 recurrences of the same associated type. It is a gross hack and a terrible idea, here as a placeholder to prevent us from running off the rails in ill-formed code. This will go away when we get further along the path with recursive protocol constraints.
@swift-ci please smoke test and merge |
1 similar comment
@swift-ci please smoke test and merge |
@@ -646,6 +646,7 @@ public protocol Collection : _Indexable, Sequence { | |||
/// supplying `IndexingIterator` as its associated `Iterator` | |||
/// type. | |||
associatedtype Iterator = IndexingIterator<Self> | |||
where Self.Iterator.Element == _Element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we rename _Element to Element, and write Element in place of Iterator.Element everywhere? :-)
@dabrahams suggested that above... I'm going to take a shot at it |
Start using associated type where clauses in the standard library to model same-type constraints that were inexpressible before SE-0142. Clean up the warnings that result from these constraints being in the protocols where they belong.
Resolves SR-2121 and standard library ABI FIXMEs 68, 89, 90, 91, 92, 94, 96, and most of 99. Some of these are merely warnings in the unit testing library, but a few (68, 92, 99) are part of the core standard library protocols.