Make sure arguments are evaluated in the correct typer state. #1460
Conversation
| def parent: Option[TyperState] = None | ||
|
|
||
| /** The closest ancestor of this typer state (including possible this typer state itself) | ||
| * which is not yet committed. |
There was a problem hiding this comment.
The documentation should state what is returned when this state and all of its ancestors are committed, or this method should return an Option
| @@ -546,7 +546,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic => | |||
| val fun1 = typedExpr(tree.fun, originalProto) | |||
|
|
|||
| // Warning: The following lines are dirty and fragile. We record that auto-tupling was demanded as | |||
| // a side effect in adapt. If it was, we assume the tupled proto-type in the rest of the application. | |||
| // a side effect in adapt. If it was, we assume the tupled proto-type in the rest of the application, | |||
| // until, possibly, we have to fall back to insert an implicit on thq qualifier. | |||
There was a problem hiding this comment.
So why does the code do tryWithImplicitOnQualifier(fun1, originalProto) and then fallbacks to tryWithImplicitOnQualifier(fun1, proto), isn't originalProto the non-tupled one?
There was a problem hiding this comment.
We have to try with both tupled and untupled. I'll add a comment.
isErroneous forced LazyRefs through the `existsPart` combinator. This might prompt further errors or infinite recursions, so should be avoided. Seen in the wild when trying to trace t1756.scala with -Ylog:front and typr printer on.
There's a tricky interaction with caching of typed arguments in FunProto types and backtracking using different typer states. We might end up with a typed argument that is evaluated in one typer state and that is used in another. The problem is that the argument typing might have inserted type variables (maybe by adding polymorphic implicit views) that are not registered in the typer state in which the application is finally typed. In that case we will see an "orphan poly parameter" in pickling. The fix is to discard argument types is their typerstate is not committed to the one in which the application is finally typed. To apply the fix we need to track - for typer states: whether or not it was committed, and what its parent is. - for function prototypes: the typer state in which an argument with cached type was evaluated. Test case is t1756.scala, which produced an "orphan poly parameter CI" before.
|
Rebased to master. The test failures are all OOM, btw. |
| new ApplyToTyped(tree, fun1, funRef, proto.typedArgs, pt) | ||
| else | ||
| new ApplyToUntyped(tree, fun1, funRef, proto, pt)(argCtx(tree)) | ||
| convertNewGenericArray(ConstFold(app.result)) |
There was a problem hiding this comment.
Some of this code is duplicated from typedApply and this line is especially non-obvious, it would be better to have simpleApply and typedApply call into a common method
There was a problem hiding this comment.
As far as I can see, the code was moved out of typedApply, not duplicated.
| /** The closest ancestor of this typer state (including possibly this typer state itself) | ||
| * which is not yet committed, or which does not have a parent. | ||
| */ | ||
| def uncommittedAncestor: TyperState = |
There was a problem hiding this comment.
proposal: instead of storing the direct parent, lazily-cache uncommittedAncestor and test-update it every time it's updated:
private var lastUncommittedAncestor = parent
def uncommittedAncestor: TyperState = {
if (lastUncommittedAncestor.isCommited && lastUncommittedAncestor.isDefined) {
lastUncommittedAncestor = lastUncommittedAncestor.lastUncommittedAncestor
}
lastUncommittedAncestor
}
There was a problem hiding this comment.
Good point. I'll roll that into the next PR.
There's a tricky interaction with caching of typed arguments in FunProto types
and backtracking using different typer states. We might end up with a typed
argument that is evaluated in one typer state and that is used in another. The
problem is that the argument typing might have inserted type variables (maybe
by adding polymorphic implicit views) that are not registered in the typer
state in which the application is finally typed. In that case we will see
an "orphan poly parameter" in pickling.
The fix is to discard argument types if their typerstate is not committed
to the one in which the application is finally typed. To apply the fix we
need to track
was evaluated.
Test case is t1756.scala, which produced an "orphan poly parameter CI" before.
Based on #1459. First new commit is "Don't force in isErroneous check".
Review by @smarter.