Skip to content
This repository was archived by the owner on Jul 12, 2024. It is now read-only.

Basic support of reflective calls #89

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Basic support of reflective calls #89

merged 1 commit into from
Apr 11, 2024

Conversation

tanishiking
Copy link
Owner

@tanishiking tanishiking commented Apr 10, 2024

#71

Added a reflectiveProxies field to typeData, this field contains an
array of struct with methodId of i32, and a reference to a
reflective proxy function.
When the compiler found an Apply to the reflective proxy, we lookup the
method to call from the reflectiveProxies field in the receiver's
typeData field, instead of normal table dispatching.


TODO (maybe in another PR)

(func $f#java.lang.Void#clone_R (type $f.21)
   (param $___<this> (ref any)) (result anyref)
   local.get $___<this>
   call $f#java.lang.Void#clone_Ljava.lang.Object)
  • java.lang.Object#notify/notifyAll
  • and more... (see ReflectiveCallTest)

Comment on lines +736 to +710
// TODO: investigate what's going on
// Don't know why, but it seems that `this` isn't boxed even if
// t.receiver.tpe = ClassType(ClassName<java.lang.Boolean>), and
// primReceiverType = BooleanType
// This wired patch is required for `(func $f#java.lang.Boolean#compareTo_Ljava.lang.Boolean_R`
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, validation fails because of the following wasm code

  (func $f#java.lang.Boolean#compareTo_Ljava.lang.Boolean_R (type $f.268)
     (param $___<this> i32) (param $that anyref) (result anyref)
    
     local.get $___<this>
     ;; <this> is already unboxed
     ref.as_non_null
     call $__scalaJSHelpers#uZ
     local.get $that
     call $f#java.lang.Boolean#compareTo_Ljava.lang.Boolean_I
     call $__scalaJSHelpers#bI)

🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum that's weird indeed. In the ClassDef of a hijacked class, the tpe of This() trees should always be the primitive. So here I would expect t.receiver.tpe == BooleanType, and therefore we should enter the first if (t.receiver.tpe == primReceiverType).

@sjrd sjrd linked an issue Apr 10, 2024 that may be closed by this pull request
@tanishiking tanishiking force-pushed the reflective-call branch 2 times, most recently from c25f493 to 6b20614 Compare April 11, 2024 08:12
Added a `reflectiveProxies` field to `typeData`, this field contains an
array of struct with `methodId` of `i32`, and a reference to a
reflective proxy function.
When the compiler found an Apply to the reflective proxy, we lookup the
method to call from the `reflectiveProxies` field in the receiver's
`typeData` field, instead of normal table dispatching.
@tanishiking tanishiking changed the title WIP: Support reflective call Basic support of reflective calls Apr 11, 2024
@tanishiking tanishiking marked this pull request as ready for review April 11, 2024 15:50
Copy link
Collaborator

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Good stuff.

There's one issue but it is pretty niche. We can fix it later.

@@ -47,6 +47,17 @@ object TypeTransformer {
case _ => List(transformType(t))
}

def transformTypeRef(t: IRTypes.TypeRef)(implicit ctx: ReadOnlyWasmContext): Types.WasmType =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't be correct for ClassRefs of JS types. We need to use

transformType(ctx.inferTypeFromTypeRef(typeRef))

instead.

Note that this is not a problem in transformType because a ClassType(someJSType) is not valid in the first place. But ClassRef(someJSType) is valid; its corresponding type is AnyType.

@sjrd sjrd merged commit 782d6ca into main Apr 11, 2024
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reflective calls crash the linker backend
2 participants