Fix #2142: Skolemize arguments of dependent methods if necessary #2215
Conversation
These were not printed before, fell back to toString method.
Strictly speaking, all the info about a skolem type is printed, e.g.
A(?2)
But it's reassuring to have an explanation line like
?2 is an unknown value of type A
This was missing before, led to errors not being detected.
We now consider a type also as stable if it refers to an ExprType whose result type is stable. The previous commit made pos/z1720.scala break, because it skolemized unstable argument types. This commit makes the test pass again.
| /** Substitute argument type `argType` for parameter `pref` in type `tp`, | ||
| * skolemizing the argument type if it is not stable and `pref` occurs in `tp`. | ||
| */ | ||
| def substArgForParam(tp: Type, argType: Type, pref: ParamRef)(implicit ctx: Context) = { |
There was a problem hiding this comment.
Minor suggestion: all the subst* methods have the from argument before the to argument, here it's the reverse order which can be a bit confusing. If this is changed it probably makes sense to rename the method to substParamWithArg too.
| def substArgForParam(tp: Type, argType: Type, pref: ParamRef)(implicit ctx: Context) = { | ||
| val tp1 = tp.substParam(pref, argType) | ||
| if ((tp1 eq tp) || argType.isStable) tp1 | ||
| else tp.substParam(pref, SkolemType(argType.widen)) |
There was a problem hiding this comment.
I would have expected widenSingleton here but I don't know if that makes sense or not.
There was a problem hiding this comment.
For SkolemTypes, it's widen also elsewhere. We don't want to get ExprTypes in there.
| @tailrec final def isStable(implicit ctx: Context): Boolean = stripTypeVar match { | ||
| case tp: TermRef => tp.termSymbol.isStable && tp.prefix.isStable | ||
| final def isStable(implicit ctx: Context): Boolean = stripTypeVar match { | ||
| case tp: TermRef => tp.termSymbol.isStable && tp.prefix.isStable || tp.info.isStable |
There was a problem hiding this comment.
Was this change also needed to make a test pass? It seems more suspicious.
There was a problem hiding this comment.
Yes, pos/z1720 started failing without the fix. The commit comment indicates this.
| def assignType(tree: untpd.Apply, fn: Tree, args: List[Tree])(implicit ctx: Context) = { | ||
| val ownType = fn.tpe.widen match { | ||
| case fntpe: MethodType => | ||
| if (sameLength(fntpe.paramInfos, args) || ctx.phase.prev.relaxedTyping) fntpe.instantiate(args.tpes) |
There was a problem hiding this comment.
Would it make sense to change the definition of instantiate to do the skolemization?
There was a problem hiding this comment.
No, I think that's too low-level. By analogy, asSeenFrom does not do skolemization, but reports back that skolemization is needed, and then Typer does it.
Change name and align order of parameters.
odersky
changed the title
This PR detects errors around unstable argument types to dependent methods that were
not reported before.