[code-reflection] RFR: Replace use of MethodTypeDesc with FunctionType

Maurizio Cimadamore mcimadamore at openjdk.org
Wed Feb 14 10:22:05 UTC 2024


On Wed, 14 Feb 2024 01:15:53 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

> Replace use of `MethodTypeDesc` with `FunctionType` in the core API.
> 
> This pushes `MethodTypeDesc` more towards the reflective operation usages. There are convenience methods added to `MethodTypeDesc` for conversion/resolution between `FunctionType`. However, it may that the right approach here is convenience methods for conversion/resolution between `java.lang.invoke.MethodType` (or sequences of `Class<?>`) and `FunctionType`, which means we can likely remove `MethodTypeDesc`.

Good work. I'm looking at usages for `MethodTypeDesc`, as I believe that should no longer be needed. I can see the following:

* `CoreOps::new` use `MethodTypeDesc`. I'm not sure that's correct - I think in this cases you need a symbolic reference to the constructor being called - so that seems more a job for MethodDesc, not MethodTypeDesc? (In fact, `invoke` takes a MethodDesc already). I noticed that this asymmetry spreads to consumer of the IR (e.g. the interpreter). I think what one typically wants, with a NewOp is to get a method handle for the constructor to invoke (which suggests MethodDesc). This move might be complicated by the fact that NewOp is shared by regular instance creation expression and array creation expression (and, for arrays, Java has no symbolic references - although, we could make one up, by adding a fictional Array<> type element. Javac does something similar).

* Related: FuncCallOp does NOT take a MethodDesc, but a MethodTypeDesc. This brings up the question as to whether MethodDesc is meant to be Java-only or not (I believe you intended it to be Java-only).

* MethodDesc should use FunctionType for its type, not MethodTypeDesc

* Body::descriptor, Invokable::funcDescriptor and Op::descriptor all deal with MethodTypeDesc. Perhaps, again, the question here is, whether `FunctionType` is a general type element, or one to be used only in the Java dialect? Changing Body to have a FunctionType will likely have ripple effects in several builders in CoreOps.

* Tangent: perhaps it would be clearer if `MethodDesc` and `FieldDesc` would be called `MethodRef` and `FieldRef` ? (a "descriptor" in JVMS terms is typically just a type, not a symbolic reference). E.g. https://docs.oracle.com/javase/specs/jvms/se21/html/jvms-4.html#jvms-4.3.2 (What we want to model with these are basically CP references, not just types)

Overall, this seems doable - perhaps the bigger modelling question is whether FunctionType, MethodDesc, FieldDesc should be dialect-agnostic or not. For the latter two, I think I lean towards no. But I think FunctionType is an important concept, and one that is relatively agnostic (and so, usable directly by the more abstract op/body abstractions). Overall, I think it doesn't 100% help that all types (whether dialect-specific or not) are sqashed under the same "type" package. But I guess the same is true for "CoreOps" which has both truly core stuff (func, funcCall) and Java-specific stuff (newOp, invokeOp).

-------------

PR Comment: https://git.openjdk.org/babylon/pull/22#issuecomment-1943459885


More information about the babylon-dev mailing list