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

Support reflective calls for structural refinements of js.Any types #4886

Open
steinybot opened this issue Jul 27, 2023 · 6 comments
Open

Support reflective calls for structural refinements of js.Any types #4886

steinybot opened this issue Jul 27, 2023 · 6 comments
Labels
language Affects language semantics.

Comments

@steinybot
Copy link

steinybot commented Jul 27, 2023

I've read the Reflexive calls section but something feels like it is still missing.

My understanding is that for the example:

import scala.scalajs.js

type T = { def foo(x: Int): String }
def print(obj: T) = obj.foo(100)

object Foo extends js.Object {
  def foo(x: Int): String = x.toString
}

print(Foo)

Will fail at runtime with:

TypeError: $n(...).foo__I__ is not a function

However with some indirection we can do:

import scala.scalajs.js

type T = js.Object { def foo(x: Int): String }
def print(obj: T) = {
  obj.asInstanceOf[JsFoo].foo(100)
}

object Foo extends js.Object {
  def foo(x: Int): String = x.toString
}

@js.native
trait JsFoo extends js.Object {
  def foo(x: Int): String = js.native
}

print(Foo)

Which will succeed at runtime.

It seems to me like we ought to be able to have reflexive calls work provided that the structural refinement is on a js.Any (like T in this last example).

We don't need to generate the forwarder methods, we can treat it exactly the same as we would for a native JS trait, i.e. no name translation.

It of course relies on you getting the names in the structural refinement correct but that is the same with native JS traits. The one limitation is that you can't use a @JSName annotation within the structural refinement but I don't think that should be a blocker.

In short, I think this should work:

import scala.scalajs.js

type T = js.Object { def foo(x: Int): String }
def print(obj: T) = obj.foo(100)

object Foo extends js.Object {
  def foo(x: Int): String = x.toString
}

print(Foo)
@gzm0 gzm0 added the language Affects language semantics. label Jul 30, 2023
@sjrd
Copy link
Member

sjrd commented Jul 31, 2023

This was of course considered many years ago, when we faced the issue of reflective calls for JS objects. Besides the problem of @JSName (which is not even checkable as far we know), there are other issues.

If we support it for js.Object { ... }, we have to support it for Any { ... }, because it's a supertype, and we cannot check that this particular upcast is not used anywhere. For Any, which is the common case, we would have to dynamically type-test whether the receiver is a Scala.js object or not. That is doable although inconvenient. Moreover, when it is a Scala.js object, it might actually be used as a JS object, thanks to its @JSExported members. But those can also use alternative names, and we would not know whether it's the alternative JS name that we must use or the original Scala name.

The indirection you write by hand solves this problem, because you tell the compiler in advance which one of the schemes it's supposed to use.

@sjrd sjrd changed the title Support reflexive calls for structural refinements of js.Any types Support reflective calls for structural refinements of js.Any types Jul 31, 2023
@steinybot
Copy link
Author

steinybot commented Jul 31, 2023

Why do we need to dynamically type-test the receiver? I was naively thinking that we would look at the definition of foo and if it is a structural refinement on js.Any then the name would not be translated.

Is this not similar to how native JS traits work? I was presuming in that case we look at foo and see that it is defined on a JS native trait and not do a translation. Surely for native JS traits we are looking at the defintion to find @JSName annotations, not the receiver.

Why can't we limit this to JS refinements? In other words we check that the component type preceeding the refinement has an upper bound of js.Any.

@sjrd
Copy link
Member

sjrd commented Jul 31, 2023

Because you can assign js.Any { def foo: Int } to Any { def foo: Int }. To preserve the LSP, everything that works with js.Any { def foo: Int } must work with Any { def foo: Int }.

@steinybot
Copy link
Author

I think I get what you are saying now. The thing is that LSP is alread being violated with the current implementation.

In the original example of:

import scala.scalajs.js

type T = { def foo(x: Int): String }
def print(obj: T) = obj.foo(100)

object Foo extends js.Object {
  def foo(x: Int): String = x.toString
}

print(Foo)

Foo is assignable to T but you cannot use it like a T.

A more complete example showing the various cases:

import scala.language.reflectiveCalls
import scala.scalajs.js

type ScalaRefinement = { def foo(x: Int): String }

type JsRefinement = js.Any { def foo(x: Int): String }

def printScalaRefinement(obj: ScalaRefinement) = {
  obj.foo(100)
}

def printJsRefinement(obj: JsRefinement) = {
  // At runtime looks for the function foo__I__ which will fail for any JS type.
  // This only allows JS types so it will fail for any input.
  obj.foo(100)
}

object JsFoo extends js.Object {
  def foo(x: Int): String = x.toString
}

object ScalaFoo {
  def foo(x: Int): String = x.toString
}

// Compiles but fails at runtime. Violates LSP.
printScalaRefinement(JsFoo)
//printJsRefinement(JsFoo)

// Succeeds. Upholds LSP.
printScalaRefinement(ScalaFoo)

// Fails to compile. Upholds LSP.
//printJsRefinement(ScalaFoo)

My suggestion is that only for the case of printJsRefinement, if we are accessing a member of a JS refinement then we use JS semantics and not Scala semantics.

This would improve the situation with regards to LSP as you can see here:

import scala.language.reflectiveCalls
import scala.scalajs.js

type ScalaRefinement = { def foo(x: Int): String }

type JsRefinement = js.Any { def foo(x: Int): String }

trait JsTrait extends js.Any { def foo(x: Int): String }

def printScalaRefinement(obj: ScalaRefinement) = {
  obj.foo(100)
}

def printJsRefinement(obj: JsRefinement) = {
  // Pretend that the Scala.js compiler was able to look at obj.foo and see that
  // it is selecting a term from a JS refinement and apply JS semantics instead.
  // That is, no name translation as if it did the following:
  obj.asInstanceOf[JsTrait].foo(100)
}

object JsFoo extends js.Object {
  def foo(x: Int): String = x.toString
}

object ScalaFoo {
  def foo(x: Int): String = x.toString
}

// Compiles but fails at runtime. Violates LSP.
//printScalaRefinement(JsFoo)

// Succeeds. Upholds LSP.
printScalaRefinement(ScalaFoo)
printJsRefinement(JsFoo)

// Fails to compile. Upholds LSP.
//printJsRefinement(ScalaFoo)

So yeah it isn't going to improve the case of printScalaRefinement(JsFoo) but it is no worse. It at least makes refinements possible for JS types which would be extremely useful in so many situations.

@sjrd
Copy link
Member

sjrd commented Aug 4, 2023

I realized this morning that this can be implemented in user-space in Scala 3:
https://scastie.scala-lang.org/sjrd/y1687V7HQUG8PUrTEhiwQw/20

object JSReflectiveCalls:
  import scala.language.implicitConversions

  final class Wrapper(private val x: js.Any) extends AnyVal with Selectable:
    @inline def selectDynamic(propName: String): Any =
      x.asInstanceOf[js.Dynamic].selectDynamic(propName)
    @inline def applyDynamic(propName: String)(args: Any*): Any =
      x.asInstanceOf[js.Dynamic].applyDynamic(propName)(args.asInstanceOf[Seq[js.Any]]*)
  end Wrapper

  implicit def toJSReflectiveCalls(x: js.Any): Wrapper =
    new Wrapper(x)
end JSReflectiveCalls

import JSReflectiveCalls.toJSReflectiveCalls

type T = js.Any { def foo(x: Int): String; val bar: Int }

def print(obj: T): String = obj.foo(100 + obj.bar)

object Foo extends js.Object {
  def foo(x: Int): String = x.toString
  val bar: Int = 5
}

print(Foo)

@gzm0
Copy link
Contributor

gzm0 commented Aug 6, 2023

I've been thinking about this, and the only reason I see not to add better support for js.Any refined reflective calls is if we cannot make it calling convention annotation (@JSName et al.) safe.

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

No branches or pull requests

3 participants