Skip to content
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

incorrect bytecode generated for inner objects #1572

Closed
scabug opened this issue Dec 8, 2008 · 16 comments
Closed

incorrect bytecode generated for inner objects #1572

scabug opened this issue Dec 8, 2008 · 16 comments
Assignees

Comments

@scabug
Copy link

scabug commented Dec 8, 2008

Given the following test code:

trait a {
  object foo {
    def bar = List(1,2).map(_*2)
  }
}

trait b {
  object foo {
    def bar = List(1,2).map(_*2)
  }
  class foo { }
}

...the generated bytecode for a$$foo$$ contains references to the non-existent a$$foo, as does the anon function generated for the map. In the case of b (which defines a class of the same name as the object) the references incorrectly point to the class instead of the object. This can be seen by way of reflection:

scala> Class.forName("a$$foo$$").getDeclaredClasses
res0: Array[java.lang.Class[_]] = Array()
scala> Class.forName("a$$foo").getDeclaredClasses
java.lang.ClassNotFoundException: a$$foo

scala> Class.forName("b$$foo$$").getDeclaredClasses
res1: Array[java.lang.Class[_]] = Array()
scala> Class.forName("b$$foo").getDeclaredClasses 
res2: Array[java.lang.Class[_]] = Array(class b$$foo$$$$anonfun$$bar$$2)

I came across this attempting to use proguard on the scala jars. It objects to processing them because of inconsistent InnerClasses attributes due to the above issue - for instance there are references to the non-existent class scala/tools/nsc/symtab/Definitions$$definitions .

@scabug
Copy link
Author

scabug commented Dec 8, 2008

Imported From: https://issues.scala-lang.org/browse/SI-1572?orig=1
Reporter: @paulp
Attachments:

@scabug
Copy link
Author

scabug commented Dec 8, 2008

@paulp said:
Sorry, I should better have demonstrated the other direction.

scala> Class.forName("a$$foo$$$$anonfun$$bar$$1").getDeclaringClass
java.lang.NoClassDefFoundError: a$$foo
	at java.lang.Class.getDeclaringClass(Native Method)
scala> Class.forName("b$$foo$$$$anonfun$$bar$$2").getDeclaringClass
res2: java.lang.Class[_] = class b$$foo

@scabug
Copy link
Author

scabug commented Dec 8, 2008

@paulp said:
In a related issue which also saddens proguard, there are references to scala.Nothing and scala.Null in the bytecode, which I believe are all supposed to be transformed to scala.Nothing$$ and .Null$$. I am attaching a file listing the ones it found in the 2.7.2 final jars.

@scabug
Copy link
Author

scabug commented Jan 8, 2009

Lalit Pant (lpant) said:
Here is some more information on this issue, to add to the information already provided:

I see two problems here in the generated class-files:

  • Incorrect !InnerClasses attributes
  • Incorrect Method Signature attributes involving Scala.Nothing and Scala.Null

More details below:

== Problem 1 - Incorrect !InnerClasses attributes ==
This code demonstrates the problem:

object OneLevelOuter {
    def bar = List(1,2).map(_*2)
}

object TwoLevelOuter {
  object Outer {
    def bar = List(1,2).map(_*2)
  }
}  

Here, !OneLevelOuter$$.class has an !InnerClasses attribute with the following information:[[BR]]
inner_class_info: !OneLevelOuter$$$$anonfun$$bar$$1[[BR]]
outer_class_info: !OneLevelOuter. This looks incorrect. It should be: !OneLevelOuter$$. In this case, !OneLevelOuter actually does exist, so outer_class_info points to the wrong class.

!TwoLevelOuter$$Outer$$.class has an !InnerClasses attribute with the following information:[[BR]]
inner_class_info: !TwoLevelOuter$$Outer$$$$anonfun$$bar$$2[[BR]]
outer_class_info: !TwoLevelOuter$$Outer. This looks incorrect. It should be: !TwoLevelOuter$$Outer$$. In this case, !TwoLevelOuter$$Outer does not exist, so outer_class_info points to a non-existant class.

== Problem 2 - Incorrect Method Signature attributes involving Scala.Nothing and Scala.Null ==

This code demonstrates the problem:

object MethodSig {
  val Empty = new collection.immutable.Stack[Nothing]
  def m1: Nothing = {
    throw new RuntimeException()
  }
}

This translates to the following in the classfile for !MethodSig$$:

A method called Empty with:[[BR]]
Descriptor: ()Lscala/collection/immutable/Stack;[[BR]]
Signature: ()Lscala/collection/immutable/Stack<Lscala/Nothing;>;[[BR]]

A method called m1 with:[[BR]]
Descriptor: ()Lscala/runtime/Nothing$$;[[BR]]
Signature: none, because this is not a generic method.

As you can see, Nothing is referenced correctly in the Descriptor for m1, but not in the Signature for Empty.

@scabug
Copy link
Author

scabug commented Feb 20, 2009

@SethTisue said:
tiny jar file you can pass to ProGuard to shut it up about Nothing & Null

@scabug
Copy link
Author

scabug commented Feb 20, 2009

@SethTisue said:
I hope the jar I just attached helps someone. This is the workaround I am using in my own project. You just add it to the libraryjars (not the injars!) you pass ProGuard. It only makes the Nothing/Null errors go away; it doesn't do anything about the other stuff (the inner class stuff). The only thing I know to do about the inner class thing with ProGuard is use -ignorewarnings.

@scabug
Copy link
Author

scabug commented Mar 28, 2009

@dragos said:
This is a thorny issue. The problem is that top level objects may generate two classfiles: one for the object itself, which ends in '$$', and a 'mirror' for Java compatibility, having the real name and containing static forwarders. The question is which one is the proper outer class for a given inner class? Right now, the outer is set to be the mirror, as it allows Java code to use the same names as Scala when referring to inner classes.

If I am not mistaken, InnerClasses attributes have to be consistent across a JVM, so the same class cannot appear as having different outer classes in different class files.

I don't see a good solution to this, so I am open to suggestions.

@scabug
Copy link
Author

scabug commented Mar 30, 2009

@dragos said:
The Nothing and Null references have been fixed in r17396. I leave the ticket open for the inner classes issue.

@scabug
Copy link
Author

scabug commented Mar 31, 2009

@paulp said:
Replying to [comment:8 dragos]:

If I am not mistaken, InnerClasses attributes have to be consistent across a JVM, so the same class cannot appear as having different outer classes in different class files.

Not that I think pursuing inconsistency is the way to go, but the spec says "The Java virtual machine does not currently check the consistency of the InnerClasses attribute with any class file actually representing a class or interface referenced by the attribute."

http://java.sun.com/docs/books/jvms/second_edition/html/ClassFile.doc.htmlSI-79996

Although I'm not sure it and you are talking about the same thing.

With respect to this issue, I would say anytime "using scala from java" comes into conflict with other reasonable goals, "using scala from java" should lose out, especially if it's just "using scala from java with pretty names." (I do think "using java from scala" should be a high priority.)

@scabug
Copy link
Author

scabug commented Mar 31, 2009

@dragos said:
Replying to [comment:10 extempore]:

Replying to [comment:8 dragos]:

If I am not mistaken, InnerClasses attributes have to be consistent across a JVM, so the same class cannot appear as having different outer classes in different class files.

Not that I think pursuing inconsistency is the way to go, but the spec says "The Java virtual machine does not currently check the consistency of the InnerClasses attribute with any class file actually representing a class or interface referenced by the attribute."

I vaguely remember having tried to set the outer class to be the current class, so Outer for the mirror class, and Outer$$ for the module class, and getting runtime exceptions on the JVM. I don't think the spec talks about the same thing. I understand it as the outer name might not exist as a class on the class path. What I was hinting at, is having two different, loaded and linked classes, Outer and Outer$$, which have different outer entries for the same inner class. Maybe it was this bug: #1167.

I think this is quite easy to hack, but I am a bit short on time until next week. If someone here is ready to do it, I can point out relevant pieces of code.

With respect to this issue, I would say anytime "using scala from java" comes into conflict with other reasonable goals, "using scala from java" should lose out, especially if it's just "using scala from java with pretty names." (I do think "using java from scala" should be a high priority.)

Agreed. So you would make inner classes member of the module class, and live with '$$' names in Java? That was the way it worked some time ago, then it was 'fixed' to support nicer names for inner classes. I don't remember all the arguments, but we should check at least #1126 before doing the change.

@scabug
Copy link
Author

scabug commented Mar 31, 2009

@paulp said:
Even for top level objects, forwarders are already installed only on a "best effort" basis, right? There's no getting around the need to know ugly details for anyone set on using scala from java. So yes, I would tentatively vote that scala inner classes should not profess knowledge of the mirror class. I see the implications need exploring, but I'm trying really hard not to get knocked too far off track on my parser work. I will come back to this when I can.

@scabug
Copy link
Author

scabug commented Aug 31, 2009

@dragos said:
I change the way InnerClasses are reported for inner objects, but kept the old scheme for top level objects. That should not be a problem: in the case when there's a companion class, the reference is correct. If there's no companion, that the compiler generates a mirror class, and again the reference is 'bound'. I think this way we get the best of both worlds.

I had to fix another problem with the way java signatures were generated, but now proguard is happy with the scala compiler bytecode, except for two warnings about reflective calls in the interpreter.

The fix is in r18618.

@scabug
Copy link
Author

scabug commented Aug 31, 2009

@SethTisue said:
I'm overjoyed!! Thank you.

@scabug
Copy link
Author

scabug commented Mar 2, 2010

Paul LaCrosse (pjlacrosse) said:
Replying to [comment:13 dragos]:

I change the way InnerClasses are reported for inner objects, but kept the old scheme for top level objects. That should not be a problem: in the case when there's a companion class, the reference is correct. If there's no companion, that the compiler generates a mirror class, and again the reference is 'bound'. I think this way we get the best of both worlds.

I had to fix another problem with the way java signatures were generated, but now proguard is happy with the scala compiler bytecode, except for two warnings about reflective calls in the interpreter.

The fix is in r18618.
While I've been using ProGuard 4.5Beta3 with no problems with Scala 2.8Beta1, switching to recent nightly builds of Scala 2.8 (re)introduces the problems of bytecode references to non-existant classes.

It is not limited to the Scala library, but shows up for user classes:

[proguard] Warning: com.test.app.abc.data.QueryDtl$$: can't find referenced class com.test.app.abc.data.QueryDtl$$$$anonfun$$apply$$1

@scabug
Copy link
Author

scabug commented Mar 2, 2010

@paulp said:
Replying to [comment:16 pjlacrosse]:

It is not limited to the Scala library, but shows up for user classes:

[proguard] Warning: com.test.app.abc.data.QueryDtl$$: can't find referenced class com.test.app.abc.data.QueryDtl$$$$anonfun$$apply$$1

Please supply us with something more concrete than an error message if you want this to have any chance of being investigated. It's quite frustrating when people expend so little effort making a report.

@scabug
Copy link
Author

scabug commented Mar 3, 2010

Paul LaCrosse (pjlacrosse) said:
Ok, I will only report errors where I've had the time to fully investigate how to create a minimal failing sample. Otherwise, I will keep the failures secret and let other people discover them on their own.

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

No branches or pull requests

2 participants