Skip to content

Rework insertion of stabilizing definitions#8716

Merged
lrytz merged 2 commits intoscala:2.13.xfrom
lrytz:t11756
Apr 15, 2020
Merged

Rework insertion of stabilizing definitions#8716
lrytz merged 2 commits intoscala:2.13.xfrom
lrytz:t11756

Conversation

@lrytz
Copy link
Member

@lrytz lrytz commented Feb 14, 2020

Introduce a typer mode to know when we're type checking a qualifier
of a select, or a function part of an application. When a stabilizing
qualifier is created, insert it once we get back out of this mode.

In x.y.foo(bar), if foo has a second implicit argument list, make
sure that the block with the stabilizer is only created after the
implicit is inferred, i.e., run adapt on $stabilizer.foo(bar)
to get $stabilizer.foo(bar)(<implicit arg>) first, then create

{
  val $stabilizer = x.y
  $stabilizer.foo(bar)(<implicit arg>)
}

Before, we would create

{
  val $stabilizer = x.y
  $stabilizer.foo(bar)
}

then assign a MethodType to the block, and run adapt. This worked in
some situations by chance, but not always.

Fixes scala/bug#11756 and fixes scala/bug#11609 (tested also with shapeless)

@scala-jenkins scala-jenkins added this to the 2.13.3 milestone Feb 14, 2020
@lrytz
Copy link
Member Author

lrytz commented Feb 14, 2020

cc @som-snytt (alternative to PR #8208), @milessabin. @retronym does this come close to what you suggested in here scala/bug#11756 (comment)?

@lrytz lrytz requested a review from retronym February 14, 2020 15:34
@lrytz lrytz force-pushed the t11756 branch 6 times, most recently from df9b903 to 8cd15db Compare February 20, 2020 11:52
@lrytz
Copy link
Member Author

lrytz commented Feb 20, 2020

@retronym this is ready for a review. I'll try to also run it through the community build (had some seemingly unrelated issues before).

@lrytz lrytz modified the milestones: 2.13.3, 2.13.2 Feb 20, 2020
@retronym
Copy link
Member

I wonder if we could describe the way that stabiliizers are inserted in "spec-ese"? We should ensure that we align our implenetation here with Scala 3.

@retronym
Copy link
Member

@lrytz I'll give this some more thought and review tomorrow. A new typer mode together with new context state requires more than a little navel gazing!

@lrytz
Copy link
Member Author

lrytz commented Feb 21, 2020

Community build is green (except for one unrelated failure) https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/3215/

@lrytz

This comment has been minimized.

@lrytz
Copy link
Member Author

lrytz commented Mar 20, 2020

Ready for another review :)

@soronpo
Copy link

soronpo commented Mar 21, 2020

I tested the snapshot on my codebase and everything works great! Final blocker is now lifted and I'll be able to move up from TLS v2.12.4.

@lrytz lrytz requested a review from retronym March 25, 2020 09:01
@lrytz

This comment has been minimized.

Introduce a typer mode to know when we're type checking a qualifier
of a select, or a function part of an application. When a stabilizing
qualifier is created, insert it once we get back out of this mode.

In `x.y.foo(bar)`, if `foo` has a second implicit argument list, make
sure that the block with the stabilizer is only created after the
implicit is inferred, i.e., run `adapt` on `$stabilizer.foo(bar)`
to get `$stabilizer.foo(bar)(<implicit arg>)` first, then create

```
{
  val $stabilizer = x.y
  $stabilizer.foo(bar)(<implicit arg>)
}
```

Before, we would create

```
{
  val $stabilizer = x.y
  $stabilizer.foo(bar)
}
```

then assign a `MethodType` to the block, and run `adapt`. This worked in
some situations by chance, but not always.
@lrytz lrytz requested a review from retronym April 9, 2020 14:52
@lrytz lrytz merged commit d433c3e into scala:2.13.x Apr 15, 2020
@lrytz lrytz deleted the t11756 branch April 15, 2020 07:15
@soronpo
Copy link

soronpo commented Apr 15, 2020

Thank you all!

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.

Yet more errors in code execution order Scala 2.13 failing to emit code that works with previous Scala versions

5 participants