Scala.js: Support for reflective calls. #9427
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -791,9 +791,9 @@ class JSCodeGen()(using genCtx: Context) { | |
| toIRType(param.info), mutable = false, rest = false) | ||
| } | ||
|
|
||
| if (primitives.isPrimitive(sym)) { | ||
| None | ||
| } else if (sym.is(Deferred)) { | ||
| Some(js.MethodDef(js.MemberFlags.empty, methodName, originalName, | ||
| jsParams, toIRType(patchedResultType(sym)), None)( | ||
| OptimizerHints.empty, None)) | ||
|
|
@@ -2815,9 +2815,181 @@ class JSCodeGen()(using genCtx: Context) { | |
| js.ForIn(objVarDef.ref, keyVarIdent, NoOriginalName, { | ||
| js.JSFunctionApply(fVarDef.ref, List(keyVarRef)) | ||
| })) | ||
|
|
||
| case REFLECT_SELECTABLE_SELECTDYN => | ||
| // scala.reflect.Selectable.selectDynamic | ||
| genReflectiveCall(tree, isSelectDynamic = true) | ||
| case REFLECT_SELECTABLE_APPLYDYN => | ||
| // scala.reflect.Selectable.applyDynamic | ||
| genReflectiveCall(tree, isSelectDynamic = false) | ||
| } | ||
| } | ||
|
|
||
| /** Gen the SJSIR for a reflective call. | ||
| * | ||
| * Reflective calls are calls to a structural type field or method that | ||
| * involve a reflective Selectable. They look like the following in source | ||
| * code: | ||
| * {{{ | ||
| * import scala.reflect.Selectable.reflectiveSelectable | ||
| * | ||
| * type Structural = { | ||
| * val foo: Int | ||
| * def bar(x: Int, y: String): String | ||
| * } | ||
| * | ||
| * val structural: Structural = new { | ||
| * val foo: Int = 5 | ||
| * def bar(x: Int, y: String): String = x.toString + y | ||
| * } | ||
| * | ||
| * structural.foo | ||
| * structural.bar(6, "hello") | ||
| * }}} | ||
| * | ||
| * After expansion by the Scala 3 rules for structural member selections and | ||
| * calls, they look like | ||
| * | ||
| * {{{ | ||
| * reflectiveSelectable(structural).selectDynamic("foo") | ||
| * reflectiveSelectable(structural).applyDynamic("bar", | ||
| * ClassTag(classOf[Int]), ClassTag(classOf[String]) | ||
| * )( | ||
| * 6, "hello" | ||
| * ) | ||
| * }}} | ||
| * | ||
| * and eventually reach the back-end as | ||
| * | ||
| * {{{ | ||
| * reflectiveSelectable(structural).selectDynamic("foo") // same as above | ||
| * reflectiveSelectable(structural).applyDynamic("bar", | ||
| * wrapRefArray([ ClassTag(classOf[Int]), ClassTag(classOf[String]) : ClassTag ] | ||
| * )( | ||
| * genericWrapArray([ Int.box(6), "hello" : Object ]) | ||
| * ) | ||
| * }}} | ||
| * | ||
| * If we use the deprecated `import scala.language.reflectiveCalls`, the | ||
| * wrapper for the receiver `structural` are the following instead: | ||
| * | ||
| * {{{ | ||
| * reflectiveSelectableFromLangReflectiveCalls(structural)( | ||
| * using scala.languageFeature.reflectiveCalls) | ||
| * }}} | ||
| * | ||
| * (in which case we don't care about the contextual argument). | ||
| * | ||
| * In SJSIR, they must be encoded as follows: | ||
| * | ||
| * {{{ | ||
| * structural.foo;R() | ||
| * structural.bar;I;Ljava.lang.String;R( | ||
| * Int.box(6).asInstanceOf[int], | ||
| * "hello".asInstanceOf[java.lang.String] | ||
| * ) | ||
| * }}} | ||
| * | ||
| * This means that we must deconstruct the elaborated calls to recover: | ||
| * | ||
| * - the original receiver `structural` | ||
| * - the method name as a compile-time string `foo` or `bar` | ||
| * - the `tp: Type`s that have been wrapped in `ClassTag(classOf[tp])`, as a | ||
| * compile-time List[Type], from which we'll derive `jstpe.Type`s for the | ||
| * `asInstanceOf`s and `jstpe.TypeRef`s for the `MethodName.reflectiveProxy` | ||
| * - the actual arguments as a compile-time `List[Tree]` | ||
| * | ||
| * Virtually all of the code in `genReflectiveCall` deals with recovering | ||
| * those elements. Constructing the IR Tree is the easy part after that. | ||
| */ | ||
| private def genReflectiveCall(tree: Apply, isSelectDynamic: Boolean): js.Tree = { | ||
| implicit val pos = tree.span | ||
| val Apply(fun @ Select(receiver0, _), args) = tree | ||
|
|
||
| /* Extract the real receiver, which is the first argument to one of the | ||
| * implicit conversions scala.reflect.Selectable.reflectiveSelectable or | ||
| * scala.Selectable.reflectiveSelectableFromLangReflectiveCalls. | ||
| */ | ||
| val receiver = receiver0 match { | ||
| case Apply(fun1, receiver :: _) | ||
| if fun1.symbol == jsdefn.ReflectSelectable_reflectiveSelectable || | ||
| fun1.symbol == jsdefn.Selectable_reflectiveSelectableFromLangReflectiveCalls => | ||
|
There was a problem hiding this comment. I wonder if you could make that conversion method inline to avoid having to deal with it here. There was a problem hiding this comment. I tried that initially, but if I do that, then suddenly Type Mismatch errors anywhere suggests to import that implicit conversion because it could "make progress towards a solution" ... even though the result type is monomorphically I think the suggestion mechanism assumes that any There was a problem hiding this comment.
@nicolasstucki What's the current status of whether There was a problem hiding this comment. Anyway, for this specific case, I figured it would be better not to have it |
||
| genExpr(receiver) | ||
|
|
||
| case _ => | ||
| report.error( | ||
| "The receiver of Selectable.selectDynamic or Selectable.applyDynamic " + | ||
| "must be a call to the (implicit) method scala.reflect.Selectable.reflectiveSelectable. " + | ||
| "Other uses are not supported in Scala.js.", | ||
| tree.sourcePos) | ||
| js.Undefined() | ||
| } | ||
|
|
||
| // Extract the method name as a String | ||
| val methodNameStr = args.head match { | ||
| case Literal(Constants.Constant(name: String)) => | ||
| name | ||
| case _ => | ||
| report.error( | ||
| "The method name given to Selectable.selectDynamic or Selectable.applyDynamic " + | ||
| "must be a literal string. " + | ||
| "Other uses are not supported in Scala.js.", | ||
| args.head.sourcePos) | ||
| "erroneous" | ||
| } | ||
|
|
||
| val (formalParamTypeRefs, actualArgs) = if (isSelectDynamic) { | ||
| (Nil, Nil) | ||
| } else { | ||
| // Extract the param type refs and actual args from the 2nd and 3rd argument to applyDynamic | ||
| args.tail match { | ||
| case WrapArray(classTagsArray: JavaSeqLiteral) :: WrapArray(actualArgsAnyArray: JavaSeqLiteral) :: Nil => | ||
| // Extract jstpe.Type's and jstpe.TypeRef's from the ClassTag.apply(_) trees | ||
| val formalParamTypesAndTypeRefs = classTagsArray.elems.map { | ||
| // ClassTag.apply(classOf[tp]) -> tp | ||
| case Apply(fun, Literal(const) :: Nil) | ||
| if fun.symbol == defn.ClassTagModule_apply && const.tag == Constants.ClazzTag => | ||
| toIRTypeAndTypeRef(const.typeValue) | ||
| // ClassTag.SpecialType -> erasure(SepecialType.typeRef) (e.g., ClassTag.Any -> Object) | ||
| case Apply(Select(classTagModule, name), Nil) | ||
| if classTagModule.symbol == defn.ClassTagModule && | ||
| defn.SpecialClassTagClasses.exists(_.name == name.toTypeName) => | ||
| toIRTypeAndTypeRef(TypeErasure.erasure( | ||
| defn.SpecialClassTagClasses.find(_.name == name.toTypeName).get.typeRef)) | ||
| // Anything else is invalid | ||
| case classTag => | ||
| report.error( | ||
| "The ClassTags passed to Selectable.applyDynamic must be " + | ||
| "literal ClassTag(classOf[T]) expressions " + | ||
sjrd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "(typically compiler-generated). " + | ||
| "Other uses are not supported in Scala.js.", | ||
| classTag.sourcePos) | ||
| (jstpe.AnyType, jstpe.ClassRef(jsNames.ObjectClass)) | ||
| } | ||
|
|
||
| // Gen the actual args, downcasting them to the formal param types | ||
| val actualArgs = actualArgsAnyArray.elems.zip(formalParamTypesAndTypeRefs).map { | ||
| (actualArgAny, formalParamTypeAndTypeRef) => | ||
| val genActualArgAny = genExpr(actualArgAny) | ||
| js.AsInstanceOf(genActualArgAny, formalParamTypeAndTypeRef._1)(genActualArgAny.pos) | ||
| } | ||
|
|
||
| (formalParamTypesAndTypeRefs.map(_._2), actualArgs) | ||
|
|
||
| case _ => | ||
| report.error( | ||
| "Passing the varargs of Selectable.applyDynamic with `: _*` " + | ||
| "is not supported in Scala.js.", | ||
|
Comment on lines
+2981
to
+2982
There was a problem hiding this comment. Do we have neg tests for the various error conditions in this method? There was a problem hiding this comment. No. I'm not sure yet how to write negative tests for Scala.js-specific error messages. |
||
| tree.sourcePos) | ||
| (Nil, Nil) | ||
| } | ||
| } | ||
|
|
||
| val methodName = MethodName.reflectiveProxy(methodNameStr, formalParamTypeRefs) | ||
|
|
||
| js.Apply(js.ApplyFlags.empty, receiver, js.MethodIdent(methodName), actualArgs)(jstpe.AnyType) | ||
|
There was a problem hiding this comment. Judging from the code of I do not know the details of how dotty translates fields, but it seems this is problematic for exact compatibility. There was a problem hiding this comment. On the JVM, we need to look up fields because of Java interop. A Java class such as public class A {
public int foo = 5;
}qualifies for the Scala structural type { val foo: Int }so for Java we need to try fields. In the SJSIR, however, since JDK classes are implemented in Scala anyway, I don't think there can be an issue. |
||
| } | ||
|
|
||
| /** Gen actual actual arguments to Scala method call. | ||
| * Returns a list of the transformed arguments. | ||
| * | ||
|
|
@@ -2992,8 +3164,9 @@ class JSCodeGen()(using genCtx: Context) { | |
| lazy val isWrapArray: Set[Symbol] = { | ||
| val names0 = defn.ScalaValueClasses().map(sym => nme.wrapXArray(sym.name)) | ||
| val names1 = names0 ++ Set(nme.wrapRefArray, nme.genericWrapArray) | ||
| val symsInPredef = names1.map(defn.ScalaPredefModule.requiredMethod(_)) | ||
| val symsInScalaRunTime = names1.map(defn.ScalaRuntimeModule.requiredMethod(_)) | ||
| (symsInPredef ++ symsInScalaRunTime).toSet | ||
| } | ||
|
|
||
| def unapply(tree: Apply): Option[Tree] = tree match { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a thing like
scala.reflect.Selectableseems like a neat trick to easily implement reflective calls on backends that support reflection.However, here it seems that this trick is now doing the entire compiler a disservice: We have ~100 LOC simply reconstructing things and then a single LOC actually issuing the call. (and additionally failures that IIUC simply cannot happen with reflective calls).
Have you considered introducing a form of transient tree and doing the translation to a call to
Selectablelater? (I realize that the call relies on other transformations that happen earlier, so it might be worse overall).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the situation is very inconvenient. But the desugaring that is now done in Typer is actually specced as is now. It's not "a trick". It is the normal way of compiling calls to members of structural types.
reflectiveSelectableis just one possible implicit that can provide an implementation for those, but there can be other, user-defined implementation (that don't really on Java reflection, but on an internal Map, for example).The reference is here: https://dotty.epfl.ch/docs/reference/changed-features/structural-types.html
We need to support
reflectiveSelectablebecause it corresponds to Scala 2 reflective calls, and it's the only way that our back-end would emit calls to reflective proxies (which we really need in some cases; we use them in the JDK implem in Scala.js itself for example).Users can implement other
Selectables not relying on Java run-time reflection, and those would also work.So we can't introduce a form of transient tree to be desugared later. The desugaring is by spec and requires interactions with other tasks of typechecking, including implicit resolution. It's impossible to delay that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... I see. So is the expectation that any custom
Selectablethat needs compile time reflection needs to do this ceremony? The page you linked doesn't go into details about that.Further, if that is the case, shouldn't we introduce another
Selectablefor Scala.js? The main issue I see here is that with the current specification, it would be allowed to usereflectiveSelectableon its own, without structural types. However, such use would immediately lead to non-portable code. Maybe we should make this more obvious?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this is probably a larger discussion, so you might want to go ahead with this PR. I still think it is worth thinking about this a bit more.