Irregular class-files generated for InnerClasses #2749

scabug · 2009-12-02T07:04:56Z

This is a follow-up to #1167 which was a problem with the !InnerClass attribute of anonymous classes which resulted in runtime exceptions. Whereas this particular problem was fixed, the generated class-files do still not comply to the JVM spec.

Take again this example (compiled with scala-2.8.0.r19928-b20091129020233):

trait Test {
 def testFunc(i:Int) = ((i:Int) => i + 5)(i)
}

The compiler generates three classes Test, Test$$class and Test$$$$anonfun$$testFunc$$1.

Here are the relevant parts from the javap -v outputs:

% javap Test -v
Compiled from "test.scala"
public interface Test extends scala.ScalaObject
  SourceFile: "test.scala"
//...
  InnerClass: 
   public final SI-11= SI-8 of SI-10; //$$anonfun$$testFunc$$1=class Test$$$$anonfun$$testFunc$$1 of class Test
//...

% javap Test\$$class -v
Compiled from "test.scala"
public abstract class Test$$class extends java.lang.Object
  SourceFile: "test.scala"
  Scala: length = 0x
   
  InnerClass: 
   public final SI-39= SI-13 of SI-38; //$$anonfun$$testFunc$$1=class Test$$$$anonfun$$testFunc$$1 of class Test

% javap Test\$$\$$anonfun\$$testFunc\$$1 -v
Compiled from "test.scala"
public final class Test$$$$anonfun$$testFunc$$1 extends java.lang.Object implements scala.Function1,scala.ScalaObject,java.io.Serializable
  SourceFile: "test.scala"
  Scala: length = 0x
   
  InnerClass: 
   public final SI-68= SI-9 of SI-67; //$$anonfun$$testFunc$$1=class Test$$$$anonfun$$testFunc$$1 of class Test

#1167 was about Test and Test$$$$anonfun$$testFunc$$1 inconsistently reporting the outer class. This was obviously fixed. But there still remain several issues:

The inner class should really belong to Test$$class because that contains the implementation.
Test$$class and Test both contain the !InnerClass attribute
The !InnerClass attribute for anonymous classes is handled differently than done here. From [http://java.sun.com/docs/books/jvms/second_edition/html/ClassFile.doc.htmlSI-79996 JVM Spec �4.7.5]:

If C is anonymous, the value of the inner_name_index item must be zero.

and

If C is not a member, the value of the outer_class_info_index item must be zero.

Since the EnclosingMethod (see [http://java.sun.com/docs/books/jvms/second_edition/ClassFileFormat-Java5.pdf �4.8.6 of the Java 5 class-file spec]) attribute is missing Class.getEnclosingMethod/Constructor are not working.

As a reference, see a corresponding Java example:

class JavaTest {
  public void calc(){
    new Object(){
      int calculate(int i) { return i + 5; }
    }.calculate(12);
  }
}

compiled with javac -source 1.5 -target 1.5 Tester.java

And the javap output:

% javap -v JavaTest
Compiled from "Tester.java"
class JavaTest extends java.lang.Object
  SourceFile: "Tester.java"
  InnerClass: 
   SI-2; //class JavaTest$$1
  minor version: 0
  major version: 49

% javap -v JavaTest\$$1
Compiled from "Tester.java"
class JavaTest$$1 extends java.lang.Object
  SourceFile: "Tester.java"
  EnclosingMethod: length = 0x4
   00 10 00 11 
  InnerClass: 
   SI-3; //class JavaTest$$1
  minor version: 0
  major version: 49

See this [http://old.nabble.com/Re%3A-Scala-Interpreter-Oddity----General-Class-Name-Curio-p25869373.html mailing-list post], as well, for more information.

The text was updated successfully, but these errors were encountered:

scabug · 2009-12-02T07:04:56Z

Imported From: https://issues.scala-lang.org/browse/SI-2749?orig=1
Reporter: @jrudolph

scabug · 2009-12-02T15:26:32Z

@jrudolph said:
I just wanted to add, that I have no particular usecase exhibiting this problem. It just seemed odd and since I researched for the other bug, I don't wanted that information to be lost.

I would guess, the InnerClasses attribute is mainly used in the Java compiler (to recreate scopes and visibility when the source isn't available any more). Therefore, fixing this issue consequently hides from the Java compiler anonymous classes and functions generated by Scala.

However, right now, I cannot come up with a useful example how to use the metadata in EnclosingMethod.

The duplicate InnerClass entry might confuse some tools like ProGuard.

Maybe the whole issue is rather cosmetical and can wait for concrete problems.

scabug · 2010-01-06T19:33:21Z

@paulp said:
I've spent a lot of time on this recently - personally I think it's more than cosmetic, but others probably need convincing. A few notes:

Test$$$$anonfun$$testFunc$$1 cannot be generated as an anonymous inner class unless there is to be a class called Test$$$$anonfun$$testFunc, which you will notice, there is not. The java reflection implementation takes it as a given that after stripping the name of the enclosing class, an anonymous class matches the regex $$[0-9]+ and a local class that plus a simple name. That is definitely not cosmetic: the implementation depends on it.

The EnclosingMethod attribute can be generated if and only if the inner class is local or anonymous. Since we can't generate valid classes of those kinds according to java's definition without an encoding change, we can't generate the attribute either.

This is all a direct consequence of trying to use '$$' to do too much. I don't know what can be done about that.

As to where the inner class goes, at the moment I think you're right it should and could be in the implementation class, but it requires further study.

scabug · 2010-01-07T14:31:52Z

@dragos said:
A few thoughts on this:

the InnerClasses attribute is not 'duplicated'. According to the spec, it needs to appear in every class that references an inner class (but be consistent, of course). The implementation class obviously references the closure, therefore has to have it. The interface gets it because we decided to report inner classes as members of the interface; it seems right since that is the source name of the enclosing entity. It makes Java code that references them easier to write. I don't see a good reason to change that.
Making anonymous classes appear so would hide them from Java, but would solve some of these perky naming issues. I don't have the links at hand, but I recall discussions on the mailing list where people needed that.
The unusual naming convention makes compilation a bit more stable: since anonymous classes are created using a per-file counter, adding a closure in one method invalidates (renames) all closures that come afterwards. By adding the name of the enclosing method this effect is reduced to only one method. I don't remember the exact use case, but it seemed to make debugging easier for some people.
Sun's JVM seems to rely on naming conventions to recover outer class names, but that is not (IMO) required by the spec. For instance, the IBM VM is able to report the enclosing class for objects (where '$$'s are again used liberally) using the information in the InnerClasses table. It would be so much better if it was the other way around..

Having said that, I don't have any strong objections to changing these naming conventions. I just don't see any obvious way to satisfy all requirements and the effort seems disproportionate to the gains.

It boils down to two questions:

Should anonymous inner classes be reported as such? (javac won't see them anymore)
Should we report inner classes defined in traits as belonging to the implementation class? What do we do when there's no impl class (pure interfaces?) Same question goes for top-level objects, who also generate two classes from the same definition.

or in more general terms: should we revamp our naming conventions to satisfy Java binary compatibility guidelines?

There's enough meat for a SIP here, but I am very time constrained. I would happily comment/review/contribute if someone wants to take over.

scabug · 2010-06-13T01:23:17Z

@paulp said:
See also #3561.

scabug · 2010-09-16T17:16:08Z

Charles Oliver Nutter (headius) said:
This is not just cosmetic. It causes java.lang.Class.getDeclaredClasses to blow up as follows when walking classes to get methods, inner classes, etc. This is done in JRuby's Java integration layer to build up our meta-structure.

No class Scala delivers should ever break Java reflection. For now we are patching around it by catching the NoClassDefFoundError and giving up on inspecting inner classes for the class in question.

Class.java:-2:in `getDeclaredClasses0': java.lang.NoClassDefFoundError: scala/collection/mutable/ArrayBuffer$$$$anonfun$$remove$$1
	from Class.java:1699:in `getDeclaredClasses'
	from JavaClass.java:1848:in `getDeclaredClasses'
	from JavaClass.java:695:in `installClassClasses'
	from JavaClass.java:615:in `setupProxy'
	from Java.java:506:in `createProxyClass'
	from Java.java:445:in `getProxyClass'
	from Java.java:487:in `get_proxy_class'
	from JavaUtilities.java:34:in `get_proxy_class'
	from org/jruby/javasupport/JavaUtilities$$s_method_1_0$$RUBYINVOKER$$get_proxy_class.gen:65535:in `call'
	from CachingCallSite.java:312:in `cacheAndCall'
	from CachingCallSite.java:151:in `call'
	from CallOneArgNode.java:57:in `interpret'
	from LocalAsgnNode.java:123:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from WhenOneArgNode.java:36:in `whenSlowTest'
	from WhenOneArgNode.java:46:in `when'
	from CaseNode.java:133:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from BlockNode.java:71:in `interpret'
	from ASTInterpreter.java:64:in `INTERPRET_METHOD'
	from InterpretedMethod.java:184:in `call'
	from DefaultMethod.java:177:in `call'
	from CachingCallSite.java:312:in `cacheAndCall'
	from CachingCallSite.java:151:in `call'
	from FCallOneArgNode.java:36:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from BlockNode.java:71:in `interpret'
	from RootNode.java:129:in `interpret'
	from ASTInterpreter.java:101:in `INTERPRET_ROOT'
	from Ruby.java:721:in `runInterpreter'
	from Ruby.java:582:in `runNormally'
	from Ruby.java:418:in `runFromMain'
	from Main.java:286:in `run'
	from Main.java:128:in `run'
	from Main.java:97:in `main'
Caused by:
URLClassLoader.java:202:in `run': java.lang.ClassNotFoundException: scala.collection.mutable.ArrayBuffer$$$$anonfun$$remove$$1
	from AccessController.java:-2:in `doPrivileged'
	from URLClassLoader.java:190:in `findClass'
	from JRubyClassLoader.java:49:in `findClass'
	from ClassLoader.java:307:in `loadClass'
	from ClassLoader.java:248:in `loadClass'
	from Class.java:-2:in `getDeclaredClasses0'
	from Class.java:1699:in `getDeclaredClasses'
	from JavaClass.java:1848:in `getDeclaredClasses'
	from JavaClass.java:695:in `installClassClasses'
	from JavaClass.java:615:in `setupProxy'
	from Java.java:506:in `createProxyClass'
	from Java.java:445:in `getProxyClass'
	from Java.java:487:in `get_proxy_class'
	from JavaUtilities.java:34:in `get_proxy_class'
	from org/jruby/javasupport/JavaUtilities$$s_method_1_0$$RUBYINVOKER$$get_proxy_class.gen:65535:in `call'
	from CachingCallSite.java:312:in `cacheAndCall'
	from CachingCallSite.java:151:in `call'
	from CallOneArgNode.java:57:in `interpret'
	from LocalAsgnNode.java:123:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from WhenOneArgNode.java:36:in `whenSlowTest'
	from WhenOneArgNode.java:46:in `when'
	from CaseNode.java:133:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from BlockNode.java:71:in `interpret'
	from ASTInterpreter.java:64:in `INTERPRET_METHOD'
	from InterpretedMethod.java:184:in `call'
	from DefaultMethod.java:177:in `call'
	from CachingCallSite.java:312:in `cacheAndCall'
	from CachingCallSite.java:151:in `call'
	from FCallOneArgNode.java:36:in `interpret'
	from NewlineNode.java:103:in `interpret'
	from BlockNode.java:71:in `interpret'
	from RootNode.java:129:in `interpret'
	from ASTInterpreter.java:101:in `INTERPRET_ROOT'
	from Ruby.java:721:in `runInterpreter'
	from Ruby.java:582:in `runNormally'
	from Ruby.java:418:in `runFromMain'
	from Main.java:286:in `run'
	from Main.java:128:in `run'
	from Main.java:97:in `main'

scabug · 2010-09-16T18:11:06Z

@paulp said:
Replying to [comment:10 headius]:

This is not just cosmetic. It causes java.lang.Class.getDeclaredClasses to blow up as follows when walking classes to get methods, inner classes, etc. This is done in JRuby's Java integration layer to build up our meta-structure.

No class Scala delivers should ever break Java reflection.

I'm with you on both points (as you can see above where I say it's more than cosmetic) but as to the second point, note that #2083 was closed wontfix and with the indication that all bets are off for classes with a $$ in them, which of course is almost all of them. I find I can no longer crash things in the same way, because classOf denies all knowledge of the trait implementation class - I wonder when that changed.

scabug · 2010-09-16T18:49:58Z

Charles Oliver Nutter (headius) said:
I disagree with any wontfix that allows Scala to continue shipping classes that can't be 100% introspected via reflection. I do sympathize, however...JRuby generates some synthetic classes that also overload $$ and we have run into occasional problems ourselves. They're definitely bugs we accept and fix when reported, though.

It's also probably valid to say that the JDK is too sensitive to class names in these cases, but of course we're not going to get the JDK to fix this in any reasonable timeframe, so we have to be good citizens and not do things that break it.

scabug · 2010-10-28T12:16:42Z

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

Replying to [comment:10 headius]:

This is not just cosmetic. It causes java.lang.Class.getDeclaredClasses to blow up as follows when walking classes to get methods, inner classes, etc. This is done in JRuby's Java integration layer to build up our meta-structure.

No class Scala delivers should ever break Java reflection.

I'm with you on both points (as you can see above where I say it's more than cosmetic) but as to the second point, note that #2083 was closed wontfix and with the indication that all bets are off for classes with a $$ in them, which of course is almost all of them. I find I can no longer crash things in the same way, because classOf denies all knowledge of the trait implementation class - I wonder when that changed.

I don't know exactly when it changed, but I can point out the code that does that. It's the DefaultJavaContext in the ClassPath abstractions that filters out classes ending in "$$class".

That being said, I am open to any suggestions as to how to fix this.

scabug · 2010-11-13T12:31:50Z

@dragos said:
(In r23507) Fix InnerClasses attribute: anonymous classes don't have
an outer name. EnclosingMethod is correctly generated.
Fixed isAnonymousClass definition. Updated test that
depends on anonymous inner class names.
Closes (again) #3249, references #2749.
review by odersky,extempore.

scabug · 2010-11-13T12:32:57Z

@dragos said:
I close this bug as most points have been addressed. Anonymous classes are correctly marked as non-members (no outer name in the InnerClasses attribute), and the EnclosingMethod is generated. I see no test case here (I just tried the ArrayBuffer in headius' example) and it works. See r23507

scabug closed this as completed May 18, 2011

scabug assigned dragos Apr 7, 2017

This was referenced Apr 7, 2017

Unable to reflect inner classes of scala.actors.Actor #3561

Closed

getDeclaredClasses seems to miss some (REPL) or all (scalac) classes (objects) declared #4023

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Irregular class-files generated for InnerClasses #2749

Irregular class-files generated for InnerClasses #2749

scabug commented Dec 2, 2009

scabug commented Dec 2, 2009

scabug commented Dec 2, 2009

scabug commented Jan 6, 2010

scabug commented Jan 7, 2010

scabug commented Jun 13, 2010

scabug commented Sep 16, 2010

scabug commented Sep 16, 2010

scabug commented Sep 16, 2010

scabug commented Oct 28, 2010

scabug commented Nov 13, 2010

scabug commented Nov 13, 2010

Irregular class-files generated for InnerClasses #2749

Irregular class-files generated for InnerClasses #2749

Comments

scabug commented Dec 2, 2009

scabug commented Dec 2, 2009

scabug commented Dec 2, 2009

scabug commented Jan 6, 2010

scabug commented Jan 7, 2010

scabug commented Jun 13, 2010

scabug commented Sep 16, 2010

scabug commented Sep 16, 2010

scabug commented Sep 16, 2010

scabug commented Oct 28, 2010

scabug commented Nov 13, 2010

scabug commented Nov 13, 2010