[code-reflection] RFR: Cleanup JavaType factories
Maurizio Cimadamore
mcimadamore at openjdk.org
Thu May 2 17:34:21 UTC 2024
On Thu, 2 May 2024 13:12:56 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
> This PR is a biggie cleanup of the various `JavaType` classes.
>
> Perhaps the biggest contribution of this PR is to cleanup the factories in `JavaType`. The new factories are as follows:
> * a factory to create a type from a `ClassDesc`
> * a factory to create a type from a `j.l.r.Type`
> * a couple of factories for creating parameterized types, which work on `JavaType` instances
> * a couple of factories for array types
> * a couple of factories for wildcard types
> * a factory for type variables
>
> This resolves the non-orthogonality found in the current set of factories, where one can take a `JavaType` and can, simultaneously, parameterize it, and turn it into an array type. Now the factories for these two operations are distinct.
>
> The new factories also clarify the relationship between `JavaType`, `ClassDesc` and `j.l.Type` - that is:
>
> * a type can be built from a nominal descriptor, using `type(ClassDesc)`
> * a type can turned into a nominal descriptor, using `toNominalDescriptor()`
> * a type can be built from a reflective type mirror, using `type(Type)`
> * a type can turned into a nominal descriptor, using `resolve(Lookup)`
>
> These operations are symmetric - that is, you can start from a reflective type mirror, build a `JavaType` form it, then resolve that into a `Type` and check that you get back where you started. I've enhanced `JavaTypeTest` to check round trip for both `ClassDesc` and `Type`.
>
> Note that the `JavaType` can be associated with complex reflective mirrors, e.g. beyond just `Class<?>`. For this reason, existing uses of `JavaType::resolve` in `BytecodeGenerator` and `Interpreter` which were expecting to obtain a `Class` from a `resolve` call have been rewritten to use the idiom `type.erasure().resolve(...)`.
>
> As part of this PR I've also added javadoc for all the methods/fields under the `JavaType` hierarchy that did not have one, and expanded existing docs.
>
> I've added several tests to make sure that all the conversions work as expected, and I have verified the examples (by opening their files in the IDE and make sure there's no error, I did not build).
src/java.base/share/classes/java/lang/reflect/code/type/ClassType.java line 39:
> 37: public final class ClassType implements TypeVarRef.Owner, JavaType {
> 38: // Fully qualified name
> 39: private final ClassDesc type;
Note `ClassDesc` here
src/java.base/share/classes/java/lang/reflect/code/type/ClassType.java line 138:
> 136: }
> 137:
> 138: public String toInternalName() {
I've dropped `toInternalName` as it didn't seem used and, if we expose `ClassDesc` that seems enough?
src/java.base/share/classes/java/lang/reflect/code/type/JavaType.java line 145:
> 143: }
> 144:
> 145: static JavaType type(ClassDesc d) {
I've changed name to this factory (used to be `ofNominalDescriptor`). It seemed better to have a symmetry between factories taking a `j.l.r.Type` and factories taking a `ClassDesc`. But if the old name was preferred we can keep that.
src/java.base/share/classes/java/lang/reflect/code/type/JavaType.java line 167:
> 165: }
> 166:
> 167: static ClassType parameterized(JavaType type, JavaType... typeArguments) {
Ideally this would take a `ClassType` as base type (or, even a `ClassDesc`/`Class<?>`). That would require some more changes at the use site.
src/java.base/share/classes/java/lang/reflect/code/type/MethodRef.java line 73:
> 71: // Factories
> 72:
> 73: static MethodRef method(Executable e) {
I generalized this, as we needed some way to create a `MethodRef` from a constructor, but also realizing that `MethodRef` is used for both methods and constructors, so this generalization seems to make sense (if not, I can hide the new functionality).
src/java.base/share/classes/java/lang/reflect/code/type/PrimitiveType.java line 39:
> 37: public final class PrimitiveType implements JavaType {
> 38: // Fully qualified name
> 39: private final ClassDesc type;
Note `ClassDesc` here
src/java.base/share/classes/java/lang/reflect/code/type/PrimitiveType.java line 113:
> 111: * {@return {@code true} if this type is {@link JavaType#VOID}}
> 112: */
> 113: public boolean isVoid() {
Slightly unrelated RFE.
src/java.base/share/classes/java/lang/reflect/code/type/TypeVarRef.java line 99:
> 97: @Override
> 98: public JavaType toBasicType() {
> 99: return erasure().toBasicType();
Small changes here, to generalize the behavior of these methods to type-variables, using erasure. I've added new tests for this in `TestErasure`
-------------
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587620858
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587859915
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587617661
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587623118
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587619419
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587620346
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587636824
PR Review Comment: https://git.openjdk.org/babylon/pull/71#discussion_r1587627274
More information about the babylon-dev
mailing list