SI-9239 fix java generic signature when traits extend classes #4403
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
Given the following code: class C1[T] trait T1[T] extends C1[T] class C2[T] extends T1[T] The generic signature of C2 changed after ced3ca8 from "C1[T], T1[T]" to "Object, T1[T]", even though C1 was still marked as a superclass in the bytecode... C1 was removed because a subclass (T1) appeared later on, and it was possible for a trait to cause a class to be evicted. It turns out that if a class A appears in "parents", it *always* has to stay in the signature: if a trait later in the list inherits from some class, that class has to be a superclass of A (per SLS §5.1), or A itself, so in any case, the most specific superclass is A, so A should stay in the signature. Thus minimizeParents now only allows traits/interfaces to be removed from the list (the refactoring in 7552739, moving from mixinClasses to parents, makes this much easier to fix). This didn't happen for non-generic signatures because there are two separate fields in the classfile for this (one for the superclass, one for interfaces), so interfaces were processed on their own. Thus non-parametrized classes were not affected. Anything that used erased types at runtime was also fine (like isInstanceOf checks), and anything that used ScalaSignature was also unaffected. Maybethat's why so few things broke... See the test for how this affects Java (hint: badly). This changes very few things when building scala itself, and the changes seem sensible: --- sandbox/lib/scala-library.jar#!scala/runtime/NonLocalReturnControl.class +++ build/pack/lib/scala-library.jar#!scala/runtime/NonLocalReturnControl.class - Generic Signature: <T:Ljava/lang/Object;>Ljava/lang/Object;Lscala/util/control/ControlThrowable; + Generic Signature: <T:Ljava/lang/Object;>Ljava/lang/Throwable;Lscala/util/control/ControlThrowable; --- sandbox/lib/scala-library.jar#!scala/collection/mutable/QueueProxy$$anon$1.class +++ build/pack/lib/scala-library.jar#!scala/collection/mutable/QueueProxy$$anon$1.class - Generic Signature: Ljava/lang/Object;Lscala/collection/mutable/QueueProxy<TA;>; + Generic Signature: Lscala/collection/mutable/Queue<TA;>;Lscala/collection/mutable/QueueProxy<TA;>; --- sandbox/lib/scala-library.jar#!scala/collection/mutable/Iterable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/mutable/Iterable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/mutable/Iterable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/mutable/Iterable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/mutable/Iterable;>; --- sandbox/lib/scala-library.jar#!scala/collection/immutable/Traversable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/immutable/Traversable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/immutable/Traversable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/immutable/Traversable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/immutable/Traversable;>; --- sandbox/lib/scala-library.jar#!scala/collection/immutable/Iterable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/immutable/Iterable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/immutable/Iterable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/immutable/Iterable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/immutable/Iterable;>; --- sandbox/lib/scala-library.jar#!scala/collection/mutable/Traversable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/mutable/Traversable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/mutable/Traversable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/mutable/Traversable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/mutable/Traversable;>; --- sandbox/lib/scala-library.jar#!scala/throws.class +++ build/pack/lib/scala-library.jar#!scala/throws.class - Generic Signature: <T:Ljava/lang/Throwable;>Ljava/lang/Object;Lscala/annotation/StaticAnnotation; + Generic Signature: <T:Ljava/lang/Throwable;>Lscala/annotation/Annotation;Lscala/annotation/StaticAnnotation; --- sandbox/lib/scala-library.jar#!scala/collection/mutable/StackProxy$$anon$1.class +++ build/pack/lib/scala-library.jar#!scala/collection/mutable/StackProxy$$anon$1.class - Generic Signature: Ljava/lang/Object;Lscala/collection/mutable/StackProxy<TA;>; + Generic Signature: Lscala/collection/mutable/Stack<TA;>;Lscala/collection/mutable/StackProxy<TA;>; --- sandbox/lib/scala-library.jar#!scala/collection/convert/Wrappers$IterableWrapper.class +++ build/pack/lib/scala-library.jar#!scala/collection/convert/Wrappers$IterableWrapper.class - Generic Signature: <A:Ljava/lang/Object;>Ljava/lang/Object;Lscala/collection/convert/Wrappers$IterableWrapperTrait<TA;>;Lscala/Product;Lscala/Serializable; + Generic Signature: <A:Ljava/lang/Object;>Ljava/util/AbstractCollection<TA;>;Lscala/collection/convert/Wrappers$IterableWrapperTrait<TA;>;Lscala/Product;Lscala/Serializable; --- sandbox/lib/scala-library.jar#!scala/collection/Iterable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/Iterable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/Iterable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/Iterable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/Iterable;>; --- sandbox/lib/scala-library.jar#!scala/collection/Traversable$.class +++ build/pack/lib/scala-library.jar#!scala/collection/Traversable$.class - Generic Signature: Ljava/lang/Object;Lscala/collection/generic/TraversableFactory<Lscala/collection/Traversable;>; + Generic Signature: Lscala/collection/generic/GenTraversableFactory<Lscala/collection/Traversable;>;Lscala/collection/generic/TraversableFactory<Lscala/collection/Traversable;>; --- sandbox/lib/scala-library.jar#!scala/collection/parallel/AdaptiveWorkStealingForkJoinTasks$WrappedTask.class +++ build/pack/lib/scala-library.jar#!scala/collection/parallel/AdaptiveWorkStealingForkJoinTasks$WrappedTask.class - Generic Signature: <R:Ljava/lang/Object;Tp:Ljava/lang/Object;>Ljava/lang/Object;Lscala/collection/parallel/ForkJoinTasks$WrappedTask<TR;TTp;>;Lscala/collection/parallel/AdaptiveWorkStealingTasks$WrappedTask<TR;TTp;>; + Generic Signature: <R:Ljava/lang/Object;Tp:Ljava/lang/Object;>Lscala/concurrent/forkjoin/RecursiveAction;Lscala/collection/parallel/ForkJoinTasks$WrappedTask<TR;TTp;>;Lscala/collection/parallel/AdaptiveWorkStealingTasks$WrappedTask<TR;TTp;>;
|
/rebuild |
|
I hit the "Rebuild" button in jenkins (following the Details link) because the bot didn't think it had to rebuild. |
|
tracked as scala/scabot#25 |
|
I'm on vacation this week, but will have a look asap when i get back. |
|
@adriaanm thanks, I wasn't sure what was going on... |
|
review by @retronym :) |
|
LGTM. Thanks for picking this up so quickly. |
|
Excellent work, @gourlaysama! |
adriaanm
added a commit
that referenced
this pull request
Apr 9, 2015
SI-9239 fix java generic signature when traits extend classes
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.
Given the following code:
The generic signature of
C2changed after ced3ca8 from"C1[T], T1[T]"to
"Object, T1[T]", even thoughC1was still marked as a superclass inthe bytecode...
C1was removed because a subclass (T1) appeared later on, and it waspossible for a trait to cause a class to be evicted. It turns out that
if a class
Aappears inparents, it always has to stay in theminimized signature: if a trait later in the list inherits from some class, that
class has to be a superclass of
A(per SLS §5.1), orAitself, so inany case, the most specific superclass is
A, soAshould stay in thesignature.
Thus
minimizeParentsnow only allows traits/interfaces to be removedfrom the list (the refactoring in 7552739, moving from
mixinClassestoparents, makes this much easier to fix).This didn't happen for non-generic signatures because there are two
separate fields in the classfile for this (one for the superclass,
one for interfaces), so interfaces were processed on their own.
Thus non-parametrized classes were not affected. Anything that used
erased types at runtime was also fine (like
isInstanceOfchecks),and anything that used
ScalaSignaturewas also unaffected.Maybe that's why so few things broke...
See the test for how this affects Java (hint: badly).
This changes very few things when building scala itself, and the
changes seem sensible: