Version 2.0.59
Cherry-pick: Desugared lib: allow compilation with missing conversion
CL: https://r8-review.googlesource.com/c/r8/+/49987
Bug: 152240452
Change-Id: Ie1e3fa4d324133b26f2d7258a0979acef9291b30
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index c5d4ae7..c309cc4 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.58";
+ public static final String LABEL = "2.0.59";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
index d45a68c..2393956 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryAPIConverter.java
@@ -273,22 +273,22 @@
appView.options().reporter.warning(new StringDiagnostic(sb.toString()));
}
- private void warnInvalidInvoke(DexType type, DexMethod invokedMethod, String debugString) {
+ public void reportInvalidInvoke(DexType type, DexMethod invokedMethod, String debugString) {
DexType desugaredType = appView.rewritePrefix.rewrittenType(type);
appView
.options()
.reporter
- .warning(
+ .info(
new StringDiagnostic(
"Invoke to "
+ invokedMethod.holder
+ "#"
+ invokedMethod.name
- + " may not work correctly at runtime ("
+ + " may not work correctly at runtime (Cannot convert "
+ debugString
- + " type "
+ + "type "
+ desugaredType
- + " is a desugared type)."));
+ + ")."));
}
public static DexType vivifiedTypeFor(DexType type, AppView<?> appView) {
@@ -306,6 +306,7 @@
InstructionListIterator iterator,
ListIterator<BasicBlock> blockIterator) {
DexMethod invokedMethod = invokeMethod.getInvokedMethod();
+ boolean invalidConversion = false;
if (trackedAPIs != null) {
trackedAPIs.add(invokedMethod);
}
@@ -320,12 +321,13 @@
// Return conversion added only if return value is used.
if (invokeMethod.outValue() != null
&& invokeMethod.outValue().numberOfUsers() + invokeMethod.outValue().numberOfPhiUsers()
- > 0) {
+ > 0) {
returnConversion =
createReturnConversionAndReplaceUses(code, invokeMethod, returnType, newReturnType);
}
} else {
- warnInvalidInvoke(returnType, invokeMethod.getInvokedMethod(), "return");
+ reportInvalidInvoke(returnType, invokeMethod.getInvokedMethod(), "return ");
+ invalidConversion = true;
newReturnType = returnType;
}
} else {
@@ -353,7 +355,8 @@
createParameterConversion(code, argType, argVivifiedType, inValue));
newInValues.add(parameterConversions.get(parameterConversions.size() - 1).outValue());
} else {
- warnInvalidInvoke(argType, invokeMethod.getInvokedMethod(), "parameter");
+ reportInvalidInvoke(argType, invokeMethod.getInvokedMethod(), "parameter ");
+ invalidConversion = true;
newInValues.add(invokeMethod.inValues().get(i + receiverShift));
}
} else {
@@ -373,7 +376,8 @@
invokeMethod.outValue(),
newInValues);
assert newDexMethod
- == methodWithVivifiedTypeInSignature(invokedMethod, invokedMethod.holder, appView);
+ == methodWithVivifiedTypeInSignature(invokedMethod, invokedMethod.holder, appView)
+ || invalidConversion;
// Insert and reschedule all instructions.
iterator.previous();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryWrapperSynthesizer.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryWrapperSynthesizer.java
index 99994fe..ed86a23 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryWrapperSynthesizer.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryWrapperSynthesizer.java
@@ -125,7 +125,7 @@
boolean canGenerateWrapper(DexType type) {
DexClass dexClass = appView.definitionFor(type);
- if (dexClass == null) {
+ if (dexClass == null || dexClass.accessFlags.isFinal()) {
return false;
}
return dexClass.isLibraryClass() || appView.options().isDesugaredLibraryCompilation();
@@ -162,18 +162,9 @@
private DexClass getValidClassToWrap(DexType type) {
DexClass dexClass = appView.definitionFor(type);
// The dexClass should be a library class, so it cannot be null.
- assert dexClass != null
- && (dexClass.isLibraryClass() || appView.options().isDesugaredLibraryCompilation());
- if (dexClass.accessFlags.isFinal()) {
- throw appView
- .options()
- .reporter
- .fatalError(
- new StringDiagnostic(
- "Cannot generate a wrapper for final class "
- + dexClass.type
- + ". Add a custom conversion in the desugared library."));
- }
+ assert dexClass != null;
+ assert dexClass.isLibraryClass() || appView.options().isDesugaredLibraryCompilation();
+ assert !dexClass.accessFlags.isFinal();
return dexClass;
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/DesugaredLibraryAPIConversionCfCodeProvider.java b/src/main/java/com/android/tools/r8/ir/synthetic/DesugaredLibraryAPIConversionCfCodeProvider.java
index 5597bb5..3cb579f 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/DesugaredLibraryAPIConversionCfCodeProvider.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/DesugaredLibraryAPIConversionCfCodeProvider.java
@@ -30,7 +30,6 @@
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.desugar.DesugaredLibraryAPIConverter;
-import com.android.tools.r8.utils.StringDiagnostic;
import java.util.ArrayList;
import java.util.List;
import org.objectweb.asm.Opcodes;
@@ -41,24 +40,14 @@
super(appView, holder);
}
- boolean shouldConvert(
- DexType type, DesugaredLibraryAPIConverter converter, DexString methodName) {
+ boolean shouldConvert(DexType type, DesugaredLibraryAPIConverter converter, DexMethod method) {
if (!appView.rewritePrefix.hasRewrittenType(type)) {
return false;
}
if (converter.canConvert(type)) {
return true;
}
- appView
- .options()
- .reporter
- .warning(
- new StringDiagnostic(
- "Desugared library API conversion failed for "
- + type
- + ", unexpected behavior for method "
- + methodName
- + "."));
+ converter.reportInvalidInvoke(type, method, "");
return false;
}
@@ -101,7 +90,7 @@
DexType[] newParameters = forwardMethod.proto.parameters.values.clone();
for (DexType param : forwardMethod.proto.parameters.values) {
instructions.add(new CfLoad(ValueType.fromDexType(param), stackIndex));
- if (shouldConvert(param, converter, forwardMethod.name)) {
+ if (shouldConvert(param, converter, forwardMethod)) {
instructions.add(
new CfInvoke(
Opcodes.INVOKESTATIC,
@@ -132,7 +121,7 @@
instructions.add(new CfInvoke(Opcodes.INVOKEVIRTUAL, newForwardMethod, false));
}
- if (shouldConvert(returnType, converter, forwardMethod.name)) {
+ if (shouldConvert(returnType, converter, forwardMethod)) {
instructions.add(
new CfInvoke(
Opcodes.INVOKESTATIC,
@@ -188,7 +177,7 @@
int stackIndex = 1;
for (DexType param : forwardMethod.proto.parameters.values) {
instructions.add(new CfLoad(ValueType.fromDexType(param), stackIndex));
- if (shouldConvert(param, converter, forwardMethod.name)) {
+ if (shouldConvert(param, converter, forwardMethod)) {
instructions.add(
new CfInvoke(
Opcodes.INVOKESTATIC,
@@ -208,7 +197,7 @@
}
DexType returnType = forwardMethod.proto.returnType;
- if (shouldConvert(returnType, converter, forwardMethod.name)) {
+ if (shouldConvert(returnType, converter, forwardMethod)) {
instructions.add(
new CfInvoke(
Opcodes.INVOKESTATIC,
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassErrorTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassErrorTest.java
deleted file mode 100644
index 403ae3c..0000000
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassErrorTest.java
+++ /dev/null
@@ -1,57 +0,0 @@
-// 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.desugar.desugaredlibrary.conversiontests;
-
-import static junit.framework.TestCase.assertEquals;
-import static junit.framework.TestCase.fail;
-
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.TestDiagnosticMessages;
-import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
-import com.android.tools.r8.utils.AndroidApiLevel;
-import java.time.Year;
-import org.junit.Test;
-
-public class APIConversionFinalClassErrorTest extends DesugaredLibraryTestBase {
-
- @Test
- public void testFinalMethod() {
- try {
- testForD8()
- .setMinApi(AndroidApiLevel.B)
- .addProgramClasses(Executor.class)
- .addLibraryClasses(CustomLibClass.class)
- .enableCoreLibraryDesugaring(AndroidApiLevel.B)
- .compileWithExpectedDiagnostics(this::assertDiagnosis);
- fail("Expected compilation error");
- } catch (CompilationFailedException ignored) {
-
- }
- }
-
- private void assertDiagnosis(TestDiagnosticMessages d) {
- assertEquals(
- "Cannot generate a wrapper for final class java.time.Year."
- + " Add a custom conversion in the desugared library.",
- d.getErrors().get(0).getDiagnosticMessage());
- }
-
- static class Executor {
-
- public static void main(String[] args) {
- System.out.println(CustomLibClass.call(Year.now()));
- }
- }
-
- // This class will be put at compilation time as library and on the runtime class path.
- // This class is convenient for easy testing. Each method plays the role of methods in the
- // platform APIs for which argument/return values need conversion.
- static class CustomLibClass {
-
- public static long call(Year year) {
- return 0L;
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassTest.java
new file mode 100644
index 0000000..523b961
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/APIConversionFinalClassTest.java
@@ -0,0 +1,89 @@
+// 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.desugar.desugaredlibrary.conversiontests;
+
+import static org.hamcrest.core.StringContains.containsString;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.desugar.desugaredlibrary.DesugaredLibraryTestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.BooleanUtils;
+import java.nio.file.Path;
+import java.time.Year;
+import java.util.List;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class APIConversionFinalClassTest extends DesugaredLibraryTestBase {
+
+ private final TestParameters parameters;
+ private final boolean shrinkDesugaredLibrary;
+
+ private static final AndroidApiLevel MIN_SUPPORTED = AndroidApiLevel.O;
+
+ private static Path customLib;
+
+ @Parameterized.Parameters(name = "{0}, shrinkDesugaredLibrary: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(getTestParameters().withDexRuntimesStartingFromIncluding(ToolHelper.DexVm.Version.V8_1_0).withApiLevelsEndingAtExcluding(AndroidApiLevel.O).build(), BooleanUtils.values());
+ }
+
+ public APIConversionFinalClassTest(TestParameters parameters, boolean shrinkDesugaredLibrary) {
+ this.shrinkDesugaredLibrary = shrinkDesugaredLibrary;
+ this.parameters = parameters;
+ }
+
+ @BeforeClass
+ public static void compileCustomLib() throws Exception {
+ customLib =
+ testForD8(getStaticTemp())
+ .addProgramClasses(CustomLibClass.class)
+ .setMinApi(MIN_SUPPORTED)
+ .compile()
+ .writeToZip();
+ }
+
+ @Test
+ public void testFinalMethod() throws Exception {
+ KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters);
+ testForD8()
+ .setMinApi(AndroidApiLevel.B)
+ .addProgramClasses(Executor.class)
+ .addLibraryClasses(CustomLibClass.class)
+ .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary,
+ parameters.getApiLevel(),
+ keepRuleConsumer.get(),
+ shrinkDesugaredLibrary)
+ .addRunClasspathFiles(customLib)
+ .run(parameters.getRuntime(), Executor.class)
+ .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"));
+ }
+
+ static class Executor {
+
+ public static void main(String[] args) {
+ System.out.println(CustomLibClass.libCall(Year.now()));
+ }
+ }
+
+ // This class will be put at compilation time as library and on the runtime class path.
+ // This class is convenient for easy testing. Each method plays the role of methods in the
+ // platform APIs for which argument/return values need conversion.
+ static class CustomLibClass {
+
+ // We use Year because Year is a final class with no custom conversion but Year has been
+ // unused in the Android library so far.
+ public static long libCall(Year year) {
+ return 0L;
+ }
+ }
+}
