Skip to content

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Mar 31, 2025

This is an equivalent of EqualsHashCodeToStringGenerator from Gizmo 1.x.

Resolves #258

@Ladicek Ladicek added the 2.x Issue applies to Gizmo 2.x label Mar 31, 2025
@Ladicek Ladicek requested review from dmlloyd and mkouba March 31, 2025 12:35
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 31, 2025

Commits are supposed to be reviewed in isolation.

@Ladicek Ladicek linked an issue Mar 31, 2025 that may be closed by this pull request
@Ladicek Ladicek force-pushed the generate-equals-hashcode-tostring-2.x branch from f132ca1 to a8a2eb1 Compare April 1, 2025 15:21
@Ladicek
Copy link
Contributor Author

Ladicek commented Apr 1, 2025

I think I addressed all the comments here, except of the ifNotInstanceOf / unlessInstanceOf idea. Any more comments about that? :-)

@Ladicek Ladicek force-pushed the generate-equals-hashcode-tostring-2.x branch from a8a2eb1 to dd034ac Compare April 1, 2025 15:24
@Ladicek
Copy link
Contributor Author

Ladicek commented Apr 1, 2025

Just to be sure: I simplified the equals/hashCode/toString generator by calling BlockCreator.exprEquals/exprHashCode/exprToString/arrayEquals/arrayHashCode/arrayToString (where the last two are new). I think I stroke reasonable balance there, but I'm open to different ideas.


LocalVar result = b0.define("result", Constant.of(1));
for (FieldDesc field : fields) {
if (!field.owner().equals(thisClass)) {
Copy link
Member

Choose a reason for hiding this comment

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

You could conceivably include fields of a superclass, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't think about that, but you're right. I'm not really keen on removing the check, but I don't see a better way...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I'm thinking handling superclasses is out of scope, at least for now. The best way to handle superclasses would be to call equals() (or hashCode(); not sure about toString()) on the superclass first, and then handle the fields of this class.

@Ladicek Ladicek force-pushed the generate-equals-hashcode-tostring-2.x branch 2 times, most recently from 185f218 to a69f993 Compare April 2, 2025 11:43
Ladicek added 12 commits April 2, 2025 17:49
Also, a few more validation methods were added.
The result type of the `InstanceOf` instruction is always `boolean`.
It is _not_ the type of the argument.
This commit fixes code emission for the `Neg` instruction to also work
with `byte`, `short` and `char` types.

Argument validation was also added to the constructor, and the `Neg.writeCode()`
method uses `impossibleSwitchCase()` to throw an error.
Use `impossibleSwitchCase()` in `Rel.writeCode()`.
This is mainly for discoverability in an IDE. Java has the `if` keyword,
so presumably users will type `if` and expect to see the whole range
of conditional generating methods. `unless()` doesn't show there, while
`ifNot()` does.
The `ifNotInstanceOf()` method complements `ifInstanceOf()` in case
the caller needs to do something when the given object is _not_
an instance of given type.
Fields don't always have to be created using class types; primitive
types or array types are perfectly legal. So we need to correctly
initialize the `Signature`, which isn't always a `ClassTypeSig`.
Fortunately, `Signature` has the right factory method to create
a signature out of a `ClassDesc`.
For now, we only remember descriptors of added members (instance/static
fields, instance/static methods, constructors) and provide public access
to them.

In the future, we might want to remember all added creators, to be able
to prevent adding the same member multiple times, but even in that case,
we should still only expose access to descriptors.
First off, `arrayHashCode()` and `arrayToString()` methods were added
to complement their `expr*` counterparts.

The `expr*` methods never look for multidimensional arrays; they treat
arrays as other reference types.

The `array*` methods, on the other hand, always look for multidimensional
arrays and use the proper `Arrays.deep*` methods if necessary.
This is an equivalent of `EqualsHashCodeToStringGenerator` from Gizmo 1.x.
@Ladicek Ladicek force-pushed the generate-equals-hashcode-tostring-2.x branch from a69f993 to a3df338 Compare April 2, 2025 15:49
@dmlloyd dmlloyd merged commit f4bb006 into quarkusio:2.x Apr 2, 2025
2 checks passed
@Ladicek Ladicek deleted the generate-equals-hashcode-tostring-2.x branch April 2, 2025 16:05
@Ladicek Ladicek moved this to Done in WG - Gizmo 2 Apr 7, 2025
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.

Gizmo2: port EqualsHashCodeToStringGenerator
3 participants