Report errors if a backport is triggered for platform builds.
Bug: b/232073181
Change-Id: I66a5e00e1068421556683a32c8d68ade7791a672
diff --git a/src/main/java/com/android/tools/r8/errors/BackportDiagnostic.java b/src/main/java/com/android/tools/r8/errors/BackportDiagnostic.java
new file mode 100644
index 0000000..b775cc1
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/BackportDiagnostic.java
@@ -0,0 +1,38 @@
+// Copyright (c) 2023, 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.errors;
+
+import com.android.tools.r8.Keep;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+
+@Keep
+public class BackportDiagnostic implements DesugarDiagnostic {
+
+ private final DexMethod backport;
+ private final Origin origin;
+ private final Position position;
+
+ public BackportDiagnostic(DexMethod backport, Origin origin, Position position) {
+ this.backport = backport;
+ this.origin = origin;
+ this.position = position;
+ }
+
+ @Override
+ public Origin getOrigin() {
+ return origin;
+ }
+
+ @Override
+ public Position getPosition() {
+ return position;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ return "Attempt to backport " + backport.toSourceString();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
index bc11f33..60c67a1 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.contexts.CompilationContext.MethodProcessingContext;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.errors.BackportDiagnostic;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
@@ -51,6 +52,7 @@
import com.android.tools.r8.ir.desugar.backports.OptionalMethodRewrites;
import com.android.tools.r8.ir.desugar.backports.SparseArrayMethodRewrites;
import com.android.tools.r8.ir.desugar.desugaredlibrary.retargeter.DesugaredLibraryRetargeter;
+import com.android.tools.r8.position.MethodPosition;
import com.android.tools.r8.synthesis.SyntheticItems.GlobalSyntheticsStrategy;
import com.android.tools.r8.synthesis.SyntheticNaming;
import com.android.tools.r8.synthesis.SyntheticNaming.SyntheticKind;
@@ -102,7 +104,7 @@
}
CfInvoke invoke = instruction.asInvoke();
- MethodProvider methodProvider = getMethodProviderOrNull(invoke.getMethod());
+ MethodProvider methodProvider = getMethodProviderOrNull(invoke.getMethod(), context);
return methodProvider != null
? methodProvider.rewriteInvoke(
invoke, appView, eventConsumer, methodProcessingContext, localStackAllocator)
@@ -112,7 +114,7 @@
@Override
public boolean needsDesugaring(CfInstruction instruction, ProgramMethod context) {
return instruction.isInvoke()
- && getMethodProviderOrNull(instruction.asInvoke().getMethod()) != null
+ && getMethodProviderOrNull(instruction.asInvoke().getMethod(), context) != null
&& !appView
.getSyntheticItems()
.isSyntheticOfKind(context.getContextType(), kinds -> kinds.BACKPORT_WITH_FORWARDING);
@@ -146,7 +148,7 @@
BackportedMethods.registerSynthesizedCodeReferences(options.itemFactory);
}
- private MethodProvider getMethodProviderOrNull(DexMethod method) {
+ private MethodProvider getMethodProviderOrNull(DexMethod method, ProgramMethod context) {
DexMethod original = appView.graphLens().getOriginalMethodSignature(method);
assert original != null;
MethodProvider provider = rewritableMethods.getProvider(original);
@@ -164,6 +166,14 @@
appView.dexItemFactory().createMethod(newHolder, method.proto, method.name);
provider = rewritableMethods.getProvider(backportedMethod);
}
+ if (provider != null && appView.options().disableBackportsWithErrorDiagnostics) {
+ appView
+ .reporter()
+ .error(
+ new BackportDiagnostic(
+ provider.method, context.getOrigin(), MethodPosition.create(context)));
+ return null;
+ }
return provider;
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 51a5873..f3f358f 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -272,18 +272,15 @@
horizontalClassMergerOptions.setRestrictToSynthetics();
}
+ // Configure options according to platform build assumptions.
+ // See go/r8platformflag and b/232073181.
public void configureAndroidPlatformBuild(boolean isAndroidPlatformBuild) {
assert !androidPlatformBuild;
if (isAndroidPlatformBuild || minApiLevel.isPlatform()) {
apiModelingOptions().disableApiModeling();
+ disableBackportsWithErrorDiagnostics = true;
+ androidPlatformBuild = isAndroidPlatformBuild;
}
- if (!isAndroidPlatformBuild) {
- return;
- }
- // Configure options according to platform build assumptions.
- // See go/r8platformflag and b/232073181.
- androidPlatformBuild = isAndroidPlatformBuild;
- enableBackportMethods = false;
}
public boolean isAndroidPlatformBuild() {
@@ -624,8 +621,8 @@
public DesugarState desugarState = DesugarState.ON;
// Flag to turn on/off partial VarHandle desugaring.
public boolean enableVarHandleDesugaring = false;
- // Flag to turn on/off backport methods.
- public boolean enableBackportMethods = true;
+ // Flag to turn off backport methods (and report errors if triggered).
+ public boolean disableBackportsWithErrorDiagnostics = false;
// Flag to turn on/off reduction of nest to improve class merging optimizations.
public boolean enableNestReduction = true;
// Defines interface method rewriter behavior.
@@ -2390,14 +2387,7 @@
}
public boolean enableBackportedMethodRewriting() {
- // Disable rewriting if there are no methods to rewrite or if the API level is higher than
- // the highest known API level when the compiler is built. This ensures that when this is used
- // by the Android Platform build (which normally use an API level of 10000) there will be
- // no rewriting of backported methods. See b/147480264.
- return enableBackportMethods
- && desugarState.isOn()
- // TODO(b/232073181): This platform check should rather be controlled via the platform flag.
- && getMinApiLevel().isLessThanOrEqualTo(AndroidApiLevel.LATEST);
+ return desugarState.isOn();
}
public boolean enableTryWithResourcesDesugaring() {
diff --git a/src/test/java/com/android/tools/r8/PositionMatcher.java b/src/test/java/com/android/tools/r8/PositionMatcher.java
index 4fca1c6..888ad03 100644
--- a/src/test/java/com/android/tools/r8/PositionMatcher.java
+++ b/src/test/java/com/android/tools/r8/PositionMatcher.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import com.android.tools.r8.position.MethodPosition;
import com.android.tools.r8.position.Position;
import com.android.tools.r8.position.TextPosition;
import org.hamcrest.Description;
@@ -24,4 +25,19 @@
}
};
}
+
+ public static Matcher<Position> positionMethodName(String methodName) {
+ return new PositionMatcher() {
+ @Override
+ protected boolean matchesSafely(Position position) {
+ return position instanceof MethodPosition
+ && ((MethodPosition) position).getName().equals(methodName);
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description.appendText("with method " + methodName);
+ }
+ };
+ }
}
diff --git a/src/test/java/com/android/tools/r8/desugar/backports/BackportPlatformTest.java b/src/test/java/com/android/tools/r8/desugar/backports/BackportPlatformTest.java
new file mode 100644
index 0000000..94fb0ce
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/backports/BackportPlatformTest.java
@@ -0,0 +1,112 @@
+// Copyright (c) 2023, 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.desugar.backports;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticPosition;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.PositionMatcher;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.desugar.backports.AbstractBackportTest.MiniAssert;
+import com.android.tools.r8.errors.BackportDiagnostic;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class BackportPlatformTest extends TestBase {
+
+ static final String EXPECTED = StringUtils.lines("Hello, world");
+
+ static final List<Class<?>> CLASSES =
+ ImmutableList.of(MiniAssert.class, TestClass.class, User.class);
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultDexRuntime().withApiLevel(AndroidApiLevel.J).build();
+ }
+
+ public BackportPlatformTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(CLASSES)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(MiniAssert.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ testForD8(parameters.getBackend())
+ .addProgramClasses(CLASSES)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testPlatformR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .apply(b -> b.getBuilder().setAndroidPlatformBuild(true))
+ .addProgramClasses(CLASSES)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(MiniAssert.class)
+ .setMinApi(parameters.getApiLevel())
+ .compileWithExpectedDiagnostics(this::checkDiagnostics);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testPlatformD8() throws Exception {
+ testForD8(parameters.getBackend())
+ .apply(b -> b.getBuilder().setAndroidPlatformBuild(true))
+ .addProgramClasses(CLASSES)
+ .setMinApi(parameters.getApiLevel())
+ .compileWithExpectedDiagnostics(this::checkDiagnostics);
+ }
+
+ private void checkDiagnostics(TestDiagnosticMessages diagnostics) {
+ diagnostics
+ .assertAllErrorsMatch(
+ allOf(
+ diagnosticType(BackportDiagnostic.class),
+ diagnosticMessage(
+ containsString("int java.lang.Boolean.compare(boolean, boolean)")),
+ diagnosticPosition(PositionMatcher.positionMethodName("testBooleanCompare"))))
+ .assertOnlyErrors();
+ }
+
+ static class User {
+
+ private static void testBooleanCompare() {
+ MiniAssert.assertTrue(Boolean.compare(true, false) > 0);
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ User.testBooleanCompare();
+ System.out.println("Hello, world");
+ }
+ }
+}
