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

tests fail with latest Scala 2.13 due to productElementName #143

Closed
xuwei-k opened this issue Aug 9, 2018 · 13 comments
Closed

tests fail with latest Scala 2.13 due to productElementName #143

xuwei-k opened this issue Aug 9, 2018 · 13 comments

Comments

@xuwei-k
Copy link
Contributor

xuwei-k commented Aug 9, 2018

[genjavadoc] missing methods:
[genjavadoc] public scala.collection.Iterator<java.lang.String> akka.rk.buh.is.it.Blarb$A$.productElementNames()
[genjavadoc:error] Reporter completed abruptly with an exception after receiving event: TestFailed(Ordinal(0, 12),Scala: 
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.toString()
[genjavadoc:error]   public scala.collection.Iterator<java.lang.Object> akka.rk.buh.is.it.Blarb$A$.productIterator()
[genjavadoc:error]   public boolean akka.rk.buh.is.it.Blarb$A$.canEqual(java.lang.Object)
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productPrefix()
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.hashCode()
[genjavadoc:error]   private java.lang.Object akka.rk.buh.is.it.Blarb$A$.readResolve()
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productElementName(int)
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.productArity()
[genjavadoc:error]   public java.lang.Object akka.rk.buh.is.it.Blarb$A$.productElement(int)
[genjavadoc:error]   public scala.collection.Iterator<java.lang.String> akka.rk.buh.is.it.Blarb$A$.productElementNames() 
[genjavadoc:error] did not match Java: 
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.toString()
[genjavadoc:error]   public scala.collection.Iterator<java.lang.Object> akka.rk.buh.is.it.Blarb$A$.productIterator()
[genjavadoc:error]   public boolean akka.rk.buh.is.it.Blarb$A$.canEqual(java.lang.Object)
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productPrefix()
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.hashCode()
[genjavadoc:error]   private java.lang.Object akka.rk.buh.is.it.Blarb$A$.readResolve()
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productElementName(int)
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.productArity()
[genjavadoc:error]   public java.lang.Object akka.rk.buh.is.it.Blarb$A$.productElement(int)
[genjavadoc:error] (in class akka.rk.buh.is.it.Blarb$A$),SignatureSpec,com.typesafe.genjavadoc.SignatureSpec,Some(com.typesafe.genjavadoc.SignatureSpec),The generated java files must contain the same methods and classes as the original Scala files,contain the same methods and classes as the original Scala files,Vector(),Some(org.scalatest.exceptions.TestFailedException: Scala: 
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.toString()
[genjavadoc:error]   public scala.collection.Iterator<java.lang.Object> akka.rk.buh.is.it.Blarb$A$.productIterator()
[genjavadoc:error]   public boolean akka.rk.buh.is.it.Blarb$A$.canEqual(java.lang.Object)
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productPrefix()
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.hashCode()
[genjavadoc:error]   private java.lang.Object akka.rk.buh.is.it.Blarb$A$.readResolve()
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productElementName(int)
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.productArity()
[genjavadoc:error]   public java.lang.Object akka.rk.buh.is.it.Blarb$A$.productElement(int)
[genjavadoc:error]   public scala.collection.Iterator<java.lang.String> akka.rk.buh.is.it.Blarb$A$.productElementNames() 
[genjavadoc:error] did not match Java: 
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.toString()
[genjavadoc:error]   public scala.collection.Iterator<java.lang.Object> akka.rk.buh.is.it.Blarb$A$.productIterator()
[genjavadoc:error]   public boolean akka.rk.buh.is.it.Blarb$A$.canEqual(java.lang.Object)
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productPrefix()
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.hashCode()
[genjavadoc:error]   private java.lang.Object akka.rk.buh.is.it.Blarb$A$.readResolve()
[genjavadoc:error]   public java.lang.String akka.rk.buh.is.it.Blarb$A$.productElementName(int)
[genjavadoc:error]   public int akka.rk.buh.is.it.Blarb$A$.productArity()
[genjavadoc:error]   public java.lang.Object akka.rk.buh.is.it.Blarb$A$.productElement(int)
@SethTisue
Copy link
Member

it is easy enough to add productElementName to the expected outputs for 2.13, but something weird is happening with the new productElementNames (plural) method. investigating

@SethTisue SethTisue self-assigned this Aug 15, 2018
@SethTisue
Copy link
Member

okay well now I really need to fix this so we can publish for 2.13.0-M5 (#144)

@SethTisue
Copy link
Member

productElementNames is defined in scala.Product. the implementation is the same for every case class, it doesn't get overridden.

but if you javap -classpath .:/usr/local/scala-2.13.0-M5/lib/scala-library.jar -private scala.Product | grep productElementNames you see:

  public static scala.collection.Iterator productElementNames$(scala.Product);
  public scala.collection.Iterator<java.lang.String> productElementNames();

so already this is odd, what's the extra static method doing there?

hypothesis: is it because Product is a "universal trait", that is, it extends Any?

and then if you compile e.g. case class S(x: Int) and do javap -classpath .:/usr/local/scala-2.13.0-M5/lib/scala-library.jar -private S | grep productElementNames you see

  public scala.collection.Iterator<java.lang.String> productElementNames();

even though S doesn't override productElementNames, it still gets its own implementation, the body looks like:

         0: aload_0
         1: invokestatic  #29                 // InterfaceMethod scala/Product.productElementNames$:(Lscala/Product;)Lscala/collection/Iterator;
         4: areturn

I assume this has to do with Product being a universal trait, but I don't understand why it would actually be needed.

@SethTisue
Copy link
Member

SignatureSpec compiles the Scala code, compiles the Java code generated by the plugin, then compares the resulting lists of methods. we've learned something, above, about why productElementNames appears in the list from the Scala code.

but why doesn't productElementNames appear in the Java code? is there some logic, probably in Output.scala, that filters it out, and we need that same logic in SignatureSpec...?

@SethTisue
Copy link
Member

this seems closely related to @ktoso's #86 , specifically 60b3328 — "These synthetic static methods appear since 2.12.0-M5+ as companions to default methods"

@SethTisue
Copy link
Member

SethTisue commented Sep 18, 2018

is there some logic, probably in Output.scala, that filters it out, and we need that same logic in SignatureSpec...?

some println debugging indicates that Output#merge doesn't even see the method. hypothesis: that could be because the plugin has val runsAfter = List("fields") — perhaps the case class hasn't yet acquired the overriding method at that phase of compilation. whereas SignatureSpec uses Java reflection, so it's operating on generated bytecode

I see no clear indication in the generated bytecode that the override isn't a true override

@SethTisue
Copy link
Member

in any case, I'm satisfied this isn't a blocker for going ahead and publishing on 2.13.0-M5

@SethTisue
Copy link
Member

as a stopgap measure, #146 simply disables that part of the test

@SethTisue
Copy link
Member

I don't think I'm going to ever find the time to get to the bottom of this, too much else to do for 2.13.0-RC1 and 2.13.0. anyone else interested in delving into it...?

@SethTisue SethTisue removed their assignment Jan 18, 2019
@raboof
Copy link
Contributor

raboof commented Jan 28, 2019

Output#merge doesn't even see the method. hypothesis: that could be because the plugin has val runsAfter = List("fields") — perhaps the case class hasn't yet acquired the overriding method at that phase of compilation.

Indeed: it looks like this method is generated in the mixin stage. I guess this plugin is run after fields because later steps may modify things in ways we don't expect - it seems the flatten stage moves the InboxExtension to the top level instead of inside Inbox where it should be.

Do I understand correctly that our ways forward here are to either run genjavadoc after mixin (and make sure it for example 'unflattens' classes), or sort of 're-implement' the mixin stage in the genjavadoc logic?

@SethTisue
Copy link
Member

Yes, I think so.

Running way later in the compilation pipeline would probably mean undoing a lot. Maybe it's not impossible — even when you run at a later phase, you can use the compiler's "time travel" feature to ask how something looked before, so for example, even when you run after erasure, you can still see the types before they were erased. But still, it sounds like an ambitious change.

Re-implementing mixin in genjavadoc sounds easier to me.

But these are just my seat-of-the-pants reactions, intelligently deciding on a plan would involve a conversation between someone who knows genjavadoc better than me and someone who knows the compiler middle end better than me (Adriaan/Lukas/Jason).

@SethTisue
Copy link
Member

This is a longstanding issue that this 2.13 change (the addition of productElementNames) just happens to have turned up, right? If we've been living this long with this limitation, maybe we can continue living with it?

@raboof
Copy link
Contributor

raboof commented Jan 30, 2019

This is a longstanding issue that this 2.13 change (the addition of productElementNames) just happens to have turned up, right? If we've been living this long with this limitation, maybe we can continue living with it?

Agreed entirely. I've tried to summarize the more general issue in #163. With that I think this specific issue can be closed.

@raboof raboof closed this as completed Jan 30, 2019
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

3 participants