[Sema] More aggressive consideration of conditionally defined types in expressions #17168
Conversation
lib/Sema/ConstraintSystem.cpp
Outdated
| if (parentTy && extension && extension->isConstrainedExtension()) { | ||
| auto subMap = NAT->getSubstitutionMap(); |
There was a problem hiding this comment.
I don't really understand what this substitution map is doing (why are conformances looked up in this one, but types looked up in the context one?), so I just guessed at the appropriate replacement. Thoughts?
There was a problem hiding this comment.
It's happening because extension has more context over the types it mentions like in the example I've added:
class A<T, U> {}
extension A where T == [U], U: P {
typealias S1 = Int
}
class B<U> : A<[U], U> {}
_ = B<C>.S1()Extension's substitution map would have relationship between T, U and conformance to P but the parent one wouldn't (it only knows about T and U), so we need to use substitution map from the extension to get conformances (without using lookupConformance) but type substitutions have to come from the parent because otherwise they are going to be replaced with types from the where clause of extension which we don't want because that's exactly what we are trying to check :)
There was a problem hiding this comment.
Ah, I see.
This "little" detail seems to be fairly important; this code doesn't handle the nested type being generic, since it ends up looking at the parent's parameters and not including any info about the child's:
struct X<T> {}
extension X where T == Int {
struct Y<U> {}
}
_ = X<Int>.Y<Float>()
_ = X<Float>.Y<Int>()This case is handled when the parameters are applied though, so I'm thinking that we just avoid it by only doing this work for non-generic types.
There was a problem hiding this comment.
I think original code made sense for typealias because the assumption was that we only needed to check if declaration context requirements held, it also seems in case of inner generic you don't really need to do anything special because open* would bind contextual types appropriately...
There was a problem hiding this comment.
it also seems in case of inner generic you don't really need to do anything special because open* would bind contextual types appropriately...
Yeah, that's the approach I've now taken and it seems to work.
|
@swift-ci please smoke test |
|
Hey @AnthonyLatsis, this does half the work on https://bugs.swift.org/browse/SR-5440 (I only noticed you had it assigned after doing this, since I was working on it from the angle of https://bugs.swift.org/browse/SR-7976; sorry!), but there's still checking of validity of these conditional bounds when these types are used in decls, e.g. |
|
@huon I was getting there. I didn’t really know where exactly to look at, but now I have a head start, thanks! I’ll be glad to deal with the sema part. |
…ypes.
One can have arbitrary types defined in a constrained extension, not just
typealiases, e.g.
struct Foo<T> {}
extension Foo where T == Int {
struct X {}
}
A mention of `Foo<String>.X` should be flagged, since String != Int. Previously
this worked (in expressions) only if `X` was a typealias, and with this patch
the above example is flagged too.
Part of https://bugs.swift.org/browse/SR-5440 and
https://bugs.swift.org/browse/SR-7976 and rdar://problem/41063870.
An expression like Type.Member will first do a normal lookup, and, if only
associated types could be found, will then do associated type inference on
them.
Previously the types of these would be returned directly, but this loses the
sugar associated with the implicit typealias that is created. The sugar is
necessary to ensure that things like the checking of conditional requirements is
satisfied, such as:
protocol P {
associatedtype T
}
struct X<T> {}
extension X: P where T == Int {}
let _ = X<String>.T.self
If the underlying type 'String' (or even 'Int') of 'X<...>.T' is returned, then
the above ends up being perfectly fine, because those are always available, but
that expression itself isn't quite right because the T typealias is conditional.
With this change, once associatedtype inference runs, the typealias that it
creates is found, and the type is mapped to have all the right sugar, meaning
the above is flagged. (This hints at an extra exciting bit of the original
behaviour: if the `let ...` line was copied so that there were two, the second
one gets the expected error, because it doesn't need to run inference and just
finds the typealiases created by the first line.)
Part of https://bugs.swift.org/browse/SR-5440 and
https://bugs.swift.org/browse/SR-7976 and rdar://problem/41063870.
huonw
force-pushed
the
validate-conditional-type-in-expr
branch
from
fea0e56 to
28460bc
Compare
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test macOS |
See the commit messages.
This should complete the expression-type-checking sides of https://bugs.swift.org/browse/SR-5440 and https://bugs.swift.org/browse/SR-7976 and rdar://problem/41063870. We don't validate conditionality for decls at all yet.