Skip to content

Clean up scala.runtime.function, refactor closure adaptation #10920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 26, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Dec 26, 2020

Part of #10602

Instead, use the equivalent classes in the Scala 2 standard library in
scala.runtime.java8.

The only difference is that these classes are written in Scala instead
of Java, which exposed a bug in the Scala2Unpickler: we were calling
`atPhaseNoLater(picklerPhase)`, but this doesn't actually do anything in
situation where the Pickler phase is not used (for example, when using
the `QuoteCompiler`), this is fixed by introducing a new
`atPhaseBeforeTransforms` which does the right thing.

Because the `FirstTransform` phase itself might not be present, this
also required fixing `Phase#next` and `Phase#prev` to work on `NoPhase`:
the issue was that `myBase.NoPhase` failed with an NPE which we fix by
making `NoPhase` a static object instead of a member of `PhasesBase`.
Since an instace of `Phase` contains mutable state this requires making
it `@sharable` to pass our tests, this is safe since we never mutate
anything in `NoPhase` (this also matches what we do for `NoSymbol` and
`NoDenotation`).
Since Scala 2.12, the regular scala.Function{0, 1, 2} are valid SAM
types, so these interfaces aren't needed anymore.
@smarter smarter force-pushed the jfunction-cleanup-4 branch from 62d687e to 58064cf Compare December 26, 2020 16:18
As described in details in `Erasure#adaptClosure`, in some situation we
need to perform some transformation to make the closure implementation
method type match the type of the single abstract method of the
functional interface this closure implements, previously the
responsability for doing this was split in three places:
- The FunctionalInterfaces phase was setting the functional
  interface of specializable FunctionN instances to the corresponding
  specialized JFunction*mc* interface
- The backend set the functional interface of a Unit-returning FunctionN
  to JProcedureN
- And `adaptClosure` generated a bridge method in all other
  situations where some adaptation was needed.

This commit centralizes all of this logic in `adaptClosure` which allows
us to remove one phase, and in general makes it easier to follow what's
going on.
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@smarter smarter merged commit d8a6056 into scala:master Dec 26, 2020
@smarter smarter deleted the jfunction-cleanup-4 branch December 26, 2020 20:45
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
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.

3 participants