Skip to content

reflect.Selectable does not mangle operator names #4528

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

Open
Blaisorblade opened this issue May 14, 2018 · 12 comments
Open

reflect.Selectable does not mangle operator names #4528

Blaisorblade opened this issue May 14, 2018 · 12 comments

Comments

@Blaisorblade
Copy link
Contributor

Blaisorblade commented May 14, 2018

scala>     type T = {val a: Int; def a_=(x: Int): Unit}
// defined alias type T = Object{a: Int; a_=(x: Int): Unit}

scala> import scala.reflect.Selectable.reflectiveSelectable

scala> (new {var a = 10}: T).a = 11
java.lang.NoSuchMethodException: rs$line$3$$anon$1.a_=(int)
	at java.lang.Class.getMethod(Class.java:1786)
	at scala.reflect.Selectable$.selectDynamicMethod$extension(Selectable.scala:20)
	at scala.reflect.Selectable.selectDynamicMethod(Selectable.scala:17)
	at rs$line$3$.<init>(rs$line$3:1)
	at rs$line$3$.<clinit>(rs$line$3)
	at rs$line$3.res0(rs$line$3)

Haven't tried other characters.

Decompiling a bigger example reminds us that the generated method is called a_$eq and not a_=, while the generated code indeed tries to call a_= (per jad decompilation):

        ((Function1)Selectable$.MODULE$.reflectiveSelectable(v).selectDynamicMethod("a_=", Predef$.MODULE$.wrapRefArray(new ClassTag[] {
            ClassTag$.MODULE$.apply(Integer.TYPE)
        }))).apply(BoxesRunTime.boxToInteger(11));

Since structural types are intended to be mapped also to other backends (see applications to databases in #1886), it's not clear the encoding should be done by the compiler; we might want to mangle the name inside reflectiveSelectable.

EDIT: Discovered while testing fix to #4496 and checking all variants.

@smarter
Copy link
Member

smarter commented May 14, 2018

@sjrd How does Scala.js name mangling interact with scala.Dynamic ?

@sjrd
Copy link
Member

sjrd commented May 14, 2018

Badly.

In scalac, even calls to scala.Dynamic members are mangled before they are turned to strings. So in the back-end we decode it, but it can lead to false positives, where the user wrote obj.$plus$minus but the compiler will emit `obj['+-']. For example: https://scalafiddle.io/sf/zt1UmSh/0

@smarter
Copy link
Member

smarter commented May 14, 2018

Ouch! So it'd make sense to follow @Blaisorblade suggestion and move the mangling to library code, since Scala.js can overwrite the library code if needed?

@sjrd
Copy link
Member

sjrd commented May 14, 2018

I think so, yes.

@smarter
Copy link
Member

smarter commented May 14, 2018

OK, that means something like https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/util/NameTransformer.scala (but that operates on String instead of SimpleName) needs to be moved to dotty-library.

@Blaisorblade
Copy link
Contributor Author

@sjrd BTW, does it make sense to keep writes to a as calls to a_=, or should we have a separate method for writes? I can imagine a scenario where writes and calls to a_= are distinct things and you run into similar problems. Tho it's harder to imagine a scenario where v.a = 5 becomes a "proper write" and v.a_=(5) a method call; that depends on how early writes become setter calls.

@sjrd
Copy link
Member

sjrd commented May 15, 2018

Writes to properties are supposed to call the updateDynamic("a")(rhs), indeed!

@bishabosha bishabosha changed the title Reflective access to methods with mangled characters fails at runtime reflect.Selectable does not mangle operator names Mar 29, 2021
@bishabosha
Copy link
Member

bishabosha commented Mar 29, 2021

this is further complicated by @targetName, but then should only the reflection implementation be aware of these details?, and not some other implementation for Selectable?

@odersky
Copy link
Contributor

odersky commented Jan 23, 2022

I have the impression it's too late to change anything here now.

@odersky odersky closed this as completed Jan 23, 2022
@bishabosha
Copy link
Member

bishabosha commented Jan 24, 2022

I have the impression it's too late to change anything here now.

May I ask why?

@odersky
Copy link
Contributor

odersky commented Jan 24, 2022

I thought the issue would apply to all selectables, where changing the encoding might break existing user-defined implementations. But I see this is just reflect.Selectable, so OK. I was fooled by the needs-spec label. Can someone explain why this needs a spec?

Also I should not be the one assigned to this, I think.

@wjoel
Copy link
Contributor

wjoel commented Aug 30, 2024

FWIW, on Scala 3.5.0 the generated code does call a_$eq

//> using scala 3.5.0

type T = {val a: Int; def a_=(x: Int): Unit}
import scala.reflect.Selectable.reflectiveSelectable

@main def run() =
  (new {var a = 10}.asInstanceOf[T]).a = 11

Generates:

      10: invokevirtual #38                 // Method scala/reflect/Selectable$.reflectiveSelectable:(Ljava/lang/Object;)Lscala/reflect/Selectable;
      13: ldc           #40                 // String a_$eq
      15: getstatic     #45                 // Field scala/runtime/ScalaRunTime$.MODULE$:Lscala/runtime/ScalaRunTime$;

Full output:

Compiled from "Main.scala"
public final class Main$package$ implements java.io.Serializable {
  public static final Main$package$ MODULE$;

  public static {};
    Code:
       0: new           #2                  // class Main$package$
       3: dup
       4: invokespecial #18                 // Method "<init>":()V
       7: putstatic     #20                 // Field MODULE$:LMain$package$;
      10: return

  public void run();
    Code:
       0: getstatic     #33                 // Field scala/reflect/Selectable$.MODULE$:Lscala/reflect/Selectable$;
       3: new           #9                  // class Main$package$$anon$1
       6: dup
       7: invokespecial #34                 // Method Main$package$$anon$1."<init>":()V
      10: invokevirtual #38                 // Method scala/reflect/Selectable$.reflectiveSelectable:(Ljava/lang/Object;)Lscala/reflect/Selectable;
      13: ldc           #40                 // String a_$eq
      15: getstatic     #45                 // Field scala/runtime/ScalaRunTime$.MODULE$:Lscala/runtime/ScalaRunTime$;
      18: iconst_1
      19: anewarray     #47                 // class java/lang/Class
      22: dup
      23: iconst_0
      24: getstatic     #53                 // Field java/lang/Integer.TYPE:Ljava/lang/Class;
      27: aastore
      28: invokevirtual #57                 // Method scala/runtime/ScalaRunTime$.wrapRefArray:([Ljava/lang/Object;)Lscala/collection/immutable/ArraySeq;
      31: getstatic     #45                 // Field scala/runtime/ScalaRunTime$.MODULE$:Lscala/runtime/ScalaRunTime$;
      34: iconst_1
      35: anewarray     #4                  // class java/lang/Object
      38: dup
      39: iconst_0
      40: bipush        11
      42: invokestatic  #63                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
      45: aastore
      46: invokevirtual #67                 // Method scala/runtime/ScalaRunTime$.genericWrapArray:(Ljava/lang/Object;)Lscala/collection/immutable/ArraySeq;
      49: invokeinterface #73,  4           // InterfaceMethod scala/reflect/Selectable.applyDynamic:(Ljava/lang/String;Lscala/collection/immutable/Seq;Lscala/collection/immutable/Seq;)Ljava/lang/Object;
      54: pop
      55: return
}

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

No branches or pull requests

6 participants