Skip to content

Merge 2.10.x to master #2343

Merged
merged 71 commits into from Apr 2, 2013
@retronym
The Scala Programming Language member
retronym commented Apr 2, 2013

The main area of conflict was build.xml (refactorings/bug fixes from 2.10.x + modularization from master).

I merged the MiMa targets into the master build.xml, but disabled their execution until 2.11.0.

Review by @adriaanm

retronym and others added some commits Mar 13, 2013
@retronym retronym SI-7242 Fix crash when inner object mixes in its companion
Given:

    class C {
      trait T { C.this }            // C$T$$$outer$ : C
      object T extends T { C.this } // C$T$$$outer$ : C.this.type
    }

object T ended up with a definitions for both of the accessors.
These cannot co-exist, as they have the same erased type. A crash
ensued almost immediately in explitouter.

Fortunately, the solution is straightforward: we can just omit
the mixin outer accessor altogether, the objects own outer accessor
is compatible with it.

    scala> :javap C.T
    Compiled from "<console>"
    public interface C$T{
        public abstract C C$T$$$outer();
    }

    scala> :javap C.T$
    Compiled from "<console>"
    public class C$T$ extends java.lang.Object implements C$T{
        public C C$T$$$outer();
        public C$T$(C);
    }

I also added an assertion to give a better error message in
case we find ourselves here again.

Also includes tests from SI-3994, which I'll mark as a duplicate.
2b5fde7
@retronym retronym SI-6921 SI-7239 Tread lightly during exploratory typing
When deciding whether an Assign is a named argument or
and assignment expression, or when looking at arguments
that the current selection is applied to in order to
evaluate candidate implicit views, we risk polluting
the tree by setting error types. This happens even
if we are in 'silent' mode; that mode does silence the
error report, but not the side effect on the tree.

This commit adds strategic `duplicate` calls to
address the problem symptomatically.

Duplicating trees and retyping in general reach into
the domain of bugs umbrella-ed under SI-5464, but in
these places we should be safe because the tree is in
the argument position, not somewhere where, for example,
a case class-es synthetic companion object might be
twice entered into the same scope.

Longer term, we'd like to make type checking side effect
free, so we wouldn't need to play whack-a-mole like this.
That idea is tracked under SI-7176.
174334b
@retronym retronym SI-7232 Fix Java import vs defn. binding precendence
Java Spec:
> A single-type-import declaration d in a compilation unit c
> of package p that imports a type named n shadows, throughout
> c, the declarations of:
>   - any top level type named n declared in another compilation
>     unit of p
>   - any type named n imported by a type-import-on-demand
>     declaration in c
>   - any type named n imported by a static-import-on-demand
>     declaration in c

Scala Spec:
> Bindings of different kinds have a precedence defined on them:
>   1. Definitions and declarations that are local, inherited, or made
>      available by a package clause in the same compilation unit where
>      the definition occurs have highest precedence.
>   2. Explicit imports have next highest precedence.
6e79370
@retronym retronym SI-7249 Reign in overzealous Function0 optimization.
The fix for SI-1247 went too far, and could result in
premature evaluation of the expression that yields the
Function0.

This commit checks that said expression is safe to inline.
If not, a wrapper `() => ....` is still required.

The optimization is still enabled in sitations like the
original test case, run/t1247.scala.
552b623
@JamesIry JamesIry Read version 51 (JDK 7) class files.
This commit makes the ClassFileReader/ICodeReader parse class files
from JDK 7 (class file version 51). It does that by skipping over
the method handle related entries in the constant pool and by doing
some dummy processing on invoke dynamic instructions. The inliner
is updated to not try to inline a method with an invoke dynamic
instruction. A place holder INVOKE_DYNAMIC instruction is added to ICode
but it is designed to create an error if there's ever any attempt to
analyze it. Because the inliner is the only phase that ever tries
to analyze ICode instructions not generated from Scala source and
because Scala source will never emit an INVOKE_DYNAMIC, the place
holder INVOKE_DYNAMIC should never cause any errors.

A test is included that generates a class file with INVOKE_DYNAMIC
and then compiles Scala code that depends on it.
e78896f
@soc soc [backport] SI-7237 Always choose ForkJoinTaskSupport
... on Java 6 and above.

ForkJoinTaskSupport works on Hotspot, Avian and J9,
while ThreadPoolTaskSupport causes the test
test/files/scalacheck/parallel-collections
to reliably hang on all three.

We keep ThreadPoolTaskSupport around to keep the hope
alive that we still have a glimpse of 1.5 support.
67b8de7
@soc soc SI-7258 Don't assume order of reflection values in t6223
test/files/run/t6223's check file expects a specific
ordering of the reflected values. The ordering is not
guaranteed by the runtime/reflection API and can change.

Therefore, sort the values before comparing them.
ef85a10
@Blaisorblade Blaisorblade SI-7253: respect binary compatibility constraints
From the JLS one can prove that moving a method to a superclass is a binary
compatible change, both forward and backward. That's because when compiling a
method call `c.foo()`, where c: C, the output descriptor *must* refer to `C` and
not to the class where `foo()` is actually defined.

This patch just ensures that, and adds a test comparing generated descriptors
against the Javac output.

The sample code is from Paul Philipps, the fix and the bytecode comparison code
from me.

From 2006 (9954eaf, a fix for bug 455 in the
old bug tracker
http://www.scala-lang.org/sites/default/files/aladdin/displayItem.do%3Fid=455.html)
until 2.9, Scalac has followed this rule "often" (that is, when C is *not* an
interface).

This behavior was wrong, but the bug was hard to trigger. AFAICS, this can
create problems only when moving a method to a super interface in a library and
expecting forward binary compatibility - that is, compiling some Scala client
code against the new version of the library, and trying to run this code against
the old version of the library. This change grows an interface, so it is valid
only if clients are supposed to *not* implement the library.  Apparently, this
is so rare that nobody noticed.

Since 2.10 (0bea2ab), Scalac follows this rule
*only* when C is an interface (I assume by oversight, since the main change was
an accessibility check), so the bug was finally triggered.

The new code will have to emit INVOKEINTERFACE instead of INVOKEVIRTUAL a bit
more often, compared to 2.9 (but not to 2.10). I don't know whether
INVOKEINTERFACE is noticeably slower (it shouldn't be); but this is the safest
fix since this behavior is mandated by the JLS.
If somebody disagrees and believes the 2.9 is significantly faster, IMHO he
should send a separate pull request (although ProGuard is probably a better
place for the change).
386a5bd
@Blaisorblade Blaisorblade Remove fragile code, made redundant by previous commit
The JLS requires, as an exception to generate calls referring to Object for all
methods defined in Object itself; hence, whenever the method owner is Object,
the call should refer to the method owner. The previous commit implemented this
directly in useMethodOwner, so the special case for Object in isInterfaceCall is
now redudant. Hence, remove it.
b0560c5
@Blaisorblade Blaisorblade SI-7253: update comments and naming
Rename isInterfaceCall -> needsInterfaceCall, which is more accurate since a
receiver can't literally be an interface call.
6f4a594
@retronym retronym SI-7259 Fix detection of Java defined Selects
The fix for SI-3120, 3ff7743, introduced a fallback within
`typedSelect` that accounted for the ambiguity of a Java
selection syntax. Does `A.B` refer to a member of the type `A`
or of the companion object `A`? (The companion object here is a
fiction used by scalac to group the static members of a Java
class.)

The fallback in `typedSelect` was predicated on
`context.owner.enclosingTopLevelClass.isJavaDefined`.
However, this was incorrectly including Select-s in top-level
annotations in Scala files, which are owned by the enclosing
package class, which is considered to be Java defined. This
led to nonsensical error messages ("type scala not found.")

Instead, this commit checks the compilation unit of the context,
which is more direct and correct. (As I learned recently,
`currentUnit.isJavaDefined` would *not* be correct, as a lazy type
might complete a Java signature while compiling some other compilation
unit!)

A bonus post factum test case is included for SI-3120.
f046853
@retronym retronym Reduce duplication and increase verbosity in MiMa execution.
This helps to see whether we've broken forward or backward
binary compatibility.
e90efd6
@xeno-by xeno-by removes duplication in FreeDef extractors 99bdebb
@adriaanm adriaanm Merge pull request #2247 from retronym/ticket/7232-2
SI-7232 Fix Java import vs defn. binding precendence
44af744
@adriaanm adriaanm Merge pull request #2253 from retronym/ticket/7239
SI-6921 SI-7239 Tread lightly during exploratory typing
752aa52
@adriaanm adriaanm Merge pull request #2255 from retronym/ticket/7249
SI-7249 Reign in overzealous Function0 optimization.
144671e
@xeno-by xeno-by an amazing discovery made by Iulian
Traces were stalling macro expansions by evaluating messages even when
-Ymacro-debug-* flags were disabled.
5db04eb
@scala-jenkins

Job pr-rangepos-per-commit failed for 5db04eb (results):


Took 6 s.
to rebuild, comment "PLS REBUILD/pr-rangepos-per-commit@5db04eb" on PR 2276

Job pr-checkin-per-commit failed for 5db04eb (results):


Took 42 s.
to rebuild, comment "PLS REBUILD/pr-checkin-per-commit@5db04eb" on PR 2276

adriaanm and others added some commits Mar 18, 2013
@adriaanm adriaanm Merge pull request #2271 from retronym/topic/mima-refactor
Reduce duplication and increase verbosity in MiMa execution.
bbd69f5
@vigdorchik vigdorchik SI-5699 correct java parser for annotation defs.
Correct java source parser not to insert a constructor with the type
 of its value method.
50ee635
@scala-jenkins

Job pr-rangepos-per-commit failed for 50ee635 (results):


Took 8 s.
to rebuild, comment "PLS REBUILD/pr-rangepos-per-commit@50ee635" on PR 2274

Job pr-checkin-per-commit failed for 50ee635 (results):


Took 35 s.
to rebuild, comment "PLS REBUILD/pr-checkin-per-commit@50ee635" on PR 2274

kzys and others added some commits Mar 20, 2013
@kzys kzys SI-7013 Scaladoc: Fix StackOverflowError
No one see the result value of parse and if so, it's tail-recursive.
df29290
@xeno-by xeno-by fixes the craziness in JavaUniverse.log
This long-standing, but trivial to fix nuisance in the implementation of
runtime reflection actively avoided being fixed in both 2.10.0 and 2.10.1.

It's finally the time to put it to a rest.
fc46281
@JamesIry JamesIry Merge pull request #2269 from retronym/ticket/7259
SI-7259 Fix detection of Java defined Selects
241c8c0
@JamesIry JamesIry Merge pull request #2261 from soc/SI-7258
SI-7258 Don't assume order of reflection values in t6223
fab81c9
@JamesIry JamesIry Merge pull request #2283 from scalamacros/topic/silence-the-log
fixes the craziness in JavaUniverse.log
67e54b7
@JamesIry JamesIry Merge pull request #2276 from scalamacros/topic/macro-trace
an amazing discovery made by Iulian
2f3a7fa
@paulp paulp Merge pull request #2245 from retronym/ticket/7242
SI-7242 Fix crash when inner object mixes in its companion
54f1a39
@paulp paulp Merge pull request #2268 from scalamacros/topic/shannon-entropy
removes duplication in FreeDef extractors
e5a081c
@JamesIry JamesIry Merge pull request #2274 from vigdorchik/ticket/si-5699
SI-5699 correct java parser for annotation defs.
62dd51a
@paulp paulp Merge pull request #2264 from Blaisorblade/issue/7253
SI-7253: respect binary compatibility constraints
c5ad8b4
@retronym retronym SI-7290 Discard duplicates in switchable alternative patterns.
The pattern matcher must not allow duplicates to hit the
backend when generating switches. It already eliminates then
if they appear on different cases (with an unreachability warning.)

This commit does the same for duplicated literal patterns in an
alternative pattern: discard and warn.
2e0be83
@retronym retronym Expand test for SI-6124 to demonstrate cause of SI-7285. 499962d
@retronym retronym SI-7285 Fix match analysis with nested objects.
The fix for SI-6146 introduced `nestedMemberType` to
enumerate sealed subtypes based on the (prefixed) type
of the scrutinee and the symbols of its sealed subclasses.
That method needed to widen `ThisType(modSym)`s to
`ModuleTypeRef(modSym)` before calling `asSeenFrom`.

However, this could lead to confused in the match analysis,
which sees `ModuleTypeRef` as distinct from singleton types
on the same modules (after all, they aren't =:=). Spurious
warnings ensued.

This commit makes two changes:

 - conditionally re-narrow the result of `asSeenFrom` in `nestedMemberType`.
 - present `a.b.SomeModule.type` as `SomeModule` in warnings emitted
   by the pattern matcher.
dd89b00
@retronym retronym SI-6210 Test case for already-fixed pattern matcher bug
The fix arrived in SI-6022 / #1100 / 2.10.0-M7.
47fc00d
@retronym retronym SI-7246 Make $outer pointer elision Java aware
In e0853b3, a space-saving optimization elided the outer pointer
of inner classes if the the (protected) outer pointer of the
immediate parent class was guaranteed to point to the same instance.

But, this check failed to account for Java parent classes, which
don't follow the Scala scheme. This commit disables the optimization
in that case.

The original test case in e0853b3 was anemic, I've fleshed it out to:

  - test the presense or absense of $outer pointers with Java reflection
  - test the optimization works in the presense of aliased and annotated
    aliased types. (The former worked already, the latter required a
    change to the implementation.)
  - Test the negative case when the prefixes don't line up and the
    subclass in fact needs its own $outer.

This patch is based on work by Euguene Vigdorchik with some
additions by Jason Zaugg.
cd9e03a
@adriaanm adriaanm Merge scala/2.10.x into build-cleanup-2.10
Conflicts:
	build.xml
6802371
@paulp paulp Merge pull request #2266 from paulp/issue/7251
SI-7251, compiler crash with $.
ed45b71
@paulp paulp Merge pull request #2279 from kzys/jira-7013
SI-7013 Scaladoc: Fix StackOverflowError
114d708
@paulp paulp Merge pull request #2295 from adriaanm/build-cleanup-2.10
Build cleanup 2.10
fcc22e2
@adriaanm adriaanm Fix typos in build.xml df61e04
@retronym retronym SI-7299 Improve error message for eta-expanding 23+ param method
Before, we got `error: missing arguments for method f`.
b95ca32
@kzys kzys SI-6580 Scaladoc: Should not close void elements
Because it will generate a useless element like "</img>".

To made matters worse, Scaladoc used to generate the element with
attributes (like </img src="...">). That's why we had SI-6580.
b4344e1
James Iry Log when invokedynamic instruction is encountered
Based on the review of scala#2257, this
commit adds a debuglog when an invokedynamic instruction is found
during class file parsing as a reminder that the implementation is
just a place holder.
b7cbda7
@vigdorchik vigdorchik SI-6387 Clones accessor before name expansion
When a symbol's name is expanded due to a conflict during
composition (e.g. multiple traits with same-named members, but
which are not both visible at the language level in the concrete
class) the compiler renames some symbols with expanded names which
embed the full name of the declaring class to avoid clashes.

In the rare cases when the accessor overrides the member in base
class, such expansion either results in AbstractMethodError when
the base method is abstract, or, even worse, can change the
semantics of the program.

To avoid such issues, we clone the accessor symbol, clear its
ACCESSOR flag and enter the symbol with an unchanged name.
4e10b2c
@adriaanm adriaanm Merge pull request #2293 from retronym/ticket/6210
SI-6210 Test case for already-fixed pattern matcher bug
424f00e
@paulp paulp Merge pull request #2278 from kzys/jira-6580
SI-6580 Scaladoc: Should not close void elements
e5b5414
@paulp paulp Merge pull request #2257 from JamesIry/2.10.x_classfile_51
Read version 51 (JDK 7) class files.
28a6574
@paulp paulp Merge pull request #2304 from adriaanm/build-cleanup-2.10
Fix typos in build.xml
33503d1
@paulp paulp Merge pull request #2306 from retronym/ticket/7299
SI-7299 Improve error message for eta-expanding 23+ param method
e17121d
@paulp paulp Merge pull request #2270 from retronym/ticket/7246-2
SI-7246 Make $outer pointer elision Java aware
5e8f4b2
@paulp paulp Merge pull request #2252 from soc/SI-7237-2.10
[backport] SI-7237 Always choose ForkJoinTaskSupport
f0b866e
@adriaanm adriaanm restored dependency of pack.done on quick.bin 6620758
@adriaanm adriaanm run test.bc as part of tests on 2.10.x
also added test.bc-opt target as binary compatibility differs between (non)optimized builds...
86bea6a
@adriaanm adriaanm remove unused ant targets: test.ant, test.classload, test.positions 3bb1af9
@adriaanm adriaanm Formatting. Introduce {asm,forkjoin,fjbg}-classes props. d85c3f1
@adriaanm adriaanm Remove duplication in java builds of fjbg/asm/forkjoin ac1a0f0
@adriaanm adriaanm Preparation for faster PR validation
Added starr.number, so that `ant -propertyfile starr.number -Dlocker.skip=1`
works across branches.

Introduced target `test.core`, which skips `test.stability`, since that requires
`locker == quick`, whereas `locker == starr` with the above settings.
(It's interesting to see for which files it fails when that assumption is broken.)

Stability will still be tested on a nightly basis. It's rarely broken.
88b7a72
@adriaanm adriaanm Let continuations library sources determine docs.lib's actuality c5511de
@kzys kzys Scaladoc: Load scripts at the bottom, and with a defer attribute 0cc9496
@retronym retronym SI-7290 Minor cleanups driven by review comments.
  - make a def a val, we only need to compute it once
  - add a clarifying comment
  - only report the first duplicate
c3ad5af
@adriaanm adriaanm Run test.scaladoc before test.suite. Fail fast. c2da1c5
@adriaanm adriaanm ant clean only zaps the quick stage dc5326c
@paulp paulp Merge pull request #2288 from paulp/pr/2273-with-edits
SI-6387 Clones accessor before name expansion
9e1483d
@adriaanm adriaanm Clean up pack targets. Better dependency tracking.
Target pack.@{project} is considered up to date iff
classes emitted by quick.@{project} are.

TODO: encapsulate "classes emitted by quick.@{project}".
For now, they are duplicated as references
to `${build-@{stage}.dir}/classes/@{destproject}`
(in staged-scalac and *.build.path)
3ec36bb
@adriaanm adriaanm Merge pull request #2291 from retronym/ticket/7290
SI-7290 Discard duplicates in switchable alternative patterns.
f2a74c5
@adriaanm adriaanm Merge pull request #2292 from retronym/ticket/7285
SI-7285 Fix match analysis with nested objects
66fb811
@gkossakowski gkossakowski Merge pull request #2314 from adriaanm/build-fixes
further tweaks to the ant build
00fcd46
@adriaanm adriaanm Use java-diff-utils for diffing in partest.
We now use the unified diff format, hence the updated check files.

It's not clear to me how partest's classpath is managed,
but the approach in this commit works for the ant task and script invocation.
The diffutils jar is injected in the parent classloader.
ba21f36
@adriaanm adriaanm Strip version suffix from diffutils. 024cdd4
@gkossakowski gkossakowski Merge pull request #2330 from adriaanm/partest-difftools
Use java-diff-utils for diffing in partest.
9590dfa
@adriaanm adriaanm Merge pull request #2321 from kzys/js-defer
Scaladoc: Load scripts at the bottom, and with a defer attribute
a4785ba
@adriaanm adriaanm was assigned Apr 2, 2013
@retronym retronym Merge remote tracking branch 'origin/2.10.x' into topic/merge-2.10.x-…
…to-v2.11.0-M2-74-g00e6c8b

Conflicts:
    bincompat-backward.whitelist.conf
    bincompat-forward.whitelist.conf
    build.xml
    src/compiler/scala/reflect/reify/utils/Extractors.scala
    src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
    src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
    src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
    src/compiler/scala/tools/nsc/typechecker/Typers.scala
    src/partest/scala/tools/partest/nest/ReflectiveRunner.scala
    src/reflect/scala/reflect/internal/Types.scala
    src/reflect/scala/reflect/runtime/JavaUniverse.scala
    test/files/run/inline-ex-handlers.check
    test/files/run/t6223.check
    test/files/run/t6223.scala
    test/scaladoc/scalacheck/IndexTest.scala
51d96a3
@adriaanm
The Scala Programming Language member
adriaanm commented Apr 2, 2013

LGTM, thanks!

@adriaanm adriaanm merged commit c77dd12 into scala:master Apr 2, 2013

1 check passed

Details default pr-checkin-per-commit Took 83 min.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.