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

Incorrect quotes reflection api flags set on Java symbols #20052

Open
jchyb opened this issue Mar 29, 2024 · 2 comments · May be fixed by #22104
Open

Incorrect quotes reflection api flags set on Java symbols #20052

jchyb opened this issue Mar 29, 2024 · 2 comments · May be fixed by #22104
Assignees
Labels
area:metaprogramming:reflection Issues related to the quotes reflection API compat:java itype:bug

Comments

@jchyb
Copy link
Contributor

jchyb commented Mar 29, 2024

Compiler version

3.3.3, 3.4.0, c8c3bde (main)

Minimized code

Main.scala:

@main def main() =
  Repro[JavaClass]

Macro.scala

import scala.quoted.*

object Repro {
  inline def apply[A]: Unit = ${ applyImpl[A] }

  def applyImpl[A](using Type[A], Quotes): Expr[Unit] = {
    import quotes.*, quotes.reflect.*
    println(TypeRepr.of[A].show + " " +  TypeRepr.of[A].typeSymbol.primaryConstructor.flags.show)
    '{ () }
  }
}

JavaClass.java:

public class JavaClass {}

Output

JavaClass Flags.JavaDefined | Flags.Local | Flags.Method | Flags.Private | Flags.PrivateLocal

Expectation

The Flags.Private and Flags.PrivateLocal should not be a part of the flagset of a public constructor method
Related: scalalandio/chimney#484 (comment)

@jchyb jchyb added itype:bug compat:java area:metaprogramming:reflection Issues related to the quotes reflection API labels Mar 29, 2024
@jchyb
Copy link
Contributor Author

jchyb commented Jul 23, 2024

The issue lies in our java outline parser, which always produces a dummy constructor:

package <empty> {
  object JavaClass() {}
  class JavaClass private[this](x$1: scala.Unit) extends Object {
    def <init>() = _root_.scala.Predef.???
  }
}

The method is the actual constructor found by the parser (so for this case it's public, if we were to add a private java constructor it would become private etc.), but there is no way to locate it with the quotes.reflect methods.
In scala 2 no such system was used, so after parsing we would get:

package example {
  object JavaClass extends  {
    def <init>()
  };
  class JavaClass extends _root_.java.lang.Object {
    def <init>()
  }
}

and primaryConstructor would return the correct method. In cases where there are more public constructors it would return the first declared one.

Another issue here is the fact that if we split compilations into 2 runs, I believe we read the java class file instead of using our source outline parser, which results in obtaining the correct primaryConstructor.

Perhaps we should leave the JavaParser as is and add a quotes reflect method to return all constructors of a java class?

@MateuszKubuszok
Copy link

There was some issue with Scala 2 primaryConstructor and Java as well - https://github.com/scala/scala/blob/d76229ef6f6b2e79422d5990ba2deb4aed340277/src/reflect/scala/reflect/api/Symbols.scala#L960 - so I am not surprised.

Ideally, this behavior would be fixed, but if it cannot, I'd hope that the proper workaround could be documented, ideally in APIs documentation with some code examples. For Scala 2/Scala 3 cross-compilable macros I'd like to be able to align the behavior or at least be able to document the inconsistencies and their sources.

E.g. if it was possible to:

  • verify that it's a Java class (Flags.JavaDefined?)
  • always get a list of all constructors (sym.declarations.filter(_.isConstructor)?)
  • sort them by position and assume that the first one is primary (kinda sketchy but at least deterministic - Ordering.Option(Ordering.fromLessThan[Position] { (a, b) => a.startLine < b.startLine || (a.startLine == b.startLine && a.startColumn < b.startColumn) }))

it would be... something. For classes with a single, default constructors (like most beans) it should be basically deterministic behavior, for classes with multiple-constructors... well, we there could be some best effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metaprogramming:reflection Issues related to the quotes reflection API compat:java itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants