[Constraint System] Fix covariant erasure for constrained existentials

[angela-laar]

Constrained existentials should be type erased to an upper bound that is dependent on other type parameters.

let t: (any Sequence<Int>) = [1, 2]

for value in t {
    let v = value + 1 // the existential is opened so that `value` is generic parameter Int instead of Any
}

Fixes rdar://101343646

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test macOS

[angela-laar]

@swift-ci please smoke test

[AnthonyLatsis]

What if:

protocol P<A> {
  associatedtype A
}
protocol Q {
  associatedtype B
  associatedtype C: P<B>

  func foo() -> C
}

func test(q: any Q) {
  let _ = q.foo() // Currently erased to 'any P', but would be 'any P<B>'?
}

The erased type should not be dependent on the opened existential. Instead, I think we should keep using getNonDependentUpperBounds, but refactor it to type-erase the result of calling getDependentUpperBounds.

[angela-laar]

Can you explain a bit more what you mean by type-erase the result of getDependentUpperBounds?

[angela-laar]

Oh wait, with and without this change, that example will still produce this error: Inferred result type 'any P' requires explicit coercion due to loss of generic requirements. Looks like the example will still be erased to 'any P' so we might be ok here.

[angela-laar]

This is a good example, I can add a test for it 🙂

[AnthonyLatsis]

looks like the example will still be erased to 'any P'

Interesting, not sure if that’s expected or not. Either way we shouldn’t be assuming that getDependentUpperBounds will always produce a non-dependent result; it will break stuff one way or the other:

class C1 {}
class C2<T>: C1 {}

protocol P {
  associatedtype A
  associatedtype B: C2<A>

  func foo() -> B
}

func test(p: any P) {
  let _ = p.foo()
}

Can you explain a bit more what you mean by type-erase the result of getDependentUpperBounds?

What I mean is to reimplement getNonDependentUpperBounds in terms of getDependentUpperBounds.

This is a good example, I can add a test for it 🙂

Nvm, turns out this particular case was covered in #42559 🙃

[slavapestov]

Another thing I noticed is that typeEraseExistentialSelfReferences() is basically a copy-and-paste of typeEraseOpenedArchetypesWithRoot() from Type.cpp. Perhaps if we update both copies or merge them together it will fix this problem.

[xedin]

I think a more principled approach would be to look into why the function is being passed any IteratorProtocol rather than any IteratorProtocol<Int> as the base type.

@AnthonyLatsis If I understand your comment correctly by "the function" you mean typeEraseExistentialSelfReferences and considering next() reference.

The problem is with makeIterator reference, that's why next() doesn't get any IteratorProtocol<Int> for the base. I simplified the example down to:

func increment(n : any Sequence<Float>) {
 var iter = n.makeIterator()
}

(the solver transforms for-in loop into var $gen = <expr>.makeIterator(); while let v = $gen.next() { ... } so $gen is n in this example).

typeEraseExistentialSelfReferences gets any Sequence<Float> as a base type and refTy is () -> τ_0_0.Iterator, generic signature is <τ_0_0 where τ_0_0 : Sequence, τ_0_0.Element == Float>.

Since the transform runs on refTy it would try to call getConcreteType on τ_0_0.Iterator and that does not produce a concrete type so it falls through to erasure case and produces any IteratorProtocol.

I think we should keep using getNonDependentUpperBounds, but refactor it to type-erase the result of calling getDependentUpperBounds.

Let me see if I understand your point here - getNonDependentUpperBounds should be allowed to use use type information provided by primary associated types, but only them, to maintain the contract that visible type retains only type information as available before the reference otherwise we might end up in the situation that leaks type information from opened type to the outside. In practice it means that getNonDependentUpperBounds needs to keep erasing until it reaches bound that does not have any unbound dependent members/generic parameters (based on the existential signature), right?

For your second example this means that getNonDependentUpperBounds would produce C1 (because C2 references undetermined dependent member A) but if P did have a primary associated type A and it was bound in expression i.e. any P<Int> getNonDependentUpperBounds would produce C2<Int> instead, does that make sense?

[AnthonyLatsis]

Sorry it took so long, somehow I only noticed your reply this evening.

Let me see if I understand your point here - getNonDependentUpperBounds should be allowed to use use type information provided by primary associated types, but only them, to maintain the contract

getNonDependentUpperBounds is allowed to use all the type info provided by the receiver (the generic signature derived from the existential base type). All generic constraints contribute to the result, e.g.

class C<T> {}
protocol P { 
  associatedtype A 
  associatedtype B: C<A>
}
protocol Q: P where A == Int {
  func getB() -> B
}

do {
  let q: any Q
  let _ = q.getB() // OK, 'C<Int>'
}

getNonDependentUpperBounds produces a constraint type that is supposed to be the non-dependent supremum of the dependent upper bounds, which retain all the type info. In practice and by contract, it’s always a non-dependent type, but not always as specific as it could have been. For example, the method wasn’t updated for constrained existentials, so it doesn’t know how to build parameterized protocol types. This means it never produces a type containing a parameterized protocol type; just bare protocols.

In practice it means that getNonDependentUpperBounds needs to keep erasing until it reaches bound that does not have any unbound dependent members/generic parameters (based on the existential signature), right?

For your second example this means that getNonDependentUpperBounds would produce C1 (because C2 references undetermined dependent member A) but if P did have a primary associated type A and it was bound in expression i.e. any P<Int> getNonDependentUpperBounds would produce C2<Int> instead, does that make sense?

Yes.

---

typeEraseExistentialSelfReferences gets any Sequence<Float> as a base type and refTy is () -> τ_0_0.Iterator, generic signature is <τ_0_0 where τ_0_0 : Sequence, τ_0_0.Element == Float>.

Since the transform runs on refTy it would try to call getConcreteType on τ_0_0.Iterator and that does not produce a concrete type so it falls through to erasure case and produces any IteratorProtocol.

I see, that makes a lot of sense. So after all this suggests we have to teach getNonDependentUpperBounds to build parameterized protocol types, and probably do so on demand via a flag because — I presume — upgrading upper bounds for more than just the built-in for-in transform would compromise the requires explicit coercion diagnostic.

The robust approach is to base getNonDependentUpperBounds on getDependentUpperBounds (bonus: the latter already builds parameterized protocol types).

[angela-laar]

ok! will do, converting to draft while I make those changes

[angela-laar]

I consider the fact that your example compiles today to be a bug.

I will track fixing this diagnostic issue in a second PR since after a couple of attempts, it seems significant changes to the Coercion Checker are needed

Opened a new PR for the original issue: #65785

[xedin]

Please add more tests before merging as we've discussed.

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test macOS

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

@swift-ci please smoke test

[angela-laar]

I consider the fact that your example compiles today to be a bug.

I will track fixing this diagnostic issue in a second PR since after a couple of attempts, it seems significant changes to the Coercion Checker are needed

Opened a new PR for the original issue: #65785