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

Missing generic type in JVM signature of public field #17069

Closed
christian-mibex opened this issue Mar 8, 2023 · 11 comments · Fixed by #19207
Closed

Missing generic type in JVM signature of public field #17069

christian-mibex opened this issue Mar 8, 2023 · 11 comments · Fixed by #19207

Comments

@christian-mibex
Copy link

Compiler version

3.2.2

Minimized code

//> using scala "3.2.2"
//> using repository "https://maven.atlassian.com/repository/public"
//> using lib "com.atlassian.bitbucket.server:bitbucket-rest-model:7.14.0"

import com.atlassian.bitbucket.rest.repository.RestRepository
import scala.beans.BeanProperty

case class SomeRequest(
    @BeanProperty var repos: java.util.List[RestRepository]
)

Output

Partial output of scalap -c -p

// public class generic_java_type$u002Esc$SomeRequest implements scala.Product,java.io.Serializable {
//   private java.util.List repos;
//   public java.util.List<com.atlassian.bitbucket.rest.repository.RestRepository> repos();
//     Code:
//        0: aload_0
//        1: getfield      #39                 // Field repos:Ljava/util/List;
//        4: areturn```

Expectation

Scala 2 and Java add the generic type of the java.util.List to the repos field. Changing the version //> scala 2 in the code above produces:

// public class generic_java_type$u002Esc$SomeRequest implements scala.Product,java.io.Serializable {
//   private java.util.List<com.atlassian.bitbucket.rest.repository.RestRepository> repos;
//   public java.util.List<com.atlassian.bitbucket.rest.repository.RestRepository> repos();
//     Code:
//        0: aload_0
//        1: getfield      #28                 // Field repos:Ljava/util/List;
//        4: areturn

We got hit by this when Jackson was unable to deserialize the class correctly because it could not reflect on the type of the list.

@christian-mibex christian-mibex added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 8, 2023
@prolativ
Copy link
Contributor

prolativ commented Mar 8, 2023

The problem seems to be more general and has nothing to do with @BeanProperty. The reproduction could be reduced to just

class Foo {
    val generic: List[String] = ???
}

In the bytecode produced byt scala 3 (starting from 3.0.0 up to the newest nightly) we can see

  private final scala.collection.immutable.List generic;
  public scala.collection.immutable.List<java.lang.String> generic();

So in general result types of getters have type parameters in signatures but that's not the case for underlying fields

@prolativ prolativ added area:erasure and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 8, 2023
@prolativ prolativ added this to the Future versions milestone Mar 8, 2023
@He-Pin
Copy link

He-Pin commented Mar 8, 2023

Will this be scheduled to the next 3.3.1 version?

@prolativ
Copy link
Contributor

prolativ commented Mar 9, 2023

This doesn't seem to have very high priority for the core compiler team as the problem has existed since 3.0.0 and nobody even noticed that before. However contributions are welcome 🙂. A fix could go in 3.3.1 if it gets implemented before the branch cut-off for 3.3.1 (which should coincide with the release of stable 3.3.0, so probably in like two weeks unless more regressions appear and we don't manage to fix them on time).

I tried to have a quick look at a potential source of the issue and here are my observations:
A public field like

val generic: List[String]

in the example above is transformed to a method with an additional <accessor> flag in Getters phase, resulting in something like

<accessor> def generic: List[String]

After the type erasure in Erasure phase this becomes

<accessor> def generic: List

Then in Memoize phase for each accessor (getter or setter) an underlying field definition is generated so after this phase we have

<accessor> def generic: List
val generic: List

Finally, in GenBCode phase, the bytecode is generated. For a field like val generic this is done inside def addClassFields() in dotty.tools.backend.jvm.BCodeSkelBuilder, which in turn invokes getGenericSignature. This method tries to look back at the type of a definition before the type erasure (at the beginning of Erasure phase). In case of def generic, getGenericSignature correctly returns List[String]. However, for val generic it returns just List.

My assumption is that as val generic did not yet exist during Erasure the compiler returns the earliest available type for this definition, which is List (as taken from Memoize). So one would probably have to somehow associate this synthetic definition with the type of the corresponding def before the erasure.

@SethTisue
Copy link
Member

(an argument for prioritizing could be that Jackson is very widely used)

@prolativ prolativ changed the title Missing generic type in a java.util.List case class field Missing generic type in JVM signature of public field Mar 9, 2023
@rjdestigter
Copy link

I'm running into this issue as well when using OptaPlanner. It throws the exception defined here

@akorneev
Copy link

akorneev commented May 16, 2023

We encountered this error while trying to migrate a project (from 2.13) that uses JPA (Hibernate).

This bug seems to break interop with many Java libraries which rely on field types (reflection) for serialization/persistence.

@hohonuuli
Copy link

I'm having the same issue as @akorneev (i.e. migrating a JPA project from 2.13 to 3.3.1 fails). I'm hitting this bug with Gson. Sample code to reproduce the reflection issue is on Stackoverflow

@mbovel
Copy link
Member

mbovel commented Dec 5, 2023

@sjrd would you have any guidance about this issue? Do you think it's a hard one?

@mbovel
Copy link
Member

mbovel commented Dec 5, 2023

@EugeneFlesselle @entangled90 @AnotherMedo @Motii1: the issue we were assigned for tonight's Spree has been solved. I therefore suggest we switch to this one. Sorry for the late notice!

If it's anybody's first time hacking on Dotty, you can find information on to get started at https://dotty.epfl.ch/docs/contributing/getting-started.html.

@mbovel
Copy link
Member

mbovel commented Dec 5, 2023

Not sure how to test it. Maybe we could have the following run test?

class Foo:
    val generic: List[String] = ???

@main def test =
    val tpe = classOf[Foo].getDeclaredField("generic").getGenericType()
    val tpe2 = classOf[Foo].getDeclaredMethod("generic").getGenericReturnType()
    assert(tpe == tpe2)

@sjrd
Copy link
Member

sjrd commented Dec 5, 2023

Your entry point will be https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala
You'll want to see where def javaSig is called and follow the chain. It seems to already be transitively called for fields starting from
https://github.com/lampepfl/dotty/blob/7480582f0c539794c42d2c8b5a28384608d4ec16/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala#L403

Then figure out what method in the chain does not get applied correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants