[SILGen] Fall back to module lookup on invalid conformance when building subst. maps for vtable thunks

[AnthonyLatsis]

class Base<T> {
  func foo<U>(arg: U) where T == Int
}
class Derived<T>: Base<T> {
  func foo(<U>(arg: U) where T: Protocol
}

Here, as usual, we combine the override substitutions T := Int with the invocation generic signature <T where T: Protocol> to form a new substitution map for the reabstraction thunk, only to realize that we cannot recover the Int : Protocol conformance from just the original substitution map. As a consequence, we end up with an invalid conformance, and a crash later on once we actually emit the thunk. As ugly as it is, falling back to module lookup in SubstitutionMap::get fixes the issue.

### FIX 2

Now that we allow purely contextual where clauses, we can face non-generic methods
with generic signatures that constrain all outer params to concrete types. E.g.

class Base<T> {
  func foo() where T == Int
}
class Derived<T>: Base<T> {
  func foo() where T: Protocol
}

Remove the assertion guarding against these generic signatures on SILFunctionType
so that we always retain the override substitutions needed to transform the derived
method's formal type when we emit a reabstraction thunk.

[AnthonyLatsis]

cc @slavapestov, @jckarter Could you take a look at this please?

[slavapestov]

Can you try to come up with fixes that don't break these existing invariants? If you want to pursue the second fix further, you should finish off my old PR that removes the assertion: #23291

The hard part was that we lose performance because existing optimizations are currently disabled for generics (such as inlining) but we still want them to take place in the fully-concrete case. So some optimization passes will need changes to handle this.

[AnthonyLatsis]

I see, so we do lose performance.. I take it whatever eventually causes the performance regression does not check if the generic signature is at all useful.

Can you try to come up with fixes that don't break these existing invariants?

I did try something, but hit another one saying ≈ non-generic SIL function type not allowed to have a generic environment. I am not well versed in this yet, so I would experiment a bit more in that direction if you think it is worth exploring as an alternative to your old PR.

If you want to pursue the second fix further, you should finish off my old PR that removes the assertion: #23291

The hard part was that we lose performance because existing optimizations are currently disabled for
generics (such as inlining) but we still want them to take place in the fully-concrete case. So some
optimization passes will need changes to handle this.

Optimization passes are interesting, but I'm hesitant about the idea of making the optimizer responsible for collecting unused generic signatures (unused because the function type is already fully substituted through canonicalization) just because we need that signature in another place within the SILGen library. Does this make sense to you?

[AnthonyLatsis]

I am going to rip out the second fix so that we can proceed with the first one. If it looks sane, that is. I have no idea how often the module lookup fallback is going to be hit in the wild though, and this does fix an edge case, so maybe we should test compiler performance?

[AnthonyLatsis]

FWIW, I refactored the first part into a (hopefully temporary) targeted fix to make sure we don't end up getting performance regressions elsewhere.

[jckarter]

It would be more correct if we could capture the conformance for Int: Protocol we saw at the point we establish the override. Falling back to conformance lookup in the module is a bug, since it's an opportunity to potentially end up with a different answer from the first time we established the conformance.

[AnthonyLatsis]

Falling back to conformance lookup in the module is a bug, since it's an opportunity to potentially end up with a different answer from the first time we established the conformance.

Do you have an example of such a scenario? I'm curious.

[jckarter]

If Protocol is defined in a module that doesn't itself conform Int to Protocol, then two different modules could each declare their own retroactive conformance of Int: Protocol.

[slavapestov]

Yeah, I think @jckarter's answer is right. We need to record 'override substitutions' when we check the override, just like we record substitutions for protocol witnesses.

[shahmishal]

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

• How to change the base branch: (Link)
• More detail about the branch update: (Link)