Revert "Rewrite CONSTANT_Dynamic bootstrap method signature (part 1)"
This reverts commit cbfdaf5fd65871fd1b0446a6298ea5e21fc7b764.
Change-Id: I44ba28307e54b17625a58151ade5e0cbba147dd9
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfFrame.java b/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
index 47124c0..f8b10e0 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfFrame.java
@@ -532,27 +532,6 @@
}
public CfFrame map(java.util.function.Function<DexType, DexType> func) {
- boolean mapped = false;
- for (int var : locals.keySet()) {
- CfFrame.FrameType originalType = locals.get(var);
- CfFrame.FrameType mappedType = originalType.map(func);
- mapped = originalType != mappedType;
- if (mapped) {
- break;
- }
- }
- if (!mapped) {
- for (FrameType frameType : stack) {
- CfFrame.FrameType mappedType = frameType.map(func);
- mapped = frameType != mappedType;
- if (mapped) {
- break;
- }
- }
- }
- if (!mapped) {
- return this;
- }
Int2ReferenceSortedMap<FrameType> newLocals = new Int2ReferenceAVLTreeMap<>();
for (int var : locals.keySet()) {
newLocals.put(var, locals.get(var).map(func));
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/CfInstructionDesugaringEventConsumer.java b/src/main/java/com/android/tools/r8/ir/desugar/CfInstructionDesugaringEventConsumer.java
index 89b7d95..2493dd3 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/CfInstructionDesugaringEventConsumer.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/CfInstructionDesugaringEventConsumer.java
@@ -341,7 +341,6 @@
private void finalizeConstantDynamicDesugaring(Consumer<ProgramMethod> needsProcessing) {
for (ConstantDynamicClass constantDynamicClass : synthesizedConstantDynamicClasses) {
- constantDynamicClass.rewriteBootstrapMethodSignatureIfNeeded();
constantDynamicClass.getConstantDynamicProgramClass().forEachProgramMethod(needsProcessing);
}
synthesizedLambdaClasses.clear();
@@ -499,7 +498,6 @@
public void finalizeDesugaring() {
finalizeInvokeSpecialDesugaring();
finalizeLambdaDesugaring();
- // TODO(b/210485236): Finalize constant dynamic desugaring by rewriting signature if needed.
}
private void finalizeInvokeSpecialDesugaring() {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicClass.java b/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicClass.java
index e9fa936..492d428 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/constantdynamic/ConstantDynamicClass.java
@@ -35,7 +35,6 @@
import com.android.tools.r8.contexts.CompilationContext.MethodProcessingContext;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.CfCode;
-import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
@@ -43,7 +42,6 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexMethodHandle;
import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.FieldAccessFlags;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.MethodResolutionResult;
@@ -59,7 +57,6 @@
import com.android.tools.r8.ir.optimize.UtilityMethodsForCodeOptimizations.UtilityMethodForCodeOptimizations;
import com.android.tools.r8.synthesis.SyntheticProgramClassBuilder;
import com.android.tools.r8.utils.AndroidApiLevel;
-import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.collections.ImmutableDeque;
import com.android.tools.r8.utils.collections.ImmutableInt2ReferenceSortedMap;
import com.google.common.collect.ImmutableList;
@@ -85,8 +82,6 @@
public final DexField constantValueField;
private final DexMethod getConstMethod;
private final Behaviour behaviour;
- private DexEncodedMethod bootstrapMethodImpl;
- private DexMethod finalBootstrapMethodReference;
// Considered final but is set after due to circularity in allocation.
private DexProgramClass clazz = null;
@@ -122,30 +117,12 @@
.resolveMethod(bootstrapMethodReference, bootstrapMethodHandle.isInterface);
if (resolution.isSingleResolution()
&& resolution.asSingleResolution().getResolvedMethod().isStatic()) {
+ // Ensure that the bootstrap method is accessible from the generated class.
SingleResolutionResult result = resolution.asSingleResolution();
- bootstrapMethodImpl = result.getResolvedMethod();
- if (shouldRewriteBootstrapMethodSignature()) {
- // The bootstrap method will have its signature modified to have type Object as its first
- // argument.
- this.finalBootstrapMethodReference =
- factory.createMethod(
- result.getResolvedHolder().getType(),
- factory.createProto(
- bootstrapMethodReference.getReturnType(),
- factory.objectType,
- factory.stringType,
- factory.classType),
- bootstrapMethodImpl.getName());
- } else {
- this.finalBootstrapMethodReference = bootstrapMethodReference;
- // Ensure that the bootstrap method is accessible from the generated class.
- MethodAccessFlags flags = bootstrapMethodImpl.getAccessFlags();
- flags.unsetPrivate();
- flags.setPublic();
- }
-
+ MethodAccessFlags flags = result.getResolvedMethod().getAccessFlags();
+ flags.unsetPrivate();
+ flags.setPublic();
behaviour = CACHE_CONSTANT;
-
synthesizeConstantDynamicClass(builder);
} else {
// Unconditionally throw as the RI.
@@ -153,12 +130,6 @@
}
}
- private boolean shouldRewriteBootstrapMethodSignature() {
- // TODO(b/210485236): Check for references to the bootstrap method outside of dynamic constant.
- return !appView.enableWholeProgramOptimizations()
- && appView.options().getMinApiLevel().isLessThan(AndroidApiLevel.O);
- }
-
public Collection<CfInstruction> desugarConstDynamicInstruction(
CfConstDynamic invoke,
FreshLocalProvider freshLocalProvider,
@@ -249,11 +220,13 @@
private void invokeBootstrapMethod(ImmutableList.Builder<CfInstruction> instructions) {
assert reference.getBootstrapMethod().type.isInvokeStatic();
+ DexMethodHandle bootstrapMethodHandle = reference.getBootstrapMethod();
+ DexMethod bootstrapMethodReference = bootstrapMethodHandle.asMethod();
// TODO(b/178172809): Use MethodHandle.invokeWithArguments if supported.
instructions.add(new CfConstNull());
instructions.add(new CfConstString(reference.getName()));
instructions.add(new CfConstClass(reference.getType()));
- instructions.add(new CfInvoke(INVOKESTATIC, finalBootstrapMethodReference, false));
+ instructions.add(new CfInvoke(INVOKESTATIC, bootstrapMethodReference, false));
instructions.add(new CfCheckCast(reference.getType()));
}
@@ -350,77 +323,4 @@
assert clazz != null;
this.clazz = clazz;
}
-
- public void rewriteBootstrapMethodSignatureIfNeeded() {
- if (!shouldRewriteBootstrapMethodSignature() || behaviour != CACHE_CONSTANT) {
- return;
- }
- DexProgramClass bootstrapMethodHolder =
- appView.definitionFor(bootstrapMethodImpl.getHolderType()).asProgramClass();
- DexEncodedMethod replacement =
- bootstrapMethodHolder
- .getMethodCollection()
- .replaceDirectMethod(
- bootstrapMethodImpl.getReference(),
- encodedMethod -> {
- MethodAccessFlags newAccessFlags = encodedMethod.accessFlags.copy();
- // Ensure that the bootstrap method is accessible from the generated class.
- newAccessFlags.unsetPrivate();
- newAccessFlags.setPublic();
- DexEncodedMethod newMethod =
- DexEncodedMethod.builder()
- .setMethod(finalBootstrapMethodReference)
- .setAccessFlags(newAccessFlags)
- .setGenericSignature(encodedMethod.getGenericSignature())
- .setAnnotations(encodedMethod.annotations())
- .setParameterAnnotations(encodedMethod.parameterAnnotationsList)
- .setCode(adaptCode(encodedMethod))
- .setD8R8Synthesized()
- .setApiLevelForDefinition(encodedMethod.getApiLevelForDefinition())
- .setApiLevelForCode(encodedMethod.getApiLevelForCode())
- .build();
- newMethod.copyMetadata(encodedMethod);
- return newMethod;
- });
- if (replacement != null) {
- // Since we've copied the code object from an existing method, the code should already be
- // processed, and thus we don't need to schedule it for processing in D8.
- assert !appView.options().isGeneratingClassFiles() || replacement.getCode().isCfCode();
- assert !appView.options().isGeneratingDex() || replacement.getCode().isDexCode();
- }
- // The method might already have been moved by another dynamic constant targeting it.
- // If so, it must be defined on the holder.
- ProgramMethod modified =
- bootstrapMethodHolder.lookupProgramMethod(finalBootstrapMethodReference);
- assert modified != null;
- assert modified.getDefinition().isPublicMethod();
- }
-
- private DexType mapLookupTypeToObject(DexType type) {
- return type == appView.dexItemFactory().lookupType ? appView.dexItemFactory().objectType : type;
- }
-
- private Code adaptCode(DexEncodedMethod method) {
- assert behaviour == CACHE_CONSTANT;
- if (method.getCode().isDexCode()) {
- return method.getCode();
- }
- CfCode code = method.getCode().asCfCode();
- List<CfInstruction> newInstructions =
- ListUtils.mapOrElse(
- code.getInstructions(),
- instruction ->
- instruction.isFrame()
- ? instruction.asFrame().map(this::mapLookupTypeToObject)
- : instruction);
- return code.getInstructions() != newInstructions
- ? new CfCode(
- method.getHolderType(),
- code.getMaxStack(),
- code.getMaxLocals(),
- newInstructions,
- code.getTryCatchRanges(),
- code.getLocalVariables())
- : code;
- }
}
diff --git a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicGetDeclaredMethodsTest.java b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicGetDeclaredMethodsTest.java
index 4810b1d..de9685e 100644
--- a/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicGetDeclaredMethodsTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/constantdynamic/ConstantDynamicGetDeclaredMethodsTest.java
@@ -34,7 +34,7 @@
getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build());
}
- private static final String EXPECTED_OUTPUT_WITH_METHOD_HANDLES =
+ private static final String EXPECTED_OUTPUT =
StringUtils.lines(
"Hello, world!",
"myConstant",
@@ -42,14 +42,6 @@
"java.lang.invoke.MethodHandles$Lookup",
"java.lang.String",
"java.lang.Class");
- private static final String EXPECTED_OUTPUT_WITHOUT_METHOD_HANDLES =
- StringUtils.lines(
- "Hello, world!",
- "myConstant",
- "3",
- "java.lang.Object",
- "java.lang.String",
- "java.lang.Class");
private static final String EXPECTED_OUTPUT_R8 =
StringUtils.lines("Hello, world!", "No myConstant method");
@@ -64,7 +56,7 @@
testForJvm()
.addProgramClassFileData(getTransformedClasses())
.run(parameters.getRuntime(), MAIN_CLASS)
- .assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES);
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
@Test
@@ -72,22 +64,24 @@
testForDesugaring(parameters)
.addProgramClassFileData(getTransformedClasses())
.run(parameters.getRuntime(), MAIN_CLASS)
+ // TODO(b/210485236): This should never fail.
.applyIf(
// When not desugaring the CF code requires JDK 11.
DesugarTestConfiguration::isNotDesugared,
r -> {
if (parameters.isCfRuntime()
&& parameters.getRuntime().asCf().isNewerThanOrEqual(CfVm.JDK11)) {
- r.assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES);
+ r.assertSuccessWithOutput(EXPECTED_OUTPUT);
} else {
r.assertFailureWithErrorThatThrows(UnsupportedClassVersionError.class);
}
},
c ->
DesugarTestConfiguration.isDesugared(c)
- && parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
- r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT_WITHOUT_METHOD_HANDLES),
- r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES));
+ && parameters.isDexRuntime()
+ && parameters.asDexRuntime().getVersion().isOlderThan(Version.V8_1_0),
+ r -> r.assertFailureWithErrorThatThrows(ClassNotFoundException.class),
+ r -> r.assertSuccessWithOutput(EXPECTED_OUTPUT));
}
@Test
@@ -116,10 +110,11 @@
parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
b -> b.addDontWarn(MethodHandles.Lookup.class))
.run(parameters.getRuntime(), MAIN_CLASS)
+ // TODO(b/210485236): This should never fail.
.applyIf(
parameters.getDexRuntimeVersion().isOlderThan(Version.V8_1_0),
b -> b.assertFailureWithErrorThatThrows(ClassNotFoundException.class),
- b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES));
+ b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT));
}
private byte[] getTransformedClasses() throws IOException {
diff --git a/src/test/java/com/android/tools/r8/desugar/constantdynamic/JacocoConstantDynamicGetDeclaredMethods.java b/src/test/java/com/android/tools/r8/desugar/constantdynamic/JacocoConstantDynamicGetDeclaredMethods.java
index 8524f5d..79fc834 100644
--- a/src/test/java/com/android/tools/r8/desugar/constantdynamic/JacocoConstantDynamicGetDeclaredMethods.java
+++ b/src/test/java/com/android/tools/r8/desugar/constantdynamic/JacocoConstantDynamicGetDeclaredMethods.java
@@ -49,20 +49,13 @@
public static JacocoClasses testClasses;
private static final String MAIN_CLASS = TestRunner.class.getTypeName();
- private static final String EXPECTED_OUTPUT_WITH_METHOD_HANDLES =
+ private static final String EXPECTED_OUTPUT =
StringUtils.lines(
jacocoBootstrapMethodName,
"3",
"java.lang.invoke.MethodHandles$Lookup",
"java.lang.String",
"java.lang.Class");
- private static final String EXPECTED_OUTPUT_WITHOUT_METHOD_HANDLES =
- StringUtils.lines(
- jacocoBootstrapMethodName,
- "3",
- "java.lang.Object",
- "java.lang.String",
- "java.lang.Class");
@BeforeClass
public static void setUpInput() throws IOException {
@@ -92,7 +85,7 @@
.addProgramFiles(testClasses.getOriginal())
.enableJaCoCoAgent(ToolHelper.JACOCO_AGENT, agentOutputOnTheFly)
.run(parameters.getRuntime(), MAIN_CLASS)
- .assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES);
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
checkJacocoReport(agentOutputOnTheFly);
// Run the instrumented code.
@@ -101,7 +94,7 @@
.addProgramFiles(testClasses.getInstrumented())
.configureJaCoCoAgentForOfflineInstrumentedCode(ToolHelper.JACOCO_AGENT, agentOutputOffline)
.run(parameters.getRuntime(), MAIN_CLASS)
- .assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES);
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
checkJacocoReport(agentOutputOffline);
}
@@ -115,10 +108,11 @@
.setMinApi(parameters.getApiLevel())
.compile()
.runWithJaCoCo(agentOutput, parameters.getRuntime(), MAIN_CLASS)
+ // TODO(b/210485236): This should never fail.
.applyIf(
- parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
- b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT_WITHOUT_METHOD_HANDLES),
- b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES));
+ parameters.getDexRuntimeVersion().isOlderThan(Version.V8_1_0),
+ b -> b.assertFailureWithErrorThatThrows(ClassNotFoundException.class),
+ b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT));
checkJacocoReport(agentOutput);
}
@@ -166,10 +160,11 @@
"javax.management.**")
.compile()
.runWithJaCoCo(agentOutput, parameters.getRuntime(), MAIN_CLASS)
+ // TODO(b/210485236): This should never fail.
.applyIf(
parameters.getDexRuntimeVersion().isOlderThan(Version.V8_1_0),
b -> b.assertFailureWithErrorThatThrows(ClassNotFoundException.class),
- b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT_WITH_METHOD_HANDLES));
+ b -> b.assertSuccessWithOutput(EXPECTED_OUTPUT));
checkJacocoReport(agentOutput);
}
