Support unboxing enums used in abstract methods
- Fix issue with overrides and overloads and enum unboxing
Bug: b/171784168
Change-Id: Idf38d490d306ad0e6e54487f2735c74d0d7ad95a
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
index ee9878a..12e1f1e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
@@ -1333,76 +1333,102 @@
}
return Reason.INVALID_INVOKE_ON_ARRAY;
}
- DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, code.context());
- if (singleTarget == null) {
+
+ DexClassAndMethod resolvedMethod =
+ appView
+ .appInfo()
+ .resolveMethod(invoke.getInvokedMethod(), invoke.getInterfaceBit())
+ .getResolutionPair();
+ if (resolvedMethod == null) {
return Reason.INVALID_INVOKE;
}
- DexMethod singleTargetReference = singleTarget.getReference();
- DexClass targetHolder = singleTarget.getHolder();
- if (targetHolder.isProgramClass()) {
- if (targetHolder.isEnum() && singleTarget.getDefinition().isInstanceInitializer()) {
+ // The single target may be null if for example this is a virtual invoke into an abstract
+ // method.
+ DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, code.context());
+ DexClassAndMethod mostAccurateTarget = singleTarget == null ? resolvedMethod : singleTarget;
+
+ if (mostAccurateTarget.isProgramMethod()) {
+ if (mostAccurateTarget.getHolder().isEnum()
+ && resolvedMethod.getDefinition().isInstanceInitializer()) {
// The enum instance initializer is only allowed to be called from an initializer of the
// enum itself.
- if (getEnumUnboxingCandidateOrNull(code.context().getHolder().getType())
- != getEnumUnboxingCandidateOrNull(targetHolder.getType())
+ if (getEnumUnboxingCandidateOrNull(code.context().getHolderType())
+ != getEnumUnboxingCandidateOrNull(mostAccurateTarget.getHolderType())
|| !context.getDefinition().isInitializer()) {
return Reason.INVALID_INIT;
}
- if (code.method().isInstanceInitializer() && !invoke.getFirstArgument().isThis()) {
+ if (context.getDefinition().isInstanceInitializer()
+ && !invoke.getFirstArgument().isThis()) {
return Reason.INVALID_INIT;
}
}
// Check if this is a checkNotNull() user. In this case, we can create a copy of the method
// that takes an int instead of java.lang.Object and call that method instead.
- EnumUnboxerMethodClassification classification =
- singleTarget.getOptimizationInfo().getEnumUnboxerMethodClassification();
- if (classification.isCheckNotNullClassification()) {
- CheckNotNullEnumUnboxerMethodClassification checkNotNullClassification =
- classification.asCheckNotNullClassification();
- if (checkNotNullClassification.isUseEligibleForUnboxing(
- invoke.asInvokeStatic(), enumValue)) {
- GraphLens graphLens = appView.graphLens();
- checkNotNullMethodsBuilder
- .computeIfAbsent(
- singleTarget.asProgramMethod(),
- ignoreKey(
- () ->
- LongLivedClassSetBuilder.createConcurrentBuilderForIdentitySet(
- graphLens)),
- graphLens)
- .add(enumClass, graphLens);
- return Reason.ELIGIBLE;
+ if (singleTarget != null) {
+ EnumUnboxerMethodClassification classification =
+ singleTarget.getOptimizationInfo().getEnumUnboxerMethodClassification();
+ if (classification.isCheckNotNullClassification()) {
+ assert singleTarget.getDefinition().isStatic();
+ CheckNotNullEnumUnboxerMethodClassification checkNotNullClassification =
+ classification.asCheckNotNullClassification();
+ if (checkNotNullClassification.isUseEligibleForUnboxing(
+ invoke.asInvokeStatic(), enumValue)) {
+ GraphLens graphLens = appView.graphLens();
+ checkNotNullMethodsBuilder
+ .computeIfAbsent(
+ singleTarget.asProgramMethod(),
+ ignoreKey(
+ () ->
+ LongLivedClassSetBuilder.createConcurrentBuilderForIdentitySet(
+ graphLens)),
+ graphLens)
+ .add(enumClass, graphLens);
+ return Reason.ELIGIBLE;
+ }
}
}
// Check that the enum-value only flows into parameters whose type exactly matches the
// enum's type.
- for (int i = 0; i < singleTarget.getParameters().size(); i++) {
+ for (int i = 0; i < mostAccurateTarget.getParameters().size(); i++) {
if (invoke.getArgumentForParameter(i) == enumValue
&& !enumUnboxingCandidatesInfo.isAssignableTo(
- singleTarget.getParameter(i).toBaseType(factory), enumClass.getType())) {
- return new IllegalInvokeWithImpreciseParameterTypeReason(singleTargetReference);
+ mostAccurateTarget.getParameter(i).toBaseType(factory), enumClass.getType())) {
+ return new IllegalInvokeWithImpreciseParameterTypeReason(
+ mostAccurateTarget.getReference());
}
}
if (invoke.isInvokeMethodWithReceiver()) {
Value receiver = invoke.asInvokeMethodWithReceiver().getReceiver();
- if (receiver == enumValue && targetHolder.isInterface()) {
+ if (receiver == enumValue && mostAccurateTarget.getHolder().isInterface()) {
return Reason.DEFAULT_METHOD_INVOKE;
+
}
}
return Reason.ELIGIBLE;
}
- if (targetHolder.isClasspathClass()) {
+ if (mostAccurateTarget.getHolder().isClasspathClass()) {
return Reason.INVALID_INVOKE_CLASSPATH;
}
- assert targetHolder.isLibraryClass();
+ assert mostAccurateTarget.getHolder().isLibraryClass();
+
+ if (singleTarget == null) {
+ // We don't attempt library modeling if we don't have a single target.
+ return Reason.INVALID_INVOKE;
+ }
Reason reason =
analyzeLibraryInvoke(
- invoke, code, context, enumClass, enumValue, singleTargetReference, targetHolder);
+ invoke,
+ code,
+ context,
+ enumClass,
+ enumValue,
+ singleTarget.getReference(),
+ singleTarget.getHolder());
if (reason == Reason.ELIGIBLE) {
markMethodDependsOnLibraryModelisation(context);
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
index 8308e0b..1719dd9 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/VirtualMethodOverrideEnumUnboxingTest.java
@@ -1,7 +1,6 @@
// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-
package com.android.tools.r8.enumunboxing;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
@@ -11,6 +10,7 @@
import com.android.tools.r8.KeepConstantArguments;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.NoMethodStaticizing;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
@@ -52,6 +52,7 @@
.enableNeverClassInliningAnnotations()
.enableNoMethodStaticizingAnnotations()
.enableNoVerticalClassMergingAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
.addOptionsModification(options -> enableEnumOptions(options, enumValueOptimization))
.addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum.class))
.setMinApi(parameters)
@@ -59,44 +60,97 @@
.inspect(this::inspect)
.run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutputLines(
- "A.m(A : MyEnum, 42 : int)", "B.m(A : MyEnum, 42 : int)", "B.m(42 : int, A : MyEnum)");
+ "Middle.m(A : MyEnum, 42 : int)",
+ "Bottom.m(A : MyEnum, 42 : int)",
+ "Bottom.m(42 : int, A : MyEnum)",
+ "Middle.m(A : MyEnum, 42 : int)",
+ "Middle2.m(A : MyEnum, 42 : int)",
+ "Middle.m(A : MyEnum, 42 : int)",
+ "Bottom.m(A : MyEnum, 42 : int)",
+ "Middle.m(A : MyEnum, 42 : int)",
+ "Middle2.m(A : MyEnum, 42 : int)",
+ "Something");
}
private void inspect(CodeInspector inspector) {
- MethodSubject methodOnA = inspector.clazz(A.class).virtualMethods().get(0);
- MethodSubject methodOnB =
- inspector.clazz(B.class).uniqueMethodWithFinalName(methodOnA.getFinalName());
- assertThat(methodOnB, isPresent());
- assertTrue(methodOnB.streamInstructions().anyMatch(x -> x.asDexInstruction().isInvokeSuper()));
+ MethodSubject methodOnMiddle = inspector.clazz(Middle.class).virtualMethods().get(0);
+ MethodSubject methodOnBottom =
+ inspector.clazz(Bottom.class).uniqueMethodWithFinalName(methodOnMiddle.getFinalName());
+ assertThat(methodOnBottom, isPresent());
+ assertTrue(
+ methodOnBottom
+ .streamInstructions()
+ .anyMatch(
+ i ->
+ i.asDexInstruction().isInvokeSuper()
+ && i.getMethod() == methodOnMiddle.getMethod().getReference()));
}
static class TestClass {
public static void main(String[] args) {
MyEnum value = System.currentTimeMillis() > 0 ? MyEnum.A : MyEnum.B;
- new B().m(value, 42);
- new B().m(42, value);
+ new Bottom().m(value, 42);
+ new Bottom().m(42, value);
+ new Middle().m(value, 42);
+ new Middle2().m(value, 42);
+ fromTop(new Bottom(), value);
+ fromTop(new Middle(), value);
+ fromTop(new Middle2(), value);
+ new Middle().checkNotNullOrPrintSomething(value);
+ new Bottom().checkNotNullOrPrintSomething(value);
+ }
+
+ @NeverInline
+ public static void fromTop(Top top, MyEnum value) {
+ top.m(value, 42);
}
}
@NeverClassInline
@NoVerticalClassMerging
- static class A {
+ abstract static class Top {
@NeverInline
+ abstract void m(MyEnum x, int y);
+ }
+
+ @NeverClassInline
+ @NoVerticalClassMerging
+ @NoHorizontalClassMerging
+ static class Middle2 extends Top {
+ @NeverInline
void m(MyEnum x, int y) {
- System.out.println("A.m(" + x.toString() + " : MyEnum, " + y + " : int)");
+ System.out.println("Middle2.m(" + x.toString() + " : MyEnum, " + y + " : int)");
}
}
@NeverClassInline
- static class B extends A {
+ @NoVerticalClassMerging
+ @NoHorizontalClassMerging
+ static class Middle extends Top {
+
+ @NeverInline
+ void m(MyEnum x, int y) {
+ System.out.println("Middle.m(" + x.toString() + " : MyEnum, " + y + " : int)");
+ }
+
+ @NeverInline
+ void checkNotNullOrPrintSomething(MyEnum e) {
+ if (e == null) {
+ throw new NullPointerException();
+ }
+ }
+ }
+
+ @NeverClassInline
+ static class Bottom extends Middle {
@KeepConstantArguments
@NeverInline
@NoMethodStaticizing
void m(int x, MyEnum y) {
- System.out.println("B.m(" + x + " : int, " + y.toString() + " : MyEnum)");
+ System.out.println("Bottom.m(" + x + " : int, " + y.toString() + " : MyEnum)");
}
@KeepConstantArguments
@@ -104,7 +158,14 @@
@Override
void m(MyEnum x, int y) {
super.m(x, y);
- System.out.println("B.m(" + x.toString() + " : MyEnum, " + y + " : int)");
+ System.out.println("Bottom.m(" + x.toString() + " : MyEnum, " + y + " : int)");
+ }
+
+ @KeepConstantArguments
+ @NeverInline
+ @Override
+ void checkNotNullOrPrintSomething(MyEnum e) {
+ System.out.println("Something");
}
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java
index 56f3d9e..0eb13fb 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java
@@ -48,9 +48,7 @@
.assertNotUnboxed(EnumWithVirtualOverrideAndPrivateField.class))
.enableInliningAnnotations()
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
- .addOptionsModification(opt -> opt.testing.enableEnumUnboxingDebugLogs = true)
.setMinApi(parameters)
- .allowDiagnosticInfoMessages()
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("a", "B", "a", "B", "a", "B", "A 1 1.0 A", "B");
}
