RFR: 8352869: Verify.checkEQ: extension for NaN, VectorAPI and arbitrary Objects [v3]

[Emanuel Peter]

On Fri, 28 Mar 2025 05:01:29 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update test/hotspot/jtreg/compiler/lib/verify/Verify.java
>>   
>>   Co-authored-by: Galder Zamarreño <galder at redhat.com>
>
> test/hotspot/jtreg/compiler/lib/verify/Verify.java line 79:
> 
>> 77:      * @throws VerifyException If the comparison fails.
>> 78:      */
>> 79:     public static void checkEQ(Object a, Object b, boolean isFloatCheckWithRawBits, boolean isCheckWithArbitraryClasses) {
> 
> Just a suggestion. One boolean might be ok, but once you start adding 2 booleans it seems like a bit of a code smell to me. Do you envision more options being added? I would personally create a `VerifyOptions` record with the boolean flags options and pass that in.

I'll experiment with a `VerifyOptions`, thanks for the suggestion!

> test/hotspot/jtreg/testlibrary_tests/verify/tests/TestVerify.java line 691:
> 
>> 689:     }
>> 690: 
>> 691:     public static void checkNE(Object a, Object b) {
> 
> If `checkEQ` lives in Verify, wouldn't it make sense to also have `checkNE` there? Seems like the natural place making the API symmetric.

To be honest, I don't see the need for it being symmetric, i.e. for the API to have a `checkNE`. This `checkNE` method is just a convenience function to check that an exception is thrown if I expect it to.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2018254687
PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2018253409