Skip to content

Commit ca6bdd4

Browse files
committed
Delegate null-check-then-equals to Objects.equals
Motivation is to remove avoidable synthetic branches/instructions for null data that are intepreted as uncovered code by JaCoCo etc. Bytecode is also reduced. Note that if either operand is potential `Number` or `Character`, the backend delegates to `BoxesRuntime.equals` which handles Scala semantic edge cases for boxed primitives. This PR does not change that at all. Before: ``` public boolean test1(java.lang.String, java.lang.String); descriptor: (Ljava/lang/String;Ljava/lang/String;)Z flags: (0x0001) ACC_PUBLIC Code: stack=2, locals=4, args_size=3 0: aload_1 1: aload_2 2: astore_3 3: dup 4: ifnonnull 15 7: pop 8: aload_3 9: ifnull 22 12: goto 26 15: aload_3 16: invokevirtual #20 // Method java/lang/Object.equals:(Ljava/lang/Object;)Z 19: ifeq 26 22: iconst_1 23: goto 27 26: iconst_0 27: ireturn ``` After ``` public boolean test1(java.lang.String, java.lang.String); descriptor: (Ljava/lang/String;Ljava/lang/String;)Z flags: (0x0001) ACC_PUBLIC Code: stack=2, locals=3, args_size=3 0: aload_1 1: aload_2 2: invokestatic #22 // Method java/util/Objects.equals:(Ljava/lang/Object;Ljava/lang/Object;)Z 5: ifeq 12 8: iconst_1 9: goto 13 12: iconst_0 13: ireturn ```
1 parent e19b69f commit ca6bdd4

File tree

3 files changed

+12
-30
lines changed

3 files changed

+12
-30
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,25 +1584,10 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
15841584
genCallMethod(Object_equals, InvokeStyle.Virtual, pos)
15851585
genCZJUMP(success, failure, TestOp.NE, BOOL, targetIfNoJump)
15861586
} else {
1587-
// l == r -> if (l eq null) r eq null else l.equals(r)
1588-
val eqEqTempLocal = locals.makeLocal(ObjectRef, nme.EQEQ_LOCAL_VAR.toString)
1589-
val lNull = new asm.Label
1590-
val lNonNull = new asm.Label
1591-
1587+
// l == r -> Objects.equals(l, r)
15921588
genLoad(l, ObjectRef)
15931589
genLoad(r, ObjectRef)
1594-
locals.store(eqEqTempLocal)
1595-
bc dup ObjectRef
1596-
genCZJUMP(lNull, lNonNull, TestOp.EQ, ObjectRef, targetIfNoJump = lNull)
1597-
1598-
markProgramPoint(lNull)
1599-
bc drop ObjectRef
1600-
locals.load(eqEqTempLocal)
1601-
genCZJUMP(success, failure, TestOp.EQ, ObjectRef, targetIfNoJump = lNonNull)
1602-
1603-
markProgramPoint(lNonNull)
1604-
locals.load(eqEqTempLocal)
1605-
genCallMethod(Object_equals, InvokeStyle.Virtual, pos)
1590+
genCallMethod(Objects_equals, InvokeStyle.Static, pos)
16061591
genCZJUMP(success, failure, TestOp.NE, BOOL, targetIfNoJump)
16071592
}
16081593
}

src/reflect/scala/reflect/internal/Definitions.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,10 @@ trait Definitions extends api.StandardDefinitions {
290290
}
291291

292292
// top types
293-
lazy val AnyClass = enterNewClass(ScalaPackageClass, tpnme.Any, Nil, ABSTRACT).markAllCompleted()
294-
lazy val AnyRefClass = newAlias(ScalaPackageClass, tpnme.AnyRef, ObjectTpe).markAllCompleted()
295-
lazy val ObjectClass = getRequiredClass("java.lang.Object")
293+
lazy val AnyClass = enterNewClass(ScalaPackageClass, tpnme.Any, Nil, ABSTRACT).markAllCompleted()
294+
lazy val AnyRefClass = newAlias(ScalaPackageClass, tpnme.AnyRef, ObjectTpe).markAllCompleted()
295+
lazy val ObjectClass = getRequiredClass("java.lang.Object")
296+
lazy val ObjectsModule = getRequiredModule("java.util.Objects")
296297

297298
// Cached types for core monomorphic classes
298299
lazy val AnyRefTpe = AnyRefClass.tpe
@@ -1285,6 +1286,7 @@ trait Definitions extends api.StandardDefinitions {
12851286
def Object_equals = getMemberMethod(ObjectClass, nme.equals_)
12861287
def Object_hashCode = getMemberMethod(ObjectClass, nme.hashCode_)
12871288
def Object_toString = getMemberMethod(ObjectClass, nme.toString_)
1289+
def Objects_equals = getMemberMethod(ObjectsModule, nme.equals_)
12881290

12891291
// boxed classes
12901292
lazy val ObjectRefClass = requiredClass[scala.runtime.ObjectRef[_]]

test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,11 @@ class BytecodeTest extends BytecodeTesting {
106106
Label(7), Op(ICONST_2), Op(IRETURN)))
107107

108108
// t3: Array == is translated to reference equality, AnyRef == to null checks and equals
109-
assertSameCode(getMethod(c, "t3"), List(
110-
// Array ==
111-
VarOp(ALOAD, 1), VarOp(ALOAD, 2), Jump(IF_ACMPEQ, Label(23)),
112-
// AnyRef ==
113-
VarOp(ALOAD, 2), VarOp(ALOAD, 1), VarOp(ASTORE, 3), Op(DUP), Jump(IFNONNULL, Label(14)),
114-
Op(POP), VarOp(ALOAD, 3), Jump(IFNULL, Label(19)), Jump(GOTO, Label(23)),
115-
Label(14), VarOp(ALOAD, 3), InvokeVirtual("java/lang/Object", "equals", "(Ljava/lang/Object;)Z"), Jump(IFEQ, Label(23)),
116-
Label(19), Op(ICONST_1), Jump(GOTO, Label(26)),
117-
Label(23), Op(ICONST_0),
118-
Label(26), Op(IRETURN)))
109+
assertSameCode(getMethod(c, "t3"), List(
110+
VarOp(ALOAD, 1), VarOp(ALOAD, 2), Jump(IF_ACMPEQ, Label(11)),
111+
VarOp(ALOAD, 2), VarOp(ALOAD, 1),
112+
Invoke(INVOKESTATIC, "java/util/Objects", "equals", "(Ljava/lang/Object;Ljava/lang/Object;)Z", itf = false),
113+
Jump(IFEQ, Label(11)), Op(ICONST_1), Jump(GOTO, Label(14)), Label(11), Op(ICONST_0), Label(14), Op(IRETURN)))
119114

120115
val t4t5 = List(
121116
VarOp(ALOAD, 1), Jump(IFNULL, Label(6)),

0 commit comments

Comments
 (0)