Backend Backports #4624
Closed
Backend Backports #4624
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`javaBinaryName` returns the internal name of a class.
Also used in BTypesFromsymbols.classBTypeFromSymbol.
Weirdly, this was discovered due to a bizarre osgi bnd error:
```
[bnd] # addAll '/Users/luc/scala/scala/build/pack/lib/scala-library.jar' with :,
[bnd] # addAll '/Users/luc/scala/scala/build/osgi/scala-library.bnd' with ,
[bnd] 1 ERRORS
[bnd] The default package '.' is not permitted by the Import-Package syntax.
[bnd] This can be caused by compile errors in Eclipse because Eclipse creates
[bnd] valid class files regardless of compile errors.
[bnd] The following package(s) import from the default package [scala.collection.generic, scala.sys.process, scala.collection.parallel.mutable, scala.util, scala.collection.parallel.immutable, scala.reflect, scala.concurrent.impl, scala.util.hashing, scala.collection.parallel, scala.collection.convert, scala.io, scala, scala.collection.concurrent, scala.util.control, scala.beans, scala.concurrent.duration, scala.collection, scala.runtime, scala.math, scala.collection.mutable, scala.concurrent, scala.sys, scala.collection.immutable, scala.ref, scala.util.matching]
[bnd] /Users/luc/scala/scala/build/osgi/scala-library.bnd: bnd failed
```
Lukas diagnosed it as a problem of the generated `$deserializeLambda$` function:
One example is `scala/App$class`. Its bytecode contains this:
```
private static synthetic $deserializeLambda$(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;
GETSTATIC scala$divApp$class.$deserializeLambdaCache$ : Ljava/util/Map;
[...]
```
so it's a static field read of a top-level class.
`$div` should obviously be `/` (which this commit rectifies)
(cherry picked from commit d2cffb9)
The Java parser should not set the `DEFERRED` flag for
default methods or static methods in interfaces.
Their bytecode doesn't have it either.
Also tightens parsing of Java abstract methods to
disallow a method body.
Here's the log of how Lukas diagnosed this:
```
quick.bin:
...
BUILD FAILED
/Users/luc/scala/scala/build.xml:69: The following error occurred while executing this line:
...
/Users/luc/scala/scala/build-ant-macros.xml:350: Could not create type mk-bin due to
java.lang.BootstrapMethodError: call site initialization exception
at java.lang.invoke.CallSite.makeSite(CallSite.java:341)
at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307)
at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297)
at scala.sys.BooleanProp$.keyExists(BooleanProp.scala:72)
at scala.sys.SystemProperties$.bool(SystemProperties.scala:78)
at scala.sys.SystemProperties$.noTraceSupression$lzycompute(SystemProperties.scala:89)
at scala.sys.SystemProperties$.noTraceSupression(SystemProperties.scala:89)
at scala.util.control.NoStackTrace$.<init>(NoStackTrace.scala:31)
at scala.util.control.NoStackTrace$.<clinit>(NoStackTrace.scala)
at scala.util.control.NoStackTrace$class.fillInStackTrace(NoStackTrace.scala:22)
at scala.util.control.BreakControl.fillInStackTrace(Breaks.scala:94)
at java.lang.Throwable.<init>(Throwable.java:250)
at scala.util.control.BreakControl.<init>(Breaks.scala:94)
at scala.util.control.Breaks.<init>(Breaks.scala:29)
at scala.collection.Traversable$.<init>(Traversable.scala:95)
at scala.collection.Traversable$.<clinit>(Traversable.scala)
at scala.package$.<init>(package.scala:40)
at scala.package$.<clinit>(package.scala)
at scala.Predef$.<init>(Predef.scala:89)
at scala.Predef$.<clinit>(Predef.scala)
at scala.tools.ant.ScalaTool.<init>(ScalaTool.scala:58)
[...]
Caused by: java.lang.invoke.LambdaConversionException:
Incorrect number of parameters for static method invokeStatic
scala.sys.BooleanProp$.scala$sys$BooleanProp$$$anonfun$2$adapted:(String)Object;
0 captured parameters, 0 functional interface method parameters, 1 implementation parameters
at java.lang.invoke.AbstractValidatingLambdaMetafactory.validateMetafactoryArgs(AbstractValidatingLambdaMetafactory.java:193)
at java.lang.invoke.LambdaMetafactory.altMetafactory(LambdaMetafactory.java:473)
at java.lang.invoke.CallSite.makeSite(CallSite.java:325)
```
[source code](https://github.com/scala/scala/blob/2.11.x/src/library/scala/sys/BooleanProp.scala#L72):
```
s => s == "" || s.equalsIgnoreCase("true")
```
bytecode:
```
INVOKEDYNAMIC $init$()Lscala/compat/java8/JFunction1; [
// handle kind 0x6 : INVOKESTATIC
java/lang/invoke/LambdaMetafactory.altMetafactory(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
// arguments:
()V,
// handle kind 0x6 : INVOKESTATIC
scala/sys/BooleanProp$.scala$sys$BooleanProp$$$anonfun$2$adapted(Ljava/lang/String;)Ljava/lang/Object;,
(Ljava/lang/String;)Ljava/lang/Object;,
3,
1,
Lscala/Serializable;.class,
0
]
CHECKCAST scala/Function1
```
The mistake seems to be that the Scala compiler incorrectly selects `$init$`
([which is a default method](https://github.com/scala/scala/blob/640ffe7fceb5d573b2c12a7c7da09bfd751036a0/src/library/scala/compat/java8/JFunction1.java#L10))
as the abstract method of `JFunction1`, whereas it should be `apply` (inherited from `Function1`).
Since we're doing mixed compilation, this is almost certainly a problem of the Java parser.
(cherry picked from commit 373db1e)
…irrors According to the spec [1] the superclass of an interface is always Object. Restores the tests that were moved to pending in bf951ec, fixex part of SI-9374. [1] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.1 (cherry picked from commit c42428d)
As with regular `Apply`-s, we should compute the generated type
based on the function's type, rather than the expected type.
In the test case, the expected type was void. Now, we correctly
use the generated type of `scala/Function1`, which is enough
to generate a subsequent POP instruction.
The tree shape involved was:
```
arg0 = {
{
$anonfun()
};
scala.runtime.BoxedUnit.UNIT
}
```
(cherry picked from commit 3bca6a2)
When there are multiple closure allocations and invocations in the same method, ensure that the callsites are re-written to the body methods in a consistent order. Otherwsie the bytecode is not stable (the local variable indices depend on the order in which the calls are re-written) (cherry picked from commit 4e9be26)
…truction Implements the necessary tests to check if a method with an InvokeDynamic instruction can be inlined into a destination class. Only InvokeDynamic instructions with LambdaMetaFactory as bootstrap methods can be inlined. The accessibility checks cannot be implemented generically, because it depends on what the bootstrap method is doing. In particular, the bootstrap method receives a Lookup object as argument which can be used to access private methods of the class where the InvokeDynamic method is located. A comment in the inliner explains the details. (cherry picked from commit e9c742e test removed for the 2.11.x backport as it requires Java 8 )
Generate the invocation to LambdaDeserializer.deserializeLambda by loading the static MODULE$ field instead of calling the static method in the mirror class. This is more scala-y. Also, mirror classes don't have an InlineInfo classfile attribute, so the inliner would yield a warning about the mirror class callsite. Also skip the stack map frame instruction - frames are computed by the ams classfile writer. (cherry picked from commit 80295ff)
If the closure body method has return type Nothing$, add an `ATHROW` instruction after the callsite. This is required for computing stack map frames, as explained in a comment in BCodeBodyBuilder.adapt. Similar for closure bodies with return type Null$. (cherry picked from commit 055a373)
retronym
force-pushed
the
backport/2.12.x-to-2.11.x-20150713
branch
from
b4cd515 to
12bd178
Compare
retronym
force-pushed
the
backport/2.12.x-to-2.11.x-20150713
branch
from
12bd178 to
d146456
Compare
A macro in shapeless was generating a tree of the form:
```
{
class C#2
new C#2
}.setType(C#1)
```
This happened due to an error in the macro; it used untypecheck
to try to fix the owner-chain consistency problem, but kept a
reference to the previous version of the block-local class symbol
`C` and used this in the resulting tree.
This commit detects the particular situation we encountered, and
avoids the crash by not creating the `NestedInfo` for the
`BType` corresponding to `C#1`. The code comment discusses why I
think this is safe, and suggests a refactoring that would mean
we only ever try to construct `NestedInfo` when we are going to
need them.
(cherry picked from commit fc7cb9a)
These cause a crash in the build of Play. We should try to bring these back once we have suitable annotation awareness. Perhaps they should only be `devWarning`-s, though. (cherry picked from commit 0b19aba)
|
i added a few more backports here: https://github.com/scala/scala/compare/2.11.x...lrytz:backports?expand=1. I'll wait opening a PR because validation currently fails (#4653 (comment)). There's one more thing I'd like to include: #4638, except for the change to use |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously been working on indylambda and the new backend on the 2.11.x branch
and merging the work forward. However, in the last few weeks, we've made a
few changes directly on the 2.12.x branch. The motivation for these was
a) to have Java 8 available to allow testing of the work, and b) eleventh hour
fixups for 2.11.0-M2.
This PR backports the important fixes to 2.11.x. These were low hanging fruit
(few merge conflicts), and low risk (only touching GenBCode / indylambda).
Review by @SethTisue / @lrytz