Remove Objects#requireNonNull for definitely non-null reference.
Bug: 124246610
Change-Id: Id8e068ec9d79c5af65ccef94b3864b195132dd18
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 55bc508..977c90b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -292,6 +292,13 @@
debugData = null;
}
+ public boolean hasSameOrNoLocal(Value other) {
+ assert other != null;
+ return hasLocalInfo()
+ ? other.getLocalInfo() == this.getLocalInfo()
+ : !other.hasLocalInfo();
+ }
+
public List<Instruction> getDebugLocalStarts() {
if (debugData == null) {
return Collections.emptyList();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 30712e6..f7baf3e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -48,6 +48,7 @@
import com.android.tools.r8.graph.ParameterUsagesInfo.ParameterUsage;
import com.android.tools.r8.graph.ParameterUsagesInfo.ParameterUsageBuilder;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption;
+import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.AlwaysMaterializingNop;
@@ -1632,10 +1633,30 @@
Instruction current = iterator.next();
if (current.isInvokeMethod()) {
InvokeMethod invoke = current.asInvokeMethod();
- if (invoke.outValue() != null && !invoke.outValue().hasLocalInfo()) {
+ Value outValue = invoke.outValue();
+ // TODO(b/124246610): extend to other variants that receive error messages or supplier.
+ if (invoke.getInvokedMethod() == dexItemFactory.objectsMethods.requireNonNull) {
+ Value obj = invoke.arguments().get(0);
+ if ((outValue == null && obj.hasLocalInfo())
+ || (outValue != null && !obj.hasSameOrNoLocal(outValue))) {
+ continue;
+ }
+ Nullability nullability = obj.getTypeLattice().nullability();
+ if (nullability.isDefinitelyNotNull()) {
+ if (outValue != null) {
+ outValue.replaceUsers(obj);
+ needToNarrowValues.addAll(outValue.affectedValues());
+ }
+ iterator.removeOrReplaceByDebugLocalRead();
+ } else if (nullability.isDefinitelyNull()) {
+ // TODO(b/124246610): throw NPE.
+ // Refactor UninstantiatedTypeOptimization#replaceCurrentInstructionWithThrowNull
+ // and move it to iterator.
+ }
+ } else if (outValue != null && !outValue.hasLocalInfo()) {
if (libraryMethodsReturningReceiver.contains(invoke.getInvokedMethod())) {
if (checkArgumentType(invoke, 0)) {
- invoke.outValue().replaceUsers(invoke.arguments().get(0));
+ outValue.replaceUsers(invoke.arguments().get(0));
invoke.setOutValue(null);
}
} else if (appInfoWithLiveness != null) {
@@ -1650,7 +1671,6 @@
// Replace the out value of the invoke with the argument and ignore the out value.
if (argumentIndex >= 0 && checkArgumentType(invoke, argumentIndex)) {
Value argument = invoke.arguments().get(argumentIndex);
- Value outValue = invoke.outValue();
assert outValue.verifyCompatible(argument.outType());
if (argument.getTypeLattice().lessThanOrEqual(
outValue.getTypeLattice(), appInfo)) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ObjectsRequireNonNullTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ObjectsRequireNonNullTest.java
new file mode 100644
index 0000000..7a2a84e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ObjectsRequireNonNullTest.java
@@ -0,0 +1,132 @@
+// Copyright (c) 2019, 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.ir.optimize;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.Streams;
+import java.util.Objects;
+import org.junit.Test;
+
+class ObjectsRequireNonNullTestMain {
+
+ static class Foo {
+ @NeverInline
+ void bar() {
+ System.out.println("Foo::bar");
+ }
+
+ @NeverInline
+ @Override
+ public String toString() {
+ return "Foo::toString";
+ }
+ }
+
+ @NeverInline
+ static void unknownArg(Foo foo) {
+ // It's unclear the argument is definitely null or not null.
+ Foo checked = Objects.requireNonNull(foo);
+ checked.bar();
+ }
+
+ public static void main(String[] args) {
+ Foo instance = new Foo();
+ // Not removable in debug mode.
+ Object nonNull = Objects.requireNonNull(instance);
+ System.out.println(nonNull);
+ // Removable because associated locals are changed while type casting.
+ Foo checked = Objects.requireNonNull(instance);
+ checked.bar();
+
+ unknownArg(instance);
+ try {
+ unknownArg(null);
+ } catch (NullPointerException npe) {
+ System.out.println("Expected NPE");
+ }
+ }
+}
+
+public class ObjectsRequireNonNullTest extends TestBase {
+ private static final String JAVA_OUTPUT = StringUtils.lines(
+ "Foo::toString",
+ "Foo::bar",
+ "Foo::bar",
+ "Expected NPE"
+ );
+ private static final Class<?> MAIN = ObjectsRequireNonNullTestMain.class;
+
+ @Test
+ public void testJvmOutput() throws Exception {
+ testForJvm().addTestClasspath().run(MAIN).assertSuccessWithOutput(JAVA_OUTPUT);
+ }
+
+ private static boolean isObjectsRequireNonNull(DexMethod method) {
+ return method.toSourceString().equals(
+ "java.lang.Object java.util.Objects.requireNonNull(java.lang.Object)");
+ }
+
+ private long countObjectsRequireNonNull(MethodSubject method) {
+ return Streams.stream(method.iterateInstructions(instructionSubject -> {
+ if (instructionSubject.isInvoke()) {
+ return isObjectsRequireNonNull(instructionSubject.getMethod());
+ }
+ return false;
+ })).count();
+ }
+
+ private void test(TestRunResult result, int expectedCount) throws Exception {
+ CodeInspector codeInspector = result.inspector();
+ ClassSubject mainClass = codeInspector.clazz(MAIN);
+ MethodSubject mainMethod = mainClass.mainMethod();
+ assertThat(mainMethod, isPresent());
+ long count = countObjectsRequireNonNull(mainMethod);
+ assertEquals(expectedCount, count);
+
+ MethodSubject unknownArg = mainClass.uniqueMethodWithName("unknownArg");
+ assertThat(unknownArg, isPresent());
+ // Due to the nullable argument, requireNonNull should remain.
+ assertEquals(1, countObjectsRequireNonNull(unknownArg));
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ TestRunResult result = testForD8()
+ .debug()
+ .addProgramClassesAndInnerClasses(MAIN)
+ .run(MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ test(result, 2);
+
+ result = testForD8()
+ .release()
+ .addProgramClassesAndInnerClasses(MAIN)
+ .run(MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ test(result, 0);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ // CF disables move result optimization.
+ TestRunResult result = testForR8(Backend.DEX)
+ .addProgramClassesAndInnerClasses(MAIN)
+ .enableInliningAnnotations()
+ .addKeepMainRule(MAIN)
+ .run(MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ test(result, 0);
+ }
+}
