Fix #3333: adapt child instantiation #3475
Conversation
|
@AleksanderBG Do you want to review? This may be helpful to your WIP. |
| @@ -584,10 +584,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { | |||
| // precondition: `tp1` should have the shape `path.Child`, thus `ThisType` is always covariant | |||
| val thisTypeMap = new TypeMap { | |||
| def apply(t: Type): Type = t match { | |||
| case tp @ ThisType(tref) if !tref.symbol.isStaticOwner && !tref.symbol.is(Module) => | |||
| // TODO: stackoverflow here | |||
| // newTypeVar(TypeBounds.upper(mapOver(tp.underlying))) | |||
There was a problem hiding this comment.
Out of curiosity: what was the original issue here? Was tp present somewhere in tp.underlying? If yes, then wouldn't simply first creating the TVar, memoising it in a map keyed under tp and mapping over tp.underlying be enough? I'm asking, as basing on this code I wrote this and I'm not sure if it is correct.
There was a problem hiding this comment.
I forgot exactly what's the problem, but it's related to ClassInfo#selfType. tp.underlying is supposed to be decreasing (I assume), so I think the case you mention will not occur. For the task you want to deal with, the memoization in your code is correct.
| if (protoTp1 <:< tp2 && isFullyDefined(protoTp1, ForceDegree.noBottom)) protoTp1 | ||
| if (protoTp1 <:< tp2) { | ||
| isFullyDefined(protoTp1, force) | ||
| instUndetMap(protoTp1) |
There was a problem hiding this comment.
If I understand correctly how isFullyDefined behaves, then if it returns true, all TVars in protoTp1 should be already instantiated and instUndetMap will be a no-op. Would it make sense to only use instUndetMap if isFullyDefined returned false?
There was a problem hiding this comment.
Yes, good catch, I've updated the code, thanks a lot.
Fix #3333 #3455 #3469: adapt child instantiation
Structural types in #3333 is left open.
ThisTypefor modules.