Ignore inner class scanning in the compiler to support dynamic languages like groovy

[scabug]

Using java libraries that have been generated by languages like groovy where inner classes might not exist from scala causes the scala compiler to throw and error.

The only way to get around this is either remove the inner classes of the library generated during compile time and then put them back in during runtime, or ignore the exception in the scala compiler itself and continue with the class compilation.

The second option is much more integration friendly and if done via a settings flag can be turned on and off as required.

As an example see:

https://github.com/gerritjvv/glue/blob/master/core/glue-unit/src/main/groovy/org/glue/unit/om/impl/ScriptedGlueProcessImpl.groovy

From line 72.

The code I use actually use IMain from groovy, and example is pasted in below.

                                def settings = new Settings()
                                def imain = new IMain(settings)
                                imain.setContextClassLoader()


                                imain.interpret("""
                   import scala.tools.nsc.interpreter._
                                   import scala.tools.nsc._
                   def getIMain(cls:ClassLoader) = {
                                           val s = new Settings
                       s.usejavacp.value = true
                       s.ignoreinnercls.value = true
                       s.embeddedDefaults(cls)
                       new IMain(s){
                         override protected def parentClassLoader:ClassLoader = cls
                       }
                   }
                                   val m = getIMain _
                """)

                                def v = imain.valueOfTerm("m").get().asType(Function1).apply(
                                        new ScalaClassLoader(ctx.class.classLoader))
                                v.setContextClassLoader()

                                def ctx1 = DefaultGlueContextBuilder.buildStaticGlueContextMap(ctx)
                                imain.bind(new NamedParamClass("ctx", "java.util.Map[String, Object]", ctx1))

                                v.bind(new NamedParamClass("ctx", "java.util.Map[String, Object]", ctx1))
                                v.interpret(new String(script.value.decodeBase64()))

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-7741?orig=1
Reporter: Gerrit Jansen van Vuuren (gerritjvv)
Affected Versions: 2.11.0-M4

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
I have modified the compiler in https://github.com/gerritjvv/scala.git to support the feature mentioned.

Please note that the groovy code (in glue) mentioned above also uses the ignoreinnercls feature.

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
Ok, I've tracked it down to the groovypp library that does some AST transformations (valid in groovy, almost like macros in scala) to get static typed compilation.

I've made a small project with the least possible code to show the error:

https://github.com/gerritjvv/SI7741

I'm using:
one empty interface => MyModule.groovy
one single method class that implements the MyModule interface => MyModuleImpl
The UsingIMain class calls the IMain class and performs the correct settings.

I've copied all of the jars (some are not relevant to the code) into the test-lib directory, it also contains the scala jars from my patched repo (https://github.com/gerritjvv/scala.git).
The test.sh script performs the classpath and bootclasspath setup. Passing a single parameter true to the script tells UsingIMain to set the property ignoreinnercls=true, which will activate the patch I've made,
by default its false and the scala compiler will give an error.

Below are examples of two runs I've made:

gvanvuuren@gvanvuuren-UX31E:~/checkouts/SI7741/si7741$ ./test.sh
import scala.tools.nsc.interpreter._
import scala.tools.nsc._
getIMain: (cls: ClassLoader)scala.tools.nsc.interpreter.IMain
m: ClassLoader => scala.tools.nsc.interpreter.IMain =
ctx: java.util.Map[String,Object] = {module=test.si7741.MyModuleImpl@69ee8449}
ctx: java.util.Map[String,Object] = {module=test.si7741.MyModuleImpl@69ee8449}
error: error while loading MyModule, class file '/home/gvanvuuren/checkouts/SI7741/si7741/target/si7741-0.1.0.jar(test/si7741/MyModule.class)' is broken
(class java.util.NoSuchElementException/None.get)

gvanvuuren@gvanvuuren-UX31E:~/checkouts/SI7741/si7741$ ./test.sh true
import scala.tools.nsc.interpreter._
import scala.tools.nsc._
getIMain: (cls: ClassLoader)scala.tools.nsc.interpreter.IMain
m: ClassLoader => scala.tools.nsc.interpreter.IMain =
ctx: java.util.Map[String,Object] = {module=test.si7741.MyModuleImpl@76ad043}
ctx: java.util.Map[String,Object] = {module=test.si7741.MyModuleImpl@76ad043}
CTX {module=test.si7741.MyModuleImpl@76ad043}
!!!!!! Result: 1
import scala.collection.JavaConversions._
import test.si7741._
mymodule: test.si7741.MyModuleImpl = test.si7741.MyModuleImpl@76ad043

[scabug]

@paulp said:
I can only offer a general opinion because there is way too much extraneous material, but do the classes in question conform to the jvm specification or not? If they don't, then adding hacks to scala to work around their non-conformance is not the answer. If they do, then please articulate in a more bytecode-focused way why the error is incorrect.

If you have to mention groovy, you are approaching the problem at too high a layer. Here is what the jvm specification says, emphasis added.

Every CONSTANT_Class_info entry in the constant_pool table which represents a class or interface C that is not a package member must have exactly one corresponding entry in the classes array.

If a class has members that are classes or interfaces, its constant_pool table (and hence its InnerClasses attribute) must refer to each such member, even if that member is not otherwise mentioned by the class. *These rules imply that a nested class or interface member will have InnerClasses information for each enclosing class and for each immediate member.*

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
Hi,

The classes adhere enough to the java spec to run and compile in the JVM, so I would assume that its ok. I've used the same classes from jython, jruby, clojure and javascript on the JVM without any errors. I think the basic issue here is that the scala compiler is stricter than the javac, although scala's strictness is a good thing there are times when you would want to have a flag to disable this strictness on inner classes (e.g. to use other non scala libraries), which otherwise would run perfectly on the JVM.

I'll try to add in a javap and byte code representation of the classes before and after it gives the error.

My first attempt at showing a possible solution is surely improvable, I don't think its a hack but just adding a flag on a feature that is part of the scalac.

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
Below is an example of the same class with and without the AST transformations, both run fine in java but the second example does not pass scalac.

--- This is MyModule without the AST transformations applied

Classfile /home/gvanvuuren/checkouts/SI7741/si7741/testjars/MyModule.class
Last modified Aug 13, 2013; size 111 bytes
MD5 checksum c2759829428a3bca912f0b6b39650567
Compiled from "MyModule.groovy"
public interface test.si7741.MyModule
SourceFile: "MyModule.groovy"
minor version: 0
major version: 47
flags: ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT
Constant pool:
#1 = Utf8 test/si7741/MyModule
#2 = Class #1 // test/si7741/MyModule
#3 = Utf8 java/lang/Object
#4 = Class #3 // java/lang/Object
#5 = Utf8 MyModule.groovy
#6 = Utf8 SourceFile
{
}

• This is MyModule with the AST transformations applied and causes scalac to throw the assertion.

gvanvuuren@gvanvuuren-UX31E:~/checkouts/SI7741/si7741/testjars$ /opt/jdk1.7.0_21/bin/javap -c -s -l -v MyModule.class
Classfile /home/gvanvuuren/checkouts/SI7741/si7741/testjars/MyModule.class
Last modified Aug 13, 2013; size 174 bytes
MD5 checksum cc571575d598491de6e273b6909e1675
Compiled from "MyModule.groovy"
public interface test.si7741.MyModule
SourceFile: "MyModule.groovy"
InnerClasses:
#8= #7 of #2; //1=class test/si7741/MyModule$1 of class test/si7741/MyModule
minor version: 0
major version: 47
flags: ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT
Constant pool:
#1 = Utf8 test/si7741/MyModule
#2 = Class #1 // test/si7741/MyModule
#3 = Utf8 java/lang/Object
#4 = Class #3 // java/lang/Object
#5 = Utf8 MyModule.groovy
#6 = Utf8 test/si7741/MyModule$1
#7 = Class #6 // test/si7741/MyModule$1
#8 = Utf8 1
#9 = Utf8 SourceFile
#10 = Utf8 InnerClasses
{
}

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
I need some help with the unit tests, trying to understand how to setup a simple IMain test but while running
ant test.junit

I get

[scalacfork] Compiling 2 files to /home/gvanvuuren/checkouts/scala-commit/scala/build/junit/classes
[scalacfork] /home/gvanvuuren/checkouts/scala-commit/scala/test/junit/scala/tools/nsc/IgnoreMissingInnerClassTest.scala:14: error: object interpreter is not a member of package scala.tools.nsc
[scalacfork] import scala.tools.nsc.interpreter.IMain
[scalacfork] ^
[scalacfork] /home/gvanvuuren/checkouts/scala-commit/scala/test/junit/scala/tools/nsc/IgnoreMissingInnerClassTest.scala:15: error: object interpreter is not a member of package scala.tools.nsc
[scalacfork] import scala.tools.nsc.interpreter.NamedParamClass
[scalacfork] ^
[scalacfork] /home/gvanvuuren/checkouts/scala-commit/scala/test/junit/scala/tools/nsc/IgnoreMissingInnerClassTest.scala:45: error: not found: type IMain
[scalacfork] new IMain(s) {import scala.tools.nsc.Settings
[scalacfork] ^

I only really need to setup a test for ClassfileParser but its abstract and from the code its not quite clear how to implement it. Any pointers?

[scabug]

@retronym said (edited on Mar 2, 2015 7:02:59 AM UTC):
Hi Gerrit,

I have been looking into a similar issue, as reported https://groups.google.com/d/msg/scala-internals/lGE15Ly6xHU/q1LLfS3w9R0J

It isn't quite the same issue as this one (in that report groovy emits the Outer$1 class, but gets the STATIC flag wrong.)

I have developed a fix that stops Scala from forcing the type information of members of non-Scala defined interfaces unnecessarily, and defers failures due to completely absent inner classes.

https://github.com/retronym/scala/compare/ticket/7741

I'm going to ask Adam Lewandowski to pick up this branch and test out how it helps with groovy interop. If you're still interested in this issue, you might like to do the same.

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
hi Jason, thanks I'll ping Adam and ask if he could merge this branch also!

[scabug]

Adam Lewandowski (alewando) said:
Gerrit,
I'm not able to build your test project. It's getting groovypp from a repository that doesn't exist anymore. Can you update it with a new repo?

[scabug]

Gerrit Jansen van Vuuren (gerritjvv) said:
Adam, unfortunately the groovypp is not available anymore in the maven repo that was made public for it :(. You can however merge in the changes from scala/scala@2.11.x...gerritjvv:master, this does not require groovy or groovypp to run.

[scabug]

Adam Lewandowski (alewando) said:
That branch has changes to the scala compiler, which is not what I'm after. Jason's branch has the proposed compiler change. I'd like to test his branch against your test case to verify that it fixes your issue. Looking closer at the test code, I see that it's invoking your modified scala compiler at runtime. I'll try pointing it at Jason's proposed modified compiler instead.

[scabug]

Adam Lewandowski (alewando) said:
Jason, I've verified that your ticket/7741 branch does resolve my issue. I also ran it through the full scala test suite without any problems.
I was able to update Gerrit's test case (pull request submitted) and verify that your changes fix his test case as well. Although, interestingly, I had to go back to an older Groovy version to reproduce his error, so perhaps that one has been fixed on the Groovy side.

Should I go ahead and submit your branch as a pull request?

[scabug]

@SethTisue said:
scala/scala#4386