[code-reflection] RFR: Refactor JavaType [v4]

Paul Sandoz psandoz at openjdk.org
Tue Apr 9 15:30:16 UTC 2024


On Tue, 9 Apr 2024 13:16:47 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR breaks up the monolithic JavaType interface into 3 classes:
>> 
>> * ClassType - used to model class types (e.g. `String`, `List<Integer>`)
>> * PrimitiveType - used to model primitive types (`int` and friends) - this one is package-private
>> * ArrayType - used to model array types (`int[]`, `List<Integer>[][]`)
>> 
>> I have retained `JavaType` at the top of the hierarchy - and then added three sealed subclasses.
>> 
>> While we discussed (offline) about the possibility of having a toplevel array type that was not a `JavaType`, I quickly realized that doing so would have required far deeper changes. For instance, the factory for creating a parameterized type takes a bunch of `JavaType` as type arguments, but if we want to model a type such as `List<int[]>`, then the mismatch `TypeElement` vs. `JavaType` becomes annoying (this was quite visible when tweaking the javac code). For this reason, I left `ArrayType` under the `JavaType` umbrella, for now.
>> 
>> I'm still not 100% happy about the way in which types are constructed. We now have the following factories in `JavaType`:
>> 
>> * `type(Class<?>)` - create a class type w/o type parameters
>> * `type(Class<?>, Class<?>[])`/`type(Class<?>, List<Class<?>>)` - create a class type w/ type parameters
>> * `type(JavaType, JavaType[])`/`type(JavaType, List<JavaType>)` - create a class type w/ type parameters
>> * `array(JavaType)` - create an array type with given component type (and dimension = 1)
>> * `array(JavaType, int)` - create an array type with given component type and dimension
>> * `ofNominalDescriptor(ClassDesc)` - create a `JavaType` from a symbolic desc
>> * `ofNominalDescriptorString(String)` - create a `JavaType` from a descriptor string
>> * `ofString(String)` - create a `JavaType` from a (model) string
>> * for primitive types, there are static constants in `JavaType` (e.g. `JavaType::INT`)
>> 
>> 
>> When rewriting the existing code to use the correct factories, I have to admit that I got myself confused several times, and ended up e.g. using `ofNominalDescriptor` in places where `ofString` was required.
>> 
>> My feeling is that we would be better off with something like this:
>> 
>> * have a way to create a `JavaType` from a `Class<?>`/`ClassDesc`/descriptor string
>> * have a single factory to create parameterized types (expressed in terms of `JavaType`)
>> * keep existing array factories, but perhaps investigate if we can get rid of the "flat" array factory that takes the dimensions (I don't see many uses...
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments

Marked as reviewed by psandoz (Lead).

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

PR Review: https://git.openjdk.org/babylon/pull/46#pullrequestreview-1989394057


More information about the babylon-dev mailing list