Skip to content

Merge 2.11.x to 2.12.x #4578

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 120 commits into from
Jun 24, 2015
Merged

Conversation

retronym
Copy link
Member

% export MB=$(git merge-base origin/2.12.x origin/2.11.x)

% git log --oneline -1
73f4056 Merge pull request #4526 from adriaanm/rework-4523

% transcript git log --graph --oneline --decorate $MB...origin/2.11.x
https://gist.github.com/2bc8a21fff6d43474b4a

... No commits marked with nomerge or backport

% transcript git diff --stat $MB...origin/2.11.x
https://gist.github.com/5f170c8b4825e969fd3b

% git merge 2.11.x
Auto-merging versions.properties
Auto-merging test/scaladoc/scalacheck/HtmlFactoryTest.scala
Auto-merging test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOpts.scala
Auto-merging test/junit/scala/collection/IteratorTest.scala
CONFLICT (content): Merge conflict in test/junit/scala/collection/IteratorTest.scala
Auto-merging test/files/neg/names-defaults-neg.scala
Auto-merging test/files/neg/names-defaults-neg.check
Auto-merging src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala
Auto-merging src/scaladoc/scala/tools/nsc/doc/html/page/diagram/DotDiagramGenerator.scala
Auto-merging src/scaladoc/scala/tools/nsc/doc/html/page/Template.scala
Removing src/repl/scala/tools/nsc/interpreter/session/JLineHistory.scala
Auto-merging src/repl/scala/tools/nsc/interpreter/Tabulators.scala
Removing src/repl/scala/tools/nsc/interpreter/JLineReader.scala
Removing src/repl/scala/tools/nsc/interpreter/Delimited.scala
Auto-merging src/repl-jline/scala/tools/nsc/interpreter/jline/FileBackedHistory.scala
Auto-merging src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
Auto-merging src/reflect/scala/reflect/internal/Symbols.scala
Auto-merging src/reflect/scala/reflect/internal/StdNames.scala
Auto-merging src/manual/scala/man1/scalac.scala
CONFLICT (content): Merge conflict in src/manual/scala/man1/scalac.scala
Auto-merging src/library/scala/util/Try.scala
Auto-merging src/library/scala/collection/mutable/ListBuffer.scala
Auto-merging src/library/scala/collection/immutable/Stream.scala
Auto-merging src/library/scala/collection/immutable/List.scala
Auto-merging src/library/scala/collection/Iterator.scala
Auto-merging src/compiler/scala/tools/nsc/typechecker/Typers.scala
Auto-merging src/compiler/scala/tools/nsc/typechecker/Infer.scala
Auto-merging src/compiler/scala/tools/nsc/typechecker/Implicits.scala
Auto-merging src/compiler/scala/tools/nsc/transform/UnCurry.scala
CONFLICT (content): Merge conflict in src/compiler/scala/tools/nsc/transform/UnCurry.scala
Auto-merging src/compiler/scala/tools/nsc/transform/Erasure.scala
Auto-merging src/compiler/scala/tools/nsc/transform/Delambdafy.scala
CONFLICT (content): Merge conflict in src/compiler/scala/tools/nsc/transform/Delambdafy.scala
Auto-merging src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
Auto-merging src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
Auto-merging src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
Auto-merging src/compiler/scala/tools/nsc/ast/DocComments.scala
Auto-merging src/compiler/scala/tools/nsc/Global.scala
Auto-merging build.xml
CONFLICT (content): Merge conflict in build.xml
Auto-merging build.sbt
Auto-merging bincompat-forward.whitelist.conf
Automatic merge failed; fix conflicts and then commit the result.

% ant -q all.clean test.suite test.scaladoc

...

Ichoran and others added 30 commits March 31, 2015 14:12
Added `defaultWriteObject` to the beginning of `writeObject` and `defaultReadObject` to the beginning of `readObject` as required by specs:
[writing](http://docs.oracle.com/javase/6/docs/platform/serialization/spec/output.html#861), [reading](http://docs.oracle.com/javase/6/docs/platform/serialization/spec/input.html#2971).

Verified that it is a no-op in terms of serialization stream (but it provides hooks that Infinispan and others may use).

No explicit tests.  If there is a change in serialization, t8549 will catch it.
`LambdaMetafactory` generates code to perform a limited number
of type adaptations when delegating from its implementation of
the functional interface method to the lambda target method.

These adaptations are: numeric widening, casting, boxing and unboxing.

However, the semantics of unboxing numerics in Java differs to Scala:
they treat `UNBOX(null)` as cause to raise a `NullPointerException`,
Scala (in `BoxesRuntime.unboxTo{Byte,Short,...}`) reinterprets the
null as zero.

Furthermore, Java has no idea how to adapt between a value class and
its wrapped type, nor from a void return to `BoxedUnit`.

This commit detects when the lambda target method would require
such adaptation. If it does, an extra method, `$anonfun$1$adapted` is
created to perform the adaptation, and this is used as the target
of the lambda.

This obviates the use of `JProcedureN` for `Unit` returning
lambdas, we know use `JFunctionN` as the functional interface
and bind this to an `$adapted` method that summons the instance
of `BoxedUnit` after calling the `void` returning lambda target.

The enclosed test cases fail without boxing changes. They don't
execute with indylambda enabled under regular partest runs yet,
you need to add scala-java8-compat to scala-library and pass
the SCALAC_OPTS to partest manually to try this out, as described
in scala#4463. Once we enable indylambda
by default, however, this test will exercise the code in this patch
all the time.

It is also possible to run the tests with:

```
% curl https://oss.sonatype.org/content/repositories/releases/org/scala-lang/modules/scala-java8-compat_2.11/0.4.0/scala-java8-compat_2.11-0.4.0.jar > scala-java8-compat_2.11-0.4.0.jar
% export INDYLAMBDA="-Ydelambdafy:method -Ybackend:GenBCode -target:jvm-1.8 -classpath .:scala-java8-compat_2.11-0.4.0.jar"
qscalac $INDYLAMBDA test/files/run/indylambda-boxing/*.scala && qscala $INDYLAMBDA Test
```
…ple-deprecated

Fixed deprecation warning in scaladoc example of Try
[indylambda] Relieve LambdaMetafactory of boxing duties [ci: last-only]
To support serialization, we use the alternative lambda metafactory
that lets us specify that our anonymous functions should extend the
marker interface `scala.Serializable`. They will also have a
`writeObject` method added that implements the serialization proxy
pattern using `j.l.invoke.SerializedLamba`.

To support deserialization, we synthesize a `$deserializeLamba$`
method in each class with lambdas. This will be called reflectively by
`SerializedLambda#readResolve`. This method in turn delegates to
`LambdaDeserializer`, currently defined [1] in `scala-java8-compat`,
that uses `LambdaMetafactory` to spin up the anonymous class and
instantiate it with the deserialized environment.

Note: `LambdaDeserializer` can reuses the anonymous class on subsequent
deserializations of a given lambda, in the same spirit as an
invokedynamic call site only spins up the class on the first time
it is run. But first we'll need to host a cache in a static field
of each lambda hosting class. This is noted as a TODO and a failing
test, and will be updated in the next commit.

`LambdaDeserializer` will be moved into our standard library in
the 2.12.x branch, where we can introduce dependencies on the
Java 8 standard library.

The enclosed test cases must be manually run with indylambda enabled.
Once we enable indylambda by default on 2.12.x, the test will
actually test the new feature.

```
% echo $INDYLAMBDA
-Ydelambdafy:method -Ybackend:GenBCode -target:jvm-1.8 -classpath .:scala-java8-compat_2.11-0.5.0-SNAPSHOT.jar
% qscala $INDYLAMBDA -e "println((() => 42).getClass)"
class Main$$anon$1$$Lambda$1/1183231938
% qscala $INDYLAMBDA -e "assert(classOf[scala.Serializable].isInstance(() => 42))"
% qscalac $INDYLAMBDA test/files/run/lambda-serialization.scala && qscala $INDYLAMBDA Test
```

This commit contains a few minor refactorings to the code that
generates the invokedynamic instruction to use more meaningful
names and to reuse Java signature generation code in ASM rather
than the DIY approach.

[1] scala/scala-java8-compat#37
We add a static field to each class that defines lambdas that
will hold a `ju.Map[String, MethodHandle]` to cache references to
the constructors of the classes originally created by
`LambdaMetafactory`.

The cache is initially null, and created on the first deserialization.

In case of a race between two threads deserializing the first
lambda hosted by a class, the last one to finish will clobber
the one-element cache of the first.

This lack of strong guarantees mirrors the current policy in
`LambdaDeserializer`.

We should consider whether to strengthen the combinaed guarantee here.
A useful benchmark would be those of the invokedynamic instruction,
which allows multiple threads to call the boostrap method in parallel,
but guarantees that if that happens, the results of all but one will
be discarded:

> If several threads simultaneously execute the bootstrap method for
> the same dynamic call site, the Java Virtual Machine must choose
> one returned call site object and install it visibly to all threads.

We could meet this guarantee easily, albeit excessively, by
synchronizing `$deserializeLambda$`. But a more fine grained
approach is possible and desirable.

A test is included that shows we are able to garbage collect
classloaders of classes that have hosted lambda deserialization.
The overriding pairs cursor used to detect erased signature clashes
was turning a blind eye to any pair that contained a private method.

However, this could lead to a `VerifyError` or `IllegalAccessError`.

Checking against javac's behaviour in both directions:

```
% cat sandbox/Test.java
public abstract class Test {
  class C { int foo() { return 0; } }
  class D extends C { private <A> int foo() { return 1; } }
}

% javac sandbox/Test.java
sandbox/Test.java:3: error: name clash: <A>foo() in Test.D and foo() in Test.C have the same erasure, yet neither overrides the other
  class D extends C { private <A> int foo() { return 1; } }
                                      ^
  where A is a type-variable:
    A extends Object declared in method <A>foo()
1 error
```

```
% cat sandbox/Test.java
public abstract class Test {
  class C { private int foo() { return 0; } }
  class D extends C { <A> int foo() { return 1; } }
}

% javac sandbox/Test.java
%
```

This commit only the exludes private symbols from the superclass
from the checks by moving the test from `excludes` to `matches`.
A previous change disabled -Ydelambdafy:method for specialized
lambdas, as `DelambdafyTransformer` made no attempt to emit
the requisite machinery to avoid boxing.

This was loosened to allow them under `-target:jvm-1.8`, in the
knowledge that `indylambda` would do the right thing.

However, this wasn't quite right: indylambda is only supported
in `GenBCode`, so we should consider that setting as well.
We neglected to do this earlier.
Document -target:jvm-1.8 in the man page
Avoid inefficient specialied lambdas w. delambdafy jvm-1.8, GenASM
…ation

[indylambda] Support lambda {de}serialization
SI-9286 Check subclass privates for "same type after erasure"
I checked the intent with Martin, who said:

> [...] qualified private members are inherited like other members,
> it’s just that their access is restricted.

I've locked this in with a test as well.
This contains LambdaDeserializer, which we refer to in our
synthetic `$deserializeLambda$` method under -target:jvm-1.8.

Note: this library is only reference in the Ant build under a
non-standard build flag that includes that library into
scala-library.jar. This is only for our internal use in running
partest for indylambda. The SBT build doesn't have the same
option at the moment.
Tracks nullness of values using an ASM analyzer.

Tracking nullness requires alias tracking for local variables and
stack values. For example, after an instance call, local variables
that point to the same object as the receiver are treated not-null.
When inlining an instance call, the inliner has to ensure that a NPE
is still thrown if the receiver object is null. By using the nullness
analysis, we can avoid emitting this code in case the receiver object
is known to be not-null.
Address feedback in  scala#4516 / 57b8da4. Save allocations of
NullnessValue - there's only 4 possible instances. Also save tuple
allocations in InstructionStackEffect.
Spark has been shipping a forked version of our REPL for
sometime. We have been trying to fold the patches back into
the mainline so they can defork. This is the last outstanding
issue.

Consider this REPL session:

```
scala> val x = StdIn.readInt

scala> class A(a: Int)

scala> serializedAndExecuteRemotely {
  () => new A(x)
}
```

As shown by the enclosed test, the REPL, even with the
Spark friendly option `-Yrepl-class-based`, will re-initialize
`x` on the remote system.

This test simulates this by running a REPL session, and then
deserializing the resulting closure into a fresh classloader
based on the class files generated by that session. Before this
patch, it printed "evaluating x" twice.

This is based on the Spark change described:

  mesos/spark#535 (comment)

A followup commit will avoid the `val lineN$read = ` part if we
import classes or type aliases only.

[Original commit from Prashant Sharma, test case from Jason Zaugg]
We only need to introduce the temporary val in the imports
wrapper when we are importing a val or module defined in the REPL.

The test case from the previous commit still passes, but
we are generating slightly simpler code.

Compared to 2.11.6, these two commits result in the following
diff:

  https://gist.github.com/retronym/aa4bd3aeef1ab1b85fe9
SI-9321 Clarify spec for inheritance of qualified private
backuitist and others added 7 commits June 22, 2015 16:30
SI-9253 avoid IndexOutOfBoundsException in TypeMaps.correspondingTypeArgument
SI-9359 Fix InnerClass entry flags for nested Java enums
Spec: Add lost references, cleanup
SI-9206 Fix REPL code indentation
@scala-jenkins scala-jenkins added this to the 2.12.0-M2 milestone Jun 24, 2015
@retronym
Copy link
Member Author

Review by @lrytz

@retronym
Copy link
Member Author

Conflicts in build.xml were actors vs repl-jline subproject changes. Other conflicts were in code that I had changed related to indylamdbda vs removal of jvm:1.{5,6,7} targets.

@retronym retronym force-pushed the merge/2.11.x-to-2.12.x-20150624 branch from 7dad058 to 1f7417c Compare June 24, 2015 08:10
@lrytz
Copy link
Member

lrytz commented Jun 24, 2015

now there's a different failure:

  [partest] !! 20 - scalap/t8679.scala                        [output differs]

Adriaan mentioned seeing the exact same failure yesterday when building the 2.12 branch, so it's unrealated to the merge. Interestingly it passed in the previous run (before you updated the t8918-unary-ids test), see https://scala-ci.typesafe.com/job/scala-2.12.x-validate-test/69/consoleFull, so it seems to be flaky.

Started a rebuild.

@lrytz
Copy link
Member

lrytz commented Jun 24, 2015

hmm, again.. i cannot reproduce it locally, tried in various ways.

@lrytz
Copy link
Member

lrytz commented Jun 24, 2015

Still cannot reproduce.

I ran the same command as jenkins

ant -Dstarr.version=2.12.0-1f7417c-SNAPSHOT -Dscalac.args.optimise=-optimise -Dlocker.skip=1 -Dextra.repo.url=https://scala-ci.typese.com/artifactory/scala-pr-validation-snapshots/ test.suite

on my machine. Even tried on a linux machine with OpenJDK and the same ANT_OPTS. Never saw scalap/t8679.scala failing.

Re-building one last time.

@lrytz
Copy link
Member

lrytz commented Jun 24, 2015

nope. still the same. i don't know what's up. /home/jenkins/workspace/scala-2.12.x-validate-test/test/files/scalap/t8679-scalap.log is empty.

@adriaanm
Copy link
Contributor

Here's the list of PRs that saw scalap failures like this one: #4460, #4504, #4514, #4526, #4547, #4556, #4561, #4562, #4565, #4578, #4580

@adriaanm
Copy link
Contributor

In other words, let's not be blocked by the flakiness, even though we should probably find out what's causing it... I suggest we disable the huge test and see if it's flake-inducing.

@adriaanm
Copy link
Contributor

@retronym thinks it's due to a hotspot bug, based on https://groups.google.com/forum/#!msg/scala-user/0GsO0Hv9yfA/NF941LpsqEAJ

adriaanm added a commit that referenced this pull request Jun 24, 2015
@adriaanm adriaanm merged commit fd526c7 into scala:2.12.x Jun 24, 2015
@adriaanm
Copy link
Contributor

I upgraded our jenkins workers to ubuntu vivid to get java 1.8.0 update 45 build 14, to get a fix for https://bugs.openjdk.java.net/browse/JDK-8058847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel, which we suppose was causing flaky test failure for scalap test

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.