RFR: 8322292: Rearrange comparison of fields in Record.equals() [v5]

Sergey Tsypanov stsypanov at openjdk.org
Fri Dec 22 13:00:11 UTC 2023


On Thu, 21 Dec 2023 19:43:50 GMT, ExE Boss <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 224:
>> 
>>> 222:             var rt2 = mh2.type().returnType();
>>> 223:             return Integer.compare(
>>> 224:                 rt1.isPrimitive() || rt1.isEnum() || rt1.isArray() ? 1 : Iterable.class.isAssignableFrom(rt1) ? -1 : 0,
>> 
>> Doesn't this put primitives, enums and arrays at the end instead of at the start? I've tried this with a simple array:
>> 
>> Class<?>[] types = { int.class, String.class, List.class, long.class, TimeUnit.class, byte[].class, Integer.class };
>> 
>> The result of sorting:
>> 
>>  Class[7] { interface java.util.List, class java.lang.String, class java.lang.Integer, int, long, class java.util.concurrent.TimeUnit, class [B }
>> 
>> By switching the -1 and 1 I get the primitives etc.  at the start:
>> 
>>  Class[7] { int, long, class java.util.concurrent.TimeUnit, class [B, class java.lang.String, class java.lang.Integer, interface java.util.List }
>> ``
>
> The `equalator`s are joined together in reverse order, so this is actually correct for the current implementation:
> 
> 
> record Example(A a, B b, C c) {
> 	public static void main(String... args) {
> 		final var left	= new Example(new A(), new B(), new C());
> 		final var right	= new Example(new A(), new B(), new C());
> 
> 		left.equals(right);
> 		// prints:
> 		// > C::equals()
> 		// > B::equals()
> 		// > A::equals()
> 	}
> }
> 
> record A() {
> 	@Override
> 	public boolean equals(Object other) {
> 		System.out.println("A::equals()");
> 		return other instanceof A;
> 	}
> }
> 
> record B() {
> 	@Override
> 	public boolean equals(Object other) {
> 		System.out.println("B::equals()");
> 		return other instanceof B;
> 	}
> }
> 
> record C() {
> 	@Override
> 	public boolean equals(Object other) {
> 		System.out.println("C::equals()");
> 		return other instanceof C;
> 	}
> }

I've added the test case for the current algorithm

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17143#discussion_r1435034161


More information about the core-libs-dev mailing list