Skip to content

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Jun 3, 2025

No description provided.

@Ladicek Ladicek added the 2.x Issue applies to Gizmo 2.x label Jun 3, 2025
@Ladicek
Copy link
Contributor Author

Ladicek commented Jun 3, 2025

I figured if we stay with GenericType, there's at least a few things I can improve about it. Commits are best reviewed in isolation -- I tried to make them all relatively small.

@Ladicek Ladicek moved this to In Progress in WG - Gizmo 2 Jun 3, 2025
Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

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

LGTM. just curious about the usage of package-qualified types.

Ladicek added 11 commits June 4, 2025 09:12
The `setFirstBound()` method is split into two to specify more precise
parameter types (`GenericType.OfClass` or `GenericType.OfTypeVariable`).

The `setOtherBounds()` method gains an overload that doesn't accept
a `List<GenericType.OfClass>`, but a varargs `OfClass...`, to simplify
the usage.
The previous type hierarchy was:

```
TypeArgument
├─ OfAnnotated
│  ├─ OfExtends
│  ├─ OfSuper
│  └─ Wildcard
└─ OfExact
```

and included one intermediary interface:

```
OfBounded (*)
├─ OfExact
├─ OfExtends
└─ OfSuper

(*) the interface
```

This is modeled after the `Signature` type hierarchy in the ClassFile API,
which is in several aspects different to how types are treated in the JLS
and how people commonly think about them.

The new type hierarchy attempts to solve this by modelling type arguments
closely to how the JLS treats them:

```
TypeArgument
├─ OfExact
└─ OfWildcard
   ├─ OfExtends
   ├─ OfSuper
   └─ OfUnbounded
```

And includes the following hierarchy of intermediary interfaces:

```
OfTyped (*)
├─ OfExact
└─ OfBounded (*)
   ├─ OfExtends
   └─ OfSuper

(*) the interfaces
```

Further, this commit adds missing `equals()` and `hashCode()` methods
to `TypeArgument` and all its subclasses.

Finally, with this commit, the unbounded wildcard is presented as `?`
in `toString()`, instead of `*` (which is how it appears in signatures).
This method assumed that if there's no first bound, then the erasure
of the type variable is always `java.lang.Object`, but that doesn't
necessarily have to be true. It is possible for a type variable to have
no first bound, but have some other bounds -- in case all bounds are
interface types. The erasure of a type variable is the erasure of its
leftmost bound, which in this case is the first of the other bounds.
Specifically, type arguments are now separated by comma and space (`, `),
not just comma (`,`).
Some factory methods are slightly more correct now, and a few factory
methods are added too.
This test was apparently supposed to declare a class with type parameters
`S extends List<T>, T extends List<S>`, but it didn't and the asserted
signature didn't conform to that either. This commit fixes that.
The original implementation used to end up in an infinite regress in case of
recursive type variables. The new one doesn't, and it's actually simpler,
because it only computes the erasure of the type variable, which is enough
for our representation of type variables.
This is because there's a difference between type variables and type parameters:
type parameters may declare bounds. We already had that difference in our
types: `TypeVariable` vs. `GenericType.OfTypeVariable`. This rename makes that
difference more apparent and the names less confusing.

Also, `TypeVariableCreator` is renamed to `TypeParameterCreator`, and a few more
places related to that.

Finally, the `TypeParameterizedCreator.typeParameter()` method no longer returns
a `TypeParameter`; it returns `GenericType.OfTypeVariable` now. This is what can
be used directly when creating a `GenericType`.
This concerns factory methods of `GenericType`, `TypeArgument` and `TypeParameter`,
as well as `AnnotatableCreator.from()`. These methods only serve the purpose
of bridging the reflection API to Gizmo and so they do not belong to the core API.
@Ladicek Ladicek force-pushed the improve-generic-type branch from 4ca4481 to c78200e Compare June 4, 2025 07:45
@dmlloyd dmlloyd merged commit e47c8e2 into quarkusio:main Jun 4, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in WG - Gizmo 2 Jun 4, 2025
@Ladicek Ladicek deleted the improve-generic-type branch June 4, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issue applies to Gizmo 2.x
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants