Dotty gets confused by raw types in bytecode signatures

[allanrenucci]

Consider the following file Test.scala:

import org.testng.IAnnotationTransformer
import org.testng.annotations.ITestAnnotation
import java.lang.reflect.Method
import java.lang.reflect.Constructor

class SingleTestAnnotationTransformer(testName: String) extends IAnnotationTransformer {
  override def transform( annotation: ITestAnnotation, testClass: java.lang.Class[_], testConstructor: Constructor[_], testMethod: Method): Unit = {}
}

Running

./bin/dotc -classpath "$(coursier fetch -p org.testng:testng:6.8.7)" Test.scala 

produces the following error:

-- [E036] Reference Error: Test.scala:9:15 -------------------------
9 |  override def transform( annotation: ITestAnnotation, testClass: java.lang.Class[_], testConstructor: Constructor[_], testMethod: Method): Unit = {
  |               ^
  |               method transform overrides nothing

[smarter]

We can see what signature Dotty expects by removing the override def:

6 |class SingleTestAnnotationTransformer(testName: String) extends IAnnotationTransformer {
  |      ^
  |class SingleTestAnnotationTransformer needs to be abstract, since def transform: 
  |  (x$0: org.testng.annotations.ITestAnnotation, x$1: Class, x$2: 
  |    java.lang.reflect.Constructor
  |  , x$3: java.lang.reflect.Method): Unit is not defined 

Notice the missing type parameters for Class and Constructor, my guess is that Dotty gets confused by the raw types used in https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/IAnnotationTransformer.java.

[allanrenucci]

Can be reproduced by compiling Foo.java with javac:

interface Foo {
  void foo(java.util.List list);
}

and then Test.scala with the previous result on the classpath:

class Test extends Foo {
  override def foo(list: java.util.List[_]): Unit = ???
}

[smarter]

I'm on it but it won't be trivial: we need to check for raw types in ClassfileParser which is hard to do without forcing too much things and getting into cycles.

[adriaanm]

Would be nice to have a lazier LazyType PolyType, where you can query just the type params without forcing the rest of the type.

[smarter]

@adriaanm Behold! https://github.com/lampepfl/dotty/blob/c50c16420b037268fe8e62bdd659e60ee2ceabfc/compiler/src/dotty/tools/dotc/core/SymDenotations.scala#L1897-L1903

My plan is in fact to see if I can use this for classfile loading too ;)

[odersky]

Sigh. It's 14 years after Java generics came out and we still have to deal with raw types. 😢

This was one of the most messy aspects of nsc, basically impossibly to get right without forcing too much.

[odersky]

It took me a while to realize: it's because of the change to native applied types. Before, it was convenient that the raw type Class was both a higher-kinded type and an existential. Now, we don't have it so easy anymore. But I have an idea what to do by adapting (and hopefully improving) the concept of cooking in nsc.

[odersky]

It turned out to be a lot easier than for nsc because member types are completers anyway. This avoids the problem of forcing too much by accident.

[retronym]

This doesn't currently cook raw types in parent position.

tail /tmp/Test.java sandbox/client.scala ;  javac -d /tmp /tmp/Test.java && scalac -cp /tmp sandbox/client.scala && ./bin/dotc -classpath /tmp sandbox/client.scala
==> /tmp/Test.java <==
class Test {
	static abstract class A<X> extends B {
		abstract B foo();
	}
	static abstract class B<X> {
		abstract A bar();
	}
}

==> sandbox/client.scala <==
class Client {
  val a = (_: Test.A[AnyRef]).foo
  val b = (_: Test.B[AnyRef]).bar
  def test(x: Test.A[AnyRef]): Test.B[_] = x
}
-- [E007] Type Mismatch Error: sandbox/client.scala:4:43 -----------------------
4 |  def test(x: Test.A[AnyRef]): Test.B[_] = x
  |                                           ^
  |                                           found:    Test.A[AnyRef](x)
  |                                           required: Test.B[_]
  |

one error found

[retronym]

It turned out to be a lot easier than for nsc because member types are completers anyway.

I've wanted to do the same thing in scalac, as it avoids spurious cycles. Here's what I tried: scala/scala#5308

I didn't pull the trigger because I was worried about the extra footprint needed by keeping the classfile bytes around for every Java class we touch (as is needed to facilitate deferred lookups to the constant pool). I plan to refactor ClassfileParser to be able to discard parts of the classfile we don't care about (e.g the Code attributes, private methods).

There is some conceptual overlap with the JDK's signature archive, ct.sym. This is a stripped down version of rt.jar. javac links against that, rather than rt.jar, for performance reasons. As of JDK9, it also contains historized versions of the class uses.

[smarter]

@retronym Thanks for the report, I opened #3298

[odersky]

I plan to refactor ClassfileParser to be able to discard parts of the classfile we don't care about (e.g the Code attributes, private methods).

That would be very helpful. We should do something similar with Tasty files which suffer the same problem.