[Sema] Fix compiler error when extending a typealias of a partially specialized generic type #73169
Conversation
|
@swift-ci please smoke test Linux |
|
i was curious to see how this was solved and guess I was overcomplicating the issue 😭 |
|
This is far from solved, FWIW 🙂. I only triggered the tests to encourage the author to iterate on failures before any feedback arrives. |
|
Is it possible to make this PR as a draft? |
|
I was converting to draft but Anthony was faster |
|
For posterity, the convert to draft button is at the bottom of the Reviewers section. |
|
deleted and pushed branch in order to trigger CI |
|
@AnthonyLatsis check this out |
|
merged main again, smoke test Linux was green already |
|
@AnthonyLatsis I see you have been quite busy with 6.1 these days. Could you review this? |
|
@swift-ci please smoke test macOS |
|
Will have a review ready by Monday. |
…red specialized nested types
|
@AnthonyLatsis |
|
@AnthonyLatsis reminder |
|
You once said a ping every two weeks is fine. So here I am pinging you @AnthonyLatsis |
There was a problem hiding this comment.
Thank you for the reminders and the patience. Another somewhat random test case I think worth having:
struct S1<T> {
struct Inner<U> {}
}
struct S2<T> {
typealias A1<U> = S1<T>.Inner<U>
typealias A2<U> = S1<T>.Inner<U> where T == Int
}
extension S2<Int>.A1 {
func foo1() {
let int: Int
let _: T = int // OK
}
}
extension S2.A2 {
func foo2() {
let int: Int
let _: T = int // OK
}
}| } | ||
|
|
||
| auto nominalGenericArguments = nominal->getDeclaredInterfaceType() | ||
| .getPointer() |
There was a problem hiding this comment.
.getPointer() is superfluous.
| ->getGenericArgs(); | ||
| auto typealiasGenericArguments = typealias->getUnderlyingType() | ||
| .getPointer() | ||
| ->getAs<BoundGenericType>() |
There was a problem hiding this comment.
.getPointer() is superfluous.
| if (nominalBoundGenericType.getPointer()->isEqual( | ||
| typealiasBoundGenericType.getPointer())) { |
There was a problem hiding this comment.
.getPointer() is superfluous.
| if (!typealiasBoundGenericType->hasTypeParameter()) { | ||
| isInferredType = true; | ||
| } else { | ||
| isInferredType = false; | ||
| break; | ||
| } |
There was a problem hiding this comment.
If we remove isInferredType, this can be
| if (!typealiasBoundGenericType->hasTypeParameter()) { | |
| isInferredType = true; | |
| } else { | |
| isInferredType = false; | |
| break; | |
| } | |
| if (typealiasBoundGenericType->hasTypeParameter()) { | |
| return false; | |
| } |
| if (!std::equal(nominalGenericParams.begin(), nominalGenericParams.end(), | ||
| typealiasGenericParams.begin(), | ||
| [](GenericTypeParamType *gp1, GenericTypeParamType *gp2) { | ||
| return gp1->isEqual(gp2); | ||
| })) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
We need to perform this check regardless of whether the original number of generic params matched.
| extension A2 { | ||
| func adding(_ x: Int) -> Self { | ||
| S2(x: self.x + x, y: y, z: z) // expected-error {{binary operator '+' cannot be applied to operands of type 'X' and 'Int'}} expected-note {{overloads for '+' exist with these partially matching parameter lists: (Int, Int)}} | ||
| } | ||
| } |
There was a problem hiding this comment.
You can simplify these specialization checks and minimize diagnostic noise like so:
| extension A2 { | |
| func adding(_ x: Int) -> Self { | |
| S2(x: self.x + x, y: y, z: z) // expected-error {{binary operator '+' cannot be applied to operands of type 'X' and 'Int'}} expected-note {{overloads for '+' exist with these partially matching parameter lists: (Int, Int)}} | |
| } | |
| } | |
| extension A2 { | |
| func test() { | |
| let int: Int | |
| let _: X = int // Is X == Int in this extension? | |
| } | |
| } |
Then you also remove the properties.
| for (const auto &type : nominalGenericParams) { | ||
| if (type->getDepth() == maxDepth) { | ||
| nominalGenericParams = nominalGenericParams.drop_back(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I like how this appears to be unsafe but is actually safe because we are trimming a non-owning view into the array. But this is probably better:
| for (const auto &type : nominalGenericParams) { | |
| if (type->getDepth() == maxDepth) { | |
| nominalGenericParams = nominalGenericParams.drop_back(); | |
| } | |
| } | |
| while (!nominalGenericParams.empty() && | |
| nominalGenericParams.back().getDepth() == maxDepth) { | |
| nominalGenericParams = nominalGenericParams.drop_back(); | |
| } |
Test case for why we need !nominalGenericParams.empty():
struct S1<T> {
struct Inner<U> {}
}
struct S2 {
typealias A<T> = S1<T>.Inner<Int>
}
extension S2.A {
func test() {
let int: Int
let _: U = int // error
}
}| struct S4<A, B> { | ||
| // Generic parameters: <A, B, C> | ||
| // Depth: 0 0 1 | ||
| struct Nested<C> { | ||
| let c: C | ||
| } | ||
| } | ||
|
|
||
| struct S5<A> { | ||
| // Generic parameters: <A, B, C> | ||
| // Depth: 0 1 1 | ||
| typealias Alias<B, C> = S4<A, B>.Nested<C> | ||
|
|
||
| } | ||
|
|
||
| extension S5.Alias{ | ||
| func adding(_ c: Int) -> Self { | ||
| S4.Nested(c: self.c + c) //expected-error {{binary operator '+' cannot be applied to operands of type 'C' and 'Int'}} expected-note {{overloads for '+' exist with these partially matching parameter lists: (Int, Int)}} | ||
| } | ||
| } |
There was a problem hiding this comment.
We need a different way to verify that Alias is not passthrough in this case. The error you are verifying is legit even if Alias was passthrough, since nothing is specialized. One way would be
struct S5<A> {
// Generic parameters: <A, B, C>
// Depth: 0 1 1
typealias Alias<B, C> = S4<A, B>.Nested<C> where A == Int
}And check that A != Int in the extension. The extension will not pick up the constraint if Alias is not passthrough:
swift/lib/Sema/TypeCheckGeneric.cpp
Lines 629 to 635 in d5ef256
Resolves #68212.