Signatures cached in NamedType can become stale, some cache invalidation is needed

[smarter]

NamedType extends SignatureCaching which defines: https://github.com/lampepfl/dotty/blob/f2e5fd08036b2ecbcbc536d49517c8d66ecfa7a5/compiler/src/dotty/tools/dotc/core/Types.scala#L3053-L3059

This code assumes that if a signature is not underdefined, it is valid for the whole run. For a NamedType, computeSignature delegates to Denotation#signature, if the denotation info is a MethodicType, this in turn is delegated to MethodicType#signature. So this scheme only works if all denotation transformers that transform MethodicType do so while preserving signature.

Unfortunately this is not the case: for example if we look at TypeErasure#transformInfo, we see that a MethodType can be replaced with a different type which will get a different signature:
https://github.com/lampepfl/dotty/blob/f2e5fd08036b2ecbcbc536d49517c8d66ecfa7a5/compiler/src/dotty/tools/dotc/core/TypeErasure.scala#L202-L216

I've written a unit test that demonstrates the problem: #8798

I don't think we can tweak Denotation#signature to always be stable. For example we can't just run TypeErasure#transformInfo at the current phase and expect it to work properly: it will return different results depending on whether the ExplicitOuter tree transformer has been run or not (see bf3b224).

Therefore, I only see two possible ways forward:

• Give up on signature caching in NamedType
• Keep signature caching, but only for MethodType we're sure will have their signature preserved by all transformers (it may be hard to figure out exaclty which one we can safely cache, TypeErasure#transformInfo is probably not the sole source of issues)

What do you think, @odersky ?

[smarter]

Therefore, I only see two possible ways forward:

Oh yeah, cache invalidation is also a possible solution as mentioned in the title: we could keep the cache valid only for a given phase id (or a set of phase ids, using a similar mechanism to the ones we uses for changesMember/changesParents/changesBaseTypes)

[odersky]

The signature design assumes that signatures are stable over all phases, and that the signature of a type T is the same as signature(erasure(T)). Otherwise signatures would not make sense. So any code in TypeErasure that violates this principle is broken.

[EDIT] But I see now the problem is signatures of denotations. And yes, we cannot assume they are stable because of ExplicitOuter. Hmm not sure what to do.

[smarter]

I guess if ExplicitOuter is the only thing we cannot handle, then we could simply forbid caching signatures of constructors pre-ExplicitOuter ?(It's still going to be quite a lot of work to fix all the other issues)

[odersky]

The purpose of a signature of a NamedType is to provide a stable reference to a member. So maybe we could always take the signature of a NamedType pre-ExplicitOuter, then we do not need to update it later.

[smarter]

So maybe we could always take the signature of a NamedType pre-ExplicitOuter

Ah, so if we go down that road, does signature generation still need to be based on erasure via sigName? Maybe something simpler could be used, like calling .widen.typeSymbol.name on every parameter.

[odersky]

sigName already bypasses erasure in most simple cases, so I believe it would come down to the same thing.