Fix #3476: Add Artifact flag to closures

[Duhemm]

This way, we will not generate generic java signatures for closures.

Note that scalac also marks closures as Artifact during UnCurry.

See:

• https://github.com/scala/scala/blob/7c8e4ac0797872190f67d1738251ded1980d8d77/src/compiler/scala/tools/nsc/ast/TreeGen.scala#L260-L272
• https://github.com/scala/scala/blob/7c8e4ac0797872190f67d1738251ded1980d8d77/src/compiler/scala/tools/nsc/transform/UnCurry.scala#L221-L222

[smarter]

That should indeed fix #3476 but why was the generic signature here wrong in the first place? Couldn't the same issue exist for non-closures? Also, could -Xverify-signature be improved to detect this kind of issues ?

[Duhemm]

I think that the reason why the signatures are wrong for those nodes is because they are transformed again after erasure (where their signatures are created). I imagine that those symbols are ignored because we know that their signatures can't be trusted to be stable, and we don't expect them to be inspected by a debugger or reflection.

Note that scalac also generates a signature that doesn't correspond to the descriptor in this case (if you force scalac to emit signatures, normally it wouldn't generate one):

class Foo {
  def m[T](x: T): Int => T = (y: Int) => x
}

  public static final T $anonfun$m$1(int);
    descriptor: (Ljava/lang/Object;I)Ljava/lang/Object;
    // code...
    Signature: #76                          // (I)TT;

The discrepancy is not caught by -Xverify either.

[Duhemm]

@smarter I've been looking into the wrong signatures for lifted methods and classes.

I've noticed that lifted methods do not get the Lifted flag (actually, it looks like it's never set anywhere in the compiler). I've added it for lifted DefDefs. This fixes the issue of wrong generic signatures for lifted DefDefs, because we don't generate generic signatures for symbols that have the flags Method and Lifted (assuming I didn't miss other spots, or set the flag too often).

Lifted classes are another story. In this example, scalac gives the class the outer reference during explicitOuter, whereas dotty adds it during lambdaLift:

class Foo {
  def m[T](x: T): Int => T = {
    def bar(y: Int) = x
    class buzz[U](z: Int) { def boo = x.toString + z.toString }
    bar _
  }
}

I'm not sure what to do here.

[smarter]

It would be better to set the flag when the denotation is transformed in liftLocals: https://github.com/Duhemm/dotty/blob/ee2d91e5e93be553751a2156e911540a66afb1da/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala#L344

[smarter]

When a class is lifted from a method it should not get a generic signature since there is no reason to call it from outside (in fact, we should probably emit them as private classes). When a class is lifted from another class it should get a generic signature, but this seems to already be working correctly:

class Foo(x: Int) {
  class buzz[U](z: U) { def boo = x.toString + z.toString }
}

[smarter]

Ping ?

[Duhemm]

@smarter I've addressed your comment and added a test.

[smarter]

@smarter I've addressed your comment and added a test.

Which comment exactly? I just tried the example from #3503 (comment) and you're still generating a generic signature for the local class buzz.

[allanrenucci]

SymUtils::isLocal

[Duhemm]

@smarter Sorry, I meant this one: #3503 (comment).

I've changed the PR so that no signature is generated for local classes.

However, the constructor of local classes may still get a wrong signature. I imagine that this is okay, since scalac will also generate a signature that doesn't match the descriptor:

dotc:

public <U extends java.lang.Object> Foo$buzz$1(int);
  descriptor: (Ljava/lang/Object;I)V
  // ...
  Signature: #48                          // <U:Ljava/lang/Object;>(I)V

scalac:

public Foo$buzz$1(Foo, int);
  descriptor: (LFoo;ILjava/lang/Object;)V
  // ...
  Signature: #53                          // (LFoo;I)V

Finally, not generating signatures for local classes means that code such as:

def foo = {
  class Foo[U, T <: java.util.List[U]]
  classOf[Foo[_, _]].getTypeParameters().mkString(", ")
}

will produce different outputs when compiled with scalac or dotc. I don't think this is really a valid use case, but it may be a surprising behavior.

[smarter]

However, the constructor of local classes may still get a wrong signature.

To be consistent, the methods inside a local class should not get signatures either.

[smarter]

• Please add more testcases (class inside method, class inside class inside method, ...)
• Is it possible to improve -Xverify-signature to catch the kind of invalid signatures we were generating before this PR?

[Duhemm]

I've changed the PR to avoid generating signatures for all local symbols, and added more test cases.

I'd be happy to improve -Xverify-signatures, which only does syntactical checks at the moment. I've been looking a bit at the backend, and I Imagine that we could parse the generic signatures and compare the number of parameters and their erased types with that of the method descriptor.

What do you think? Could we leave that for another PR?

[smarter]

What do you think? Could we leave that for another PR?

Sure.

[smarter]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 1 running.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3503/ to see the changes.

Benchmarks is based on merging with master (3cba055)