Skip to content

Don't leak sharpened bounds between implicit candidates#6308

Merged
retronym merged 2 commits intoscala:2.13.xfrom
milessabin:topic/t10708
Mar 5, 2018
Merged

Don't leak sharpened bounds between implicit candidates#6308
retronym merged 2 commits intoscala:2.13.xfrom
milessabin:topic/t10708

Conversation

@milessabin
Copy link
Contributor

@milessabin milessabin commented Feb 3, 2018

The bounds propagation introduced in #6140 caused a regression in scala/scala-java8-compat#97 because bounds sharpened while ranking implicit candidates leaked from candidates tested earlier to those tested later.

This commit fixes that by saving and restoring the infos of the symbols of the undetermined type parameters around the call to type check the implicit candidate which tests for it's applicability.

Initially it seemed like this ought to be a job for undoLog or Context#savingUndeteriminedTypeParams, but neither of those do the right thing here. UndoLog doesn't help because it affects the constraint on
the corresponding TypeVar rather than the info of the symbol; and savingUndeterminedTypeParams doesn't help because the relevant call to typedImplicit shares the enclosing ImplicitSearch#undetParams rather than looking at the context.

Fixes scala/bug#10708.

The bounds propagation introduced in scala#6140 caused a
regression in scala/scala-java8-compat#97 because bounds sharpened while
ranking implicit candidates leaked from candidates tested earlier to
those tested later.

This commit fixes that by saving and restoring the infos of the symbols
of the undetermined type parameters around the call to type check the
implicit candidate which tests for it's applicability.

Initially it seemed like this ought to be a job for undoLog or
Context#savingUndeteriminedTypeParams, but neither of those do the right
thing here. UndoLog doesn't help because it affects the constraint on
the corresponding TypeVar rather than the info of the symbol; and
savingUndeterminedTypeParams doesn't help because the relevant call to
typedImplicit shares the enclosing ImplicitSearch#undetParams rather
than looking at the context.

val savedInfos = undetParams.map(_.info)
val typedFirstPending = typedImplicit(firstPending, ptChecked = true, isLocalToCallsite)
foreach2(undetParams, savedInfos){ (up, si) => up.setInfo(si) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put this in a try/finally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@milessabin
Copy link
Contributor Author

/synch

@milessabin
Copy link
Contributor Author

/rebuild

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bounds sharpened during implicit search leak between candidates

2 participants