Handle requirement environments with concrete same-type constraints. #5857
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is possible to have requirement environments in which substitution
of the conforming type for Self into the requirement's signature would
result in substituted same-type requirements that no longer involve
type parameters. This triggered an assertion in the construction of
the requiremet environement; instead, just drop the requirement
because it is no longer interesting. Witness matching will simply fail
later one.
With this fix, it now because possible to add generic requirements to
a protocol that were unsatisfiable for certain models. For example,
the following protocol:
protocol P {
associatedtype A
associatedtype B
func f<T: P>(_: T) where T.A == Self.A, T.A == Self.B
}
can only be satisfied by conforming types for which Self.A ==
Self.B. SE-0142 will introduce a proper way to add such requirements
onto associated types. This commit makes any such attempt to add
requirements onto "Self" (or its associated types) ill-formed, so we
will reject the protocol P above with a diagnostic such as:
error: instance method requirement 'f' cannot add constraint
'Self.A == Self.B' on 'Self'
Fixes rdar://problem/29075927.
…tative. We had two egregious errors in our handling of superclass constraints: * We were recording superclass constraints on non-representative potential archetypes, which meant they would get ignored. * When two equivalence classes got merged, we wouldn't add the superclass constraint to the representative. Drive-by fix noticed while addressing rdar://problem/29075927.
|
@swift-ci please smoke test and merge |
jtbandes
reviewed
Nov 18, 2016
| associatedtype A | ||
|
|
||
| func f<T: P3>(_: T) where T.A == Self.A, T.A: C // expected-error{{instance method requirement 'f' cannot add constraint 'Self.A: C' on 'Self'}} | ||
| func g<T: P3>(_: T) where T.A: C, T.A == Self.A // expected-error{{instance method requirement 'g' cannot add constraint 'Self.A: C' on 'Self'}} |
There was a problem hiding this comment.
Hmm, I spy another bug in Swift.tmLanguage. Thanks for unintentionally finding it! 😄
There was a problem hiding this comment.
@jtbandes , do you have Swift 3 code that was making use of this without crashing the compiler?
There was a problem hiding this comment.
No, sorry, I'm just talking about syntax highlighting here.
There was a problem hiding this comment.
Hah! Turns out the compiler didn't crash on all examples I banned here, so I went ahead and restricted this change to Swift 4+
|
Oops... was this not crashing in a previous Swift? I might need to conditionalize on Swift 4 |
DougGregor
added a commit
to DougGregor/swift
that referenced
this pull request
Nov 18, 2016
… mode. PR swiftlang#5857 started rejecting generic requirements that adding constraints directly to 'Self', which means the requirements would be unsatisfiable by some models. At the time that commit was merged, we had thought the compiler crashed on all instances of this problem. It turns out that, with assertions disabled, these protocols would be accepted and could be used, so downgrade the error to a 'deprecated' warning in Swift 3 compatibility mode.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is possible to have requirement environments in which substitution
of the conforming type for Self into the requirement's signature would
result in substituted same-type requirements that no longer involve
type parameters. This triggered an assertion in the construction of
the requiremet environement; instead, just drop the requirement
because it is no longer interesting. Witness matching will simply fail
later one.
With this fix, it now because possible to add generic requirements to
a protocol that were unsatisfiable for certain models. For example,
the following protocol:
can only be satisfied by conforming types for which Self.A ==
Self.B. SE-0142 will introduce a proper way to add such requirements
onto associated types. This commit makes any such attempt to add
requirements onto "Self" (or its associated types) ill-formed, so we
will reject the protocol P above with a diagnostic such as:
While we're here, fix a longstanding issue with superclass constraints getting lost in the
ArchetypeBuilder.Fixes rdar://problem/29075927.