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

selectDynamic doesn't work with @targetName #18922

Closed
hamzaremmal opened this issue Nov 14, 2023 · 6 comments · Fixed by #19081
Closed

selectDynamic doesn't work with @targetName #18922

hamzaremmal opened this issue Nov 14, 2023 · 6 comments · Fixed by #19081

Comments

@hamzaremmal
Copy link
Member

Compiler version

3.3.1

Minimized code

import scala.annotation.targetName

class X extends scala.reflect.Selectable:
	@targetName("hello")
	val `+` = "1"

val x = X()
x.selectDynamic("+")

Output

java.lang.ExceptionInInitializerError
	at Main$.<clinit>(main.scala:14)
	at Main.main(main.scala)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at sbt.Run.invokeMain(Run.scala:144)
	at sbt.Run.execute$1(Run.scala:94)
	at sbt.Run.$anonfun$runWithLoader$5(Run.scala:121)
	at sbt.Run$.executeSuccess(Run.scala:187)
	at sbt.Run.runWithLoader(Run.scala:121)
	at sbt.Run.run(Run.scala:128)
	at com.olegych.scastie.sbtscastie.SbtScastiePlugin$$anon$1.$anonfun$run$1(SbtScastiePlugin.scala:38)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
	at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:32)
	at sbt.ScastieTrapExit$App.run(ScastieTrapExit.scala:258)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NoSuchMethodException: Playground$X.+()
	at java.base/java.lang.Class.getMethod(Class.java:2227)
	at scala.reflect.Selectable.applyDynamic(Selectable.scala:38)
	at scala.reflect.Selectable.applyDynamic$(Selectable.scala:11)
	at Playground$X.applyDynamic(main.scala:5)
	at scala.reflect.Selectable.selectDynamic(Selectable.scala:28)
	at scala.reflect.Selectable.selectDynamic$(Selectable.scala:11)
	at Playground$X.selectDynamic(main.scala:5)
	at Playground$.<clinit>(main.scala:10)
	... 17 more

Expectation

It should resolve to the correct field independently of the presence of the @targetName annotation

@hamzaremmal hamzaremmal added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Nov 14, 2023
@hamzaremmal
Copy link
Member Author

We will work on this issue in today's Spree

@bishabosha
Copy link
Member

this is a duplicate of #18612

@bishabosha bishabosha closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2023
@bishabosha bishabosha added stat:duplicate and removed itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Nov 14, 2023
@bishabosha
Copy link
Member

bishabosha commented Nov 14, 2023

the other issue does not specifically mention @targetName, but the its the same issue - i.e. it doesnt rewrite the access to the name in the class file

@bishabosha
Copy link
Member

bishabosha commented Nov 14, 2023

as for the solution, should it be the argument to selectDynamic that is rewritten, or should the implementation of reflect.Selectable contain a lookup table from scala method name to bytecode name?

And in this situation we can't even know at all if the original argument has a target name or not

def doClose(closable: { def close(): Unit }): Unit =
  import reflect.Selectable.reflectiveSelectable
  closable.close()

leading to a crash:

class Resource:
  @scala.annotation.targetName("foo")
  def close(): Unit = println("closed")

scala> doClose(Resource())
java.lang.NoSuchMethodException: rs$line$2$Resource.close()
  at java.base/java.lang.Class.getMethod(Class.java:2395)
  at scala.reflect.Selectable.applyDynamic(Selectable.scala:38)
  at scala.reflect.Selectable.applyDynamic$(Selectable.scala:11)
  at scala.reflect.Selectable$DefaultSelectable.applyDynamic(Selectable.scala:51)
  at rs$line$1$.doClose(rs$line$1:3)

it would seem perhaps that structural types should not be allowed to conform to methods that have targetName

@hamzaremmal
Copy link
Member Author

hamzaremmal commented Nov 14, 2023

That's my point of opening this second issue. I was planning of fixing the first issue by encoding the parameter using scala.reflect.NameTransformer.encode. To fix this issue, we will need a more evolved logic and we will probably not cover it during today's spree.

@bishabosha bishabosha reopened this Nov 14, 2023
@bishabosha bishabosha added Spree Suitable for a future Spree itype:bug area:typer and removed stat:duplicate labels Nov 14, 2023
@bishabosha
Copy link
Member

bishabosha commented Nov 14, 2023

Ok I've reopened if we consider that the #18612 can be fixed with just name transformer (and not fix the wider issue in general about not knowing what exactly the bytecode name will be)

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.

2 participants