Erasure vs generic signatures—The battle against SI-3452

190 views
Skip to first unread message

Paolo G. Giarrusso

unread,
May 28, 2012, 1:33:34 PM5/28/12
to scala-i...@googlegroups.com
Hi all,

I have some partial fixes for SI-3452 together with analysis of the other
remaining problems and some questions, and Paul Phillips urged me to discuss
them here rather than on JIRA. I don't know if I should introduce myself, since
I never posted here; you will find a minimal presentation at the end.

My patches are [on Github][Commits] and after discussion I expect to turn
(part of) them into a pull request. Interested reviewers are @odersky (value
classes), @magarciaEPFL and @gkossakowski (backend), and probably whoever
contributed to erasure and generic signatures.

The patches introduce no failure, except a timeout during partest (which seems
to happen only locally, not on the build bot, but there are too many differences
to be sure). It seems that all tests are compiled correctly---the error is quite
unspecific, hence I could not investigate it yet:

```
/Users/pgiarrusso/Documents/Research/Sorgenti/scala/build.xml:1953:
java.lang.RuntimeException: Test suite finished with 1 case failing:
worker timed out; adding failed test [TIMOUT]
```

I tried to ensure that one can read only part of this mail and still get the
gist. I also sectioned this email for readability, and chose to use Markdown
syntax for this, like on GitHub (please tell me if prefer other solutions).

# Problem statement
To solve issue SI-3452, JVM generic signatures (also simply _signatures_ in the
following) emitted by Scalac should always be coherent with method descriptors
in emitted bytecode; when a mismatch occurs for a method, that method cannot be
called by Java code, because `javac` computes the descriptor to call from the
generic signature, and a mismatch implies that `javac` in this way will produce
an incorrect descriptor.

There are various bugs causing mismatches; since I just started hacking the
compiler, I've fixed the low-hanging fruits. I've enabled `-Ycheck:jvm`, added
[Paul's patch][PaulFix], and investigated the remaining warnings and the
testcase failures introduced by the patch.

During this investigation I discovered that Scalac erases value classes to
Object (incorrectly?) unlike the generic signature algorithm; this seems to have
more general relevance, although it might be intended. More details and
other problems in next section.

# Bugs
1. Value classes (@odersky): the erasure algorithm erases them to `Object`, while in
signatures they are left as-is. It's not clear to me why they should be erased
`Object`, and such a behavior does not seem to be described in SIP 15. I also
tried STFW, without success. On the other hand, the same is done for primitive
value types, which is also confusing---I can only imagine reasons of binary
compatibility. If that's intended, I guess I could fix the algorithm computing
signatures.
2. `Nothing` vs `Nothing$`: who implemented prepareSigMap assumed that Nothing
is erased to Nothing$ during erasure, which sounds reasonable, while for some
unclear reason that's done during code generation. I'm not sure where to fix the
problem---I feel that the `Nothing`->`Nothing$` transformation belongs to
erasure, but I implemented a less invasive fix on `prepareSigMap` (which could
be simplified a bit further).
3. Intersection types are erased incorrectly by the generic signature algorithm,
implemented in `erasure.javaSig`. I and Paul could not yet agree on the correct
behavior.
I could easily [fix][2] `erasure.javaSig`, which computes the generic
signatures, to be coherent with the erasure algorithm specified by SLS 3.7
and implemented by Scalac.
However `erasure.erasure`, which is used during the check, still gives an
incorrect result, causing a spurious warning, and reading the code does not
reveal why, only that a fair amount of code duplication seems to be involved.
4. Paul's patch fixes the majority of the remaining problems, but introduces
some regression (discussed below). I identified an algorithm which should solve
the problem, but I am not sure how to implement it.
5. There's at least one further warning which I did not investigate yet.

Bugs 3 and 4 are discussed in more detail in following sections.

## How to develop on the compiler?
I could not investigate the runtime behavior since I have yet to setup Scalac
in some IDE with debugging support - trying Scala IDE 2.1-M1-2.10 resulted in
utter failure. I might try IntelliJ next but I would accept suggestions/docs.
I'm also aware of the nice [guide to reflection by Eugene
Burmako][ReflectionGuide], I just did not have yet time to investigate it.

# Intersection types.
In this section I explain the problem with intersection types and the solution I
implemented. I am confident that my solution is correct, but I could not yet
convince Paul.

Consider the following declarations:

```
abstract class A 
trait B extends A
class Test {
  def f[T](t: T) = new A with B { }
}
```

In this code, `f` will receive a generic signature incoherent with its
descriptor (the result of erasure); this generic signature will prevent Java
code from calling `f`.
According to SLS 3.7, the erasure of `B with A` is then defined to be `B`,
since `A` is filtered out as a supertype of `B` immediately.

To me it sounded clear that generic signatures should follow the same rule.
However, before I found the SLS rule, Paul had argued that in this example, `B
<: A` is not visible in Java, hence from the point of view of Java neither `B`
nor `A` is more specific, which is something I had missed. I did not understand
if he suggested a different resolution. Paul, what do you think?

# Paul's fix and the regressions it causes
Unrelated to intersection types, Paul [fixed][PaulFix] the two biggest problems
with the bug at hand, but its fix introduces regressions. I managed to analyze the
problem and propose a conceptual solution, but implementing it is not so easy
for me, so I thought I'd better propose the algorithm for now.

The visible regression is that according to `javac`, `AbstractFunction1` does not
implement all `Function1` methods because of errors in generic signatures, as
shown in the failing testcases [detailed by Paul][PaulFix]. That error however
involves specialized signatures, hence it is more instructive to verify that
`javac` refuses code such as:

```
public static Traversable<A> trav = new scala.collection.AbstractTraversable<A>() {};
```

for the wrong reason. It complains, for instance, that `AbstractTraversable`
does not implement `seq: Traversable<A>`, which is abstract in the `Traversable`
interface. Instead, AbstractTraversable implements `seq: Traversable<Object>`,
which is incompatible since JVM generics are invariant.
`AbstractTraversable` still omit the implementation of foreach, but that's
irrelevant here.

Consider the following code (originally from Paul, altered a bit):

```
trait C[T]
trait Search2[M] {
  def search(input: M): C[Int] = null
  def filter(input: M => M): C[Int] = null
  def crazy(i1: M, i2: M => M): C[Int] = null
}
object StringSearch2 extends Search2[String]
```

When a class/object (like StringSearch) extends a mixin, the mixin composition
phase must create forwarder methods and assign them a descriptor and a generic
signature. The generic signature must however erase to the descriptor, which in
turn must match the descriptor in the superclass.
The problem arises when a type parameter of the mixin has a concrete value in
the superclass---can we simply replace the occurrences of the parameter with its
value? Not always. If we did so for `search`, it would become `search(String):
C[Object]`, which does not match the descriptor in `Search2`, that is `search(Object)`.
The algorithm computing descriptors already gets this right, but not the one for
generic signatures.

Naked occurrences of type parameters, like the one in `search`, erase to
`Object` in descriptors, and this must be preserved: they are either left as-is,
or erase to Object. Paul's change ensures this is done, even though I don't get
how. It seems that in fact his change erases all type parameters to `Object`;
while I can understand the description of his change and of `asSeenFrom` (the
method he uses), why that makes a difference is quite non-obvious to me.

For non-naked occurrences of type parameters, like the one in `filter`, we have
the opposite problem. On the one hand, they simply disappear from descriptors,
hence there is no need to erase them to `Object` if a value is substituted as
done by Paul's change. Moreover, this change is in fact harmful: for `javac`, a
method overrides another if its descriptor _and_ its generic signature are at
least as specific; moreover, type constructors are invariant for the JVM, so
their parameters must match.
Hence we have a constraint opposite to naked occurrences, and must preserve the
old behavior. Moreover, we can't reuse code from the erasure algorithm since it
does not need to handle this case.

Method `crazy` simply shows that the two cases might appear in the same descriptor.

To solve this problem, it seems necessary to create generic signatures by
substituting type arguments in Scala type signatures (before erasure) according
to the ideas outlined above---substitution would need to distinguish naked and
non-naked occurrences of type parameters. But I'd like to start from an existing
implementation of type substitution, given the amount of cases to handle, and given
that name capture seems a potential problem; I suspect it might be possible to
start from some variant of `SubstMap`/`TypeMap`.

In this code, BTW, I find confusing that `C[Int]` erases to `C[Object]` instead
of the more precise `C[java.lang.Integer]`---although that's the same for derived value
classes (and also there it's confusing). This is also not documented in SLS 3.7.
I can only think of compatibility reasons (I think that boxing used different
wrappers before 2.8, but I can't find any more where I read it).

# Who I am
I use Scala for research because I am a PhD student under the supervision of
Klaus Ostermann. I've experienced and reported various Scalac/SBT bugs during my
research activity (which is not about Scalac stability, but on developing DSELs).
I have previous experience in Open Source development---among other things, as a
Linux kernel maintainer of a peripheral subsystem (UserModeLinux). My homepage
is available [here][Home].


Jason Zaugg

unread,
May 28, 2012, 1:58:42 PM5/28/12
to scala-i...@googlegroups.com
On Mon, May 28, 2012 at 7:33 PM, Paolo G. Giarrusso
<p.gia...@gmail.com> wrote:

> ## How to develop on the compiler?
> I could not investigate the runtime behavior since I have yet to setup
> Scalac
> in some IDE with debugging support - trying Scala IDE 2.1-M1-2.10 resulted
> in
> utter failure. I might try IntelliJ next but I would accept
> suggestions/docs.
> I'm also aware of the nice [guide to reflection by Eugene
> Burmako][ReflectionGuide], I just did not have yet time to investigate it.

To use IntelliJ to debug scalac.

1. make copies of ./src/intellij/*.SAMPLE to the same directory with the
suffix removed. If you haven't already, copy ./gitignore.SAMPLE to
./gitignore.
2. install IntelliJ.
3. install the Scala Plugin, from the plugin manager within IntelliJ.
4. open the scala-lang.ipr file
5. check you have a Java SDK configured. File, Project Settings, Project,
Project SDK.
6. The cake pattern of the compiler triggers a few bugs in the plugin,
which can spin the CPU pretty hard. Not what you want in summer.
Enable "power save mode", on the inspection profile icon near the
bottom right of screen, to make this tolerable.
7. Setup a debug run configuration. Run, Edit Configurations, +, Remote,
Port = 5005.
8. From the command line:
> JAVA_OPTS="-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,
address=5005" ./build/quick/bin/scala test.scala

The JVM will wait for a debugger to connect.
9. From IntelliJ, select the debug run config, and hit F9, and voila!

You can set breakpoints and, if you're lucky, evaluate expressions [1].

Let me know if you have any problems.

-jason

[1] http://blog.jetbrains.com/scala/2011/11/14/evaluate-expression-feature-for-scala/

Hubert Plociniczak

unread,
May 28, 2012, 2:10:23 PM5/28/12
to scala-i...@googlegroups.com
On 05/28/2012 07:33 PM, Paolo G. Giarrusso wrote:
>
> ## How to develop on the compiler?
> I could not investigate the runtime behavior since I have yet to setup
> Scalac
> in some IDE with debugging support - trying Scala IDE 2.1-M1-2.10
> resulted in
> utter failure. I might try IntelliJ next but I would accept
> suggestions/docs.

When working on the compiler, your version of the Scala IDE has to be as
close to the version of the compiler as possible. The greater the
difference the bigger the chance that something will go wrong (M1 is
pretty old vs trunk).
I use the debugger on the compiler almost every day, works like a charm.
I suggest you describe your problem in detail on the IDE mailing list,
they will help you there quickly.

hubert

Paul Phillips

unread,
May 28, 2012, 6:13:53 PM5/28/12
to scala-i...@googlegroups.com
I will try to look at this in more detail, but one thing jumped out:

On Mon, May 28, 2012 at 10:33 AM, Paolo G. Giarrusso <p.gia...@gmail.com> wrote:
> ```
> public static Traversable<A> trav = new scala.collection.AbstractTraversable<A>() {};
> ```
> for the wrong reason. It complains, for instance, that
> `AbstractTraversable` does not implement `seq: Traversable<A>`,
> which is abstract in the `Traversable` interface. Instead,
> AbstractTraversable implements `seq: Traversable<Object>`, which is
> incompatible since JVM generics are invariant.

You are absolutely right, we are generating the wrong signature every time a covariant type appears.

  public abstract scala.collection.Traversable<A> seq();

This should be

  public abstract scala.collection.Traversable<? extends A> seq();

We have to migrate our definition-site variance to all the use sites.  I believe that fixing this is going to help the situation a lot.

Paul Phillips

unread,
May 28, 2012, 6:28:17 PM5/28/12
to scala-i...@googlegroups.com
On Mon, May 28, 2012 at 10:33 AM, Paolo G. Giarrusso <p.gia...@gmail.com> wrote:
In this code, BTW, I find confusing that `C[Int]` erases to `C[Object]` instead
of the more precise `C[java.lang.Integer]`---although that's the same for derived value
classes (and also there it's confusing). This is also not documented in SLS 3.7.
I can only think of compatibility reasons (I think that boxing used different
wrappers before 2.8, but I can't find any more where I read it).

The reason is that valid usage of generic signatures (valid in the sense of "conforms to JLS" and "does not cause random breakage in tools like eclipse") excludes doing anything non-trivial with primitive types.  You can't parameterize on a primitive in java; substituting the boxed type for the primitive, which is what was done in scala before 2.9, leads to inconsistent signatures when the type also appears in another position (imagine e.g. def f(xs: List[T]): T after being parameterized on Int.) This change displeased many people, understandably so, and the loss of that information was one of the leading motivations for a scala reflection library.

You can find examples, and answers to some of your other questions as well, by perusing the many closed signature tickets:


Here's one which you might find interesting:


Paolo Giarrusso

unread,
May 28, 2012, 7:02:02 PM5/28/12
to scala-i...@googlegroups.com
After your message I just tried the nightly and it's working
wonderfully, including debugging :-).
I hadn't tried reporting the bug because Eclipse became totally
unusable right after opening scalac source.
Thanks a lot for the prompt answer!
--
Paolo Giarrusso - Ph.D. Student
http://www.informatik.uni-marburg.de/~pgiarrusso/

Paolo Giarrusso

unread,
May 28, 2012, 7:57:01 PM5/28/12
to scala-i...@googlegroups.com
On Tue, May 29, 2012 at 12:13 AM, Paul Phillips <pa...@improving.org> wrote:
> I will try to look at this in more detail, but one thing jumped out:
>
> On Mon, May 28, 2012 at 10:33 AM, Paolo G. Giarrusso <p.gia...@gmail.com>
> wrote:
>> ```
>> public static Traversable<A> trav = new
>> scala.collection.AbstractTraversable<A>() {};
>> ```
>> for the wrong reason. It complains, for instance, that
>> `AbstractTraversable` does not implement `seq: Traversable<A>`,
>> which is abstract in the `Traversable` interface. Instead,
>> AbstractTraversable implements `seq: Traversable<Object>`, which is
>> incompatible since JVM generics are invariant.
>
> You are absolutely right, we are generating the wrong signature every time a
> covariant type appears.
Don't you mean variant, that is, covariant or contravariant?

>   public abstract scala.collection.Traversable<A> seq();
>
> This should be
>
>   public abstract scala.collection.Traversable<? extends A> seq();
>
> We have to migrate our definition-site variance to all the use sites.  I
> believe that fixing this is going to help the situation a lot.

What you say makes a lot of sense, but I'm not sure that this won't
hide the particular problem at hand.
`AbstractTraversable` should still _not_ declare `seq` as returning
`Traversable<Object>`, also (but not only) because
`Traversable<Object>` does not conform to `Traversable<? extends A>`.

I had overlooked one thing however: with the scheme you propose,
`AbstractTraversable.seq` would probably end up returning
`Traversable<? extends Object>` (i.e. `Traversable<?>`). However, this
is a supertype of `Traversable<? extends A>`, so
`AbstractTraversable.seq` would still not override `Traversable.seq` -
we'd need a subtype. In the argument position, however, it seems that
your change would hide the bug.

Still, I'd like that we ended up with an updated spec for SLS 3.7 and
with a spec for generic signatures, just to be able to write it at
all. Should it be in the SLS? I'd say yes, if possible, since it'd be
useful when writing Java-Scala projects.

To give you an idea of why that might be useful, there are crazy
people (like me) who end up trying to access a cake-pattern-based API
from Java for [TypeChef][1]. Of course I gave up and didn't even
bother to report the [scalac bug I found][2], since it's clearly
beyond what one could expect. I'd summarize that problem as 'scalac is
not "bug-to-bug" compatible to javac when parsing Java code'.

[1]: https://github.com/ckaestne/TypeChef/
[2]: https://github.com/ckaestne/TypeChef/commit/d3c592054b69fdea73e71c7fc4b43a7db2fcb858

Paolo Giarrusso

unread,
May 28, 2012, 10:37:13 PM5/28/12
to scala-i...@googlegroups.com
One extra bugfix:

On Mon, May 28, 2012 at 7:33 PM, Paolo G. Giarrusso
<p.gia...@gmail.com> wrote:
> 3. Intersection types are erased incorrectly by the generic signature
> algorithm,
> implemented in `erasure.javaSig`. I and Paul could not yet agree on the
> correct
> behavior.

> I could easily [fix][2] `erasure.javaSig`, which computes the generic
> signatures, to be coherent with the erasure algorithm specified by SLS 3.7
> and implemented by Scalac.

> However `erasure.erasure`, which is used during the check, still gives an
> incorrect result, causing a spurious warning, and reading the code does not
> reveal why, only that a fair amount of code duplication seems to be
> involved.

It seems that this problem was easy to solve, with a debugger at hand.

What I missed is that (apparently) after erasure, traits stop
inheriting from classes. Hence I now run erasure under beforeErasure:

- val erasedNormalizedType = erasure.erasure(sym)(normalizedTpe)
+ val erasedNormalizedType =
beforeErasure(erasure.erasure(sym)(normalizedTpe))

Moreover, this is already done in all the other code which needs to do
something similar, including erasure.javaSig.

I hope this change will not have other unintended consequences.
Currently this change appears to work exactly as expected, and fix all
the warnings that I expected it would fix (even though tests are still
running) - please let me know if you see something wrong. I pushed
this change on GitHub already.

Cheers,

Paul Phillips

unread,
May 29, 2012, 2:22:24 AM5/29/12
to scala-i...@googlegroups.com
On Mon, May 28, 2012 at 4:57 PM, Paolo Giarrusso <pgiar...@mathematik.uni-marburg.de> wrote:
`AbstractTraversable` should still _not_ declare `seq` as returning
`Traversable<Object>`, also (but not only) because
`Traversable<Object>` does not conform to `Traversable<? extends A>`.

I am missing some piece of information.  AbstractTraversable doesn't declare `seq` as returning Traversable<Object>, it returns Traversable<A>.  At least on my machine it does.  Where are you seeing Traversable<Object> ?

To give you an idea of why that might be useful, there are crazy
people (like me) who end up trying to access a cake-pattern-based API
from Java for [TypeChef][1].

Don't miss this one then:

 
 

Paul Phillips

unread,
May 29, 2012, 2:39:15 AM5/29/12
to scala-i...@googlegroups.com
Oh, and since the database was mangled apparently beyond repair when the tickets were moved into jira, you have to translate each "$$" into "$" if you want to know what that code looked like when I wrote it.  I love having to append this explanation everytime I reference one of the thousand or so tickets in which I used a dollar sign.

Paolo Giarrusso

unread,
May 29, 2012, 4:58:29 AM5/29/12
to scala-i...@googlegroups.com
On Tue, May 29, 2012 at 8:22 AM, Paul Phillips <pa...@improving.org> wrote:
>
>
> On Mon, May 28, 2012 at 4:57 PM, Paolo Giarrusso
> <pgiar...@mathematik.uni-marburg.de> wrote:
>>
>> `AbstractTraversable` should still _not_ declare `seq` as returning
>> `Traversable<Object>`, also (but not only) because
>> `Traversable<Object>` does not conform to `Traversable<? extends A>`.
>
>
> I am missing some piece of information.  AbstractTraversable doesn't declare
> `seq` as returning Traversable<Object>, it returns Traversable<A>.  At least
> on my machine it does.  Where are you seeing Traversable<Object> ?

Well, that's the result of your partial fix for SI-3452, and that's
why your fix introduces regressions :-). It's also said in the piece
of mail you first quoted.

If that doesn't ring a bell yet, a kind-of-brief-but-not-really summary follows.

The whole section "Paul's fix and the regressions it causes" is about
that patch (and has the link to it). In that section, I also explain
in (more than) enough detail what's wrong with the produced generic
signatures. In my first mail, I explain it on _this_ example:

```
trait C[T]
trait Search2[M] {
def search(input: M): C[Int] = null //M in naked position
def filter(input: M => M): C[Int] = null //M in non-naked position!
def crazy(i1: M, i2: M => M): C[Int] = null //both cases together.
}
object StringSearch2 extends Search2[String]
```

BUT there's _one_ thing I did not explain fully there:
`AbstractTraversable.seq` has the same problem as
`StringSearch2.filter` - the type parameter is erased to `Object` even
though it is in a non-naked position (see the first mail to know why
this is bad and why nakedness is important while variance is
irrelevant).
You might notice that `StringSearch2` substitutes a type argument,
while `Search2` doesn't; unlike what you'd expect, it makes no
difference here for your code.

>> To give you an idea of why that might be useful, there are crazy
>> people (like me) who end up trying to access a cake-pattern-based API
>> from Java for [TypeChef][1].
>
>
> Don't miss this one then:
>
>   https://issues.scala-lang.org/browse/SI-4389

Thanks for the link. But my code was much closer to working, abstract
type parameters are better supported and I didn't use variance.
Importing members from a Scala object in Java sources is much less
fun.
Maybe I'll manage to reduce my problem some day.

Paul Phillips

unread,
May 29, 2012, 9:11:03 AM5/29/12
to scala-i...@googlegroups.com


On Tue, May 29, 2012 at 1:58 AM, Paolo Giarrusso <pgiar...@mathematik.uni-marburg.de> wrote:
Well, that's the result of your partial fix for SI-3452, and that's
why your fix introduces regressions :-).

No, it isn't.  That commit isn't even in trunk.  Please just answer the question if you want my input.  Where are you seeing AbstractTraversable seq() declaring a return type of Traversable<Object> and not Traversable<A> ?

Here, for instance, is where I do not see it.

% javap -s scala.collection.AbstractTraversable

public abstract class scala.collection.AbstractTraversable<A> implements scala.collection.Traversable<A> {
  ...
  public scala.collection.Traversable<A> seq();
    Signature: ()Lscala/collection/Traversable;
  ...
}

Paolo Giarrusso

unread,
May 29, 2012, 11:21:00 PM5/29/12
to scala-i...@googlegroups.com
On Tue, May 29, 2012 at 3:11 PM, Paul Phillips <pa...@improving.org> wrote:
>
>
> On Tue, May 29, 2012 at 1:58 AM, Paolo Giarrusso
> <pgiar...@mathematik.uni-marburg.de> wrote:
>>
>> Well, that's the result of your partial fix for SI-3452, and that's
>> why your fix introduces regressions :-).
>
>
> No, it isn't.  That commit isn't even in trunk.  Please just answer the
> question if you want my input. Where are you seeing AbstractTraversable
> seq() declaring a return type of Traversable<Object> and not Traversable<A>
> ?

I'm sorry but there's just a misunderstanding, and I even see where I
was ambiguous.

The short answer is: I see that if and only if I _explicitly_ include
your commit by merging it onto my local branch. Why would I ever need
that to be in "trunk" to test it? A distributed VCS like Git makes
that totally unnecessary.

The longer answer (with data for reproducibility) follows. After that,
I reconnect this change (in AbstractTraversable.seq's signature) to
the test regressions.

# Answer to your question
I see that result on this branch, which includes your commit which I rebased:
https://github.com/Blaisorblade/scala/commits/issue/3452
[ WARNING, that's a rebase branch for now, so I might still rebase
that branch at will and do non-fast-forward pushed, unless you ask me
not to ]

and I don't see it on my master branch, where the only difference is
your commit (and an additional Scaladoc comment). See a branch
comparison here:
https://github.com/Blaisorblade/scala/compare/master...issue%2F3452

Tool output:

```
$ /Library/Java/JavaVirtualMachines/JDK\ 1.7.0\ Developer\
Preview.jdk/Contents/Home/bin/javap -s -classpath
dists/scala-2.10.0-20120529-021150-931a48f728/lib/scala-library.jar
scala.collection.AbstractTraversable

public abstract class scala.collection.AbstractTraversable<A>
implements scala.collection.Traversable<A> {
...
public scala.collection.Traversable<java.lang.Object> seq();
Signature: ()Lscala/collection/Traversable;
...
```

(I used to use javap -verbose and decode it by hand, by javap -s on
Java 1.7 is much better, thanks! On Java 1.6 it just doesn't work).

Did I miss something?

I never mentioned nor meant trunk; I only said that your fix
(https://github.com/paulp/scala/commit/11c11f0efc) introduces a
regression, and the URL is from your repo, not from "trunk", otherwise
I would and should have given a link to scala/scala. I'm sorry if I
was ambiguous, and more sorry if that was offensive; I guess what I
said can be understood differently when not using a DVCS like Git. How
can I avoid such misunderstandings in the future?

# Connection to the regressions
The cause for the regressions on testcases when explicitly including
your commit is the same behavior as above but on a different method,
`AbstractFunction1.andThen$mcVL$sp`, which stops matching
`Function1.andThen$mcVL$sp`:

```
public abstract class scala.runtime.AbstractFunction1<T1, R>
implements scala.Function1<T1, R> {
...
public <A extends java/lang/Object>
scala.Function1<java.lang.Object, A>
andThen$mcVL$sp(scala.Function1<scala.runtime.BoxedUnit, A>);
Signature: (Lscala/Function1;)Lscala/Function1;
...
}
```

Notice how this differs from the following, because `T1` was replaced
by `Object`:

```
public interface scala.Function1<T1, R> {
public abstract <A extends java/lang/Object> scala.Function1<T1, A>
andThen$mcVL$sp(scala.Function1<scala.runtime.BoxedUnit, A>);
Signature: (Lscala/Function1;)Lscala/Function1;
}
```

That explains javac's compiler error - this mismatch means that the
new method does not override the old one:

[partest] /Users/pgiarrusso/Documents/Research/Sorgenti/scala/test/files/pos/javaReadsSigs/fromjava.java:34:
<anonymous fromjava$1> is not abstract and does not override abstract
method <A>andThen$mcVL$sp(scala.Function1<scala.runtime.BoxedUnit,A>)
in scala.Function1
[partest] public static Function1<A, B> f1 = new
scala.runtime.AbstractFunction1<A, B>() {

BTW, I suspect another bug here - why are there so many versions of
andThen if it is marked with @annotation.unspecialized? I guess
however this is well known.

From src/library/scala/Function1.scala:
@annotation.unspecialized def andThen[A](g: R => A): T1 => A = { x =>
g(apply(x)) }

`AbstractTraversable.seq` is IMHO a better example for this bug
because specialization is not involved.

My original mail shows a minimal example, except that to the code
snippet I present:

```
trait C[T]
trait Search2[M] {
def search(input: M): C[Int] = null
def filter(input: M => M): C[Int] = null
def crazy(i1: M, i2: M => M): C[Int] = null
}
object StringSearch2 extends Search2[String]
```

I would now add `abstract class GenSearch2[M] extends Search2[M]`,
which demonstrates the regression better. Below is the `javap -s`
output after compiling on a tree including your commit---notice how
`M` is replaced by `Object` even in `GenSearch2`, but not in
`Search2`.

public interface Search2<M> {
public abstract C<java.lang.Object> search(M);
Signature: (Ljava/lang/Object;)LC;

public abstract C<java.lang.Object> filter(scala.Function1<M, M>);
Signature: (Lscala/Function1;)LC;

public abstract C<java.lang.Object> crazy(M, scala.Function1<M, M>);
Signature: (Ljava/lang/Object;Lscala/Function1;)LC;
}
public abstract class GenSearch2<M> implements Search2<M> {
public C<java.lang.Object> search(java.lang.Object);
Signature: (Ljava/lang/Object;)LC;

public C<java.lang.Object> filter(scala.Function1<java.lang.Object,
java.lang.Object>);
Signature: (Lscala/Function1;)LC;

public C<java.lang.Object> crazy(java.lang.Object,
scala.Function1<java.lang.Object, java.lang.Object>);
Signature: (Ljava/lang/Object;Lscala/Function1;)LC;

public GenSearch2();
Signature: ()V
Reply all
Reply to author
Forward
0 new messages