Revert "Rewrite holder and arguments for DexCallSites in Writer"
This reverts commit 9253506600c462e37575f709ec5f3ef9e6e1c079.
Reason for revert: Uncovered a bug in the EnumUnboxer
Change-Id: I87aad35b91fd92e0bf878f66464c27c7c3613178
diff --git a/src/main/java/com/android/tools/r8/dex/VirtualFile.java b/src/main/java/com/android/tools/r8/dex/VirtualFile.java
index 983e4eb..bce34ff 100644
--- a/src/main/java/com/android/tools/r8/dex/VirtualFile.java
+++ b/src/main/java/com/android/tools/r8/dex/VirtualFile.java
@@ -221,7 +221,6 @@
graphLens,
namingLens,
initClassLens,
- transaction.rewriter,
indexedItems.classes,
indexedItems.protos,
indexedItems.types,
diff --git a/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java b/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java
index 196bc31..f315247 100644
--- a/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java
+++ b/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java
@@ -53,7 +53,6 @@
GraphLens graphLens,
NamingLens namingLens,
InitClassLens initClassLens,
- LensCodeRewriterUtils lensCodeRewriter,
Collection<DexProgramClass> classes,
Collection<DexProto> protos,
Collection<DexType> types,
@@ -77,7 +76,7 @@
this.graphLens = graphLens;
this.namingLens = namingLens;
this.initClassLens = initClassLens;
- this.lensCodeRewriter = lensCodeRewriter;
+ this.lensCodeRewriter = new LensCodeRewriterUtils(appView);
timing.begin("Sort strings");
this.strings = createSortedMap(strings, DexString::compareTo, this::setFirstJumboString);
CompareToVisitor visitor =
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriterUtils.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriterUtils.java
index 39f7dd1..ae5e3f9 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriterUtils.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriterUtils.java
@@ -39,10 +39,6 @@
private final Map<DexProto, DexProto> protoFixupCache = new ConcurrentHashMap<>();
- // Map from original call sites to rewritten call sites to lookup call-sites when writing.
- // TODO(b/176885013): This is redundant if we canonicalize call sites.
- private final Map<DexCallSite, DexCallSite> rewrittenCallSiteCache = new ConcurrentHashMap<>();
-
public LensCodeRewriterUtils(AppView<?> appView) {
this.appView = appView;
this.definitions = appView;
@@ -60,30 +56,24 @@
}
public DexCallSite rewriteCallSite(DexCallSite callSite, ProgramMethod context) {
- return rewrittenCallSiteCache.computeIfAbsent(
- callSite,
- ignored -> {
- DexItemFactory dexItemFactory = definitions.dexItemFactory();
- DexProto newMethodProto = rewriteProto(callSite.methodProto);
- DexMethodHandle newBootstrapMethod =
- rewriteDexMethodHandle(
- callSite.bootstrapMethod, NOT_ARGUMENT_TO_LAMBDA_METAFACTORY, context);
- boolean isLambdaMetaFactory =
- dexItemFactory.isLambdaMetafactoryMethod(callSite.bootstrapMethod.asMethod());
- MethodHandleUse methodHandleUse =
- isLambdaMetaFactory
- ? ARGUMENT_TO_LAMBDA_METAFACTORY
- : NOT_ARGUMENT_TO_LAMBDA_METAFACTORY;
- List<DexValue> newArgs =
- rewriteBootstrapArguments(callSite.bootstrapArgs, methodHandleUse, context);
- if (!newMethodProto.equals(callSite.methodProto)
- || newBootstrapMethod != callSite.bootstrapMethod
- || !newArgs.equals(callSite.bootstrapArgs)) {
- return dexItemFactory.createCallSite(
- callSite.methodName, newMethodProto, newBootstrapMethod, newArgs);
- }
- return callSite;
- });
+ DexItemFactory dexItemFactory = definitions.dexItemFactory();
+ DexProto newMethodProto = rewriteProto(callSite.methodProto);
+ DexMethodHandle newBootstrapMethod =
+ rewriteDexMethodHandle(
+ callSite.bootstrapMethod, NOT_ARGUMENT_TO_LAMBDA_METAFACTORY, context);
+ boolean isLambdaMetaFactory =
+ dexItemFactory.isLambdaMetafactoryMethod(callSite.bootstrapMethod.asMethod());
+ MethodHandleUse methodHandleUse =
+ isLambdaMetaFactory ? ARGUMENT_TO_LAMBDA_METAFACTORY : NOT_ARGUMENT_TO_LAMBDA_METAFACTORY;
+ List<DexValue> newArgs =
+ rewriteBootstrapArguments(callSite.bootstrapArgs, methodHandleUse, context);
+ if (!newMethodProto.equals(callSite.methodProto)
+ || newBootstrapMethod != callSite.bootstrapMethod
+ || !newArgs.equals(callSite.bootstrapArgs)) {
+ return dexItemFactory.createCallSite(
+ callSite.methodName, newMethodProto, newBootstrapMethod, newArgs);
+ }
+ return callSite;
}
public DexMethodHandle rewriteDexMethodHandle(
@@ -106,15 +96,11 @@
// MethodHandles that are not arguments to a lambda metafactory will not be desugared
// away. Therefore they could flow to a MethodHandle.invokeExact call which means that
// we cannot member rebind. We therefore keep the receiver and also pin the receiver
- // with a keep rule (see Enqueuer.traceMethodHandle).
- // Note that the member can be repackaged or minified.
+ // with a keep rule (see Enqueuer.registerMethodHandle).
actualTarget =
definitions
.dexItemFactory()
- .createMethod(
- graphLens().lookupType(invokedMethod.holder),
- rewrittenTarget.proto,
- rewrittenTarget.name);
+ .createMethod(invokedMethod.holder, rewrittenTarget.proto, rewrittenTarget.name);
newType = oldType;
if (oldType.isInvokeDirect()) {
// For an invoke direct, the rewritten target must have the same holder as the original.
diff --git a/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java b/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java
index 330535e..d2a5729 100644
--- a/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java
+++ b/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.graph.GraphLens;
import com.android.tools.r8.graph.InitClassLens;
import com.android.tools.r8.graph.ObjectToOffsetMapping;
-import com.android.tools.r8.ir.conversion.LensCodeRewriterUtils;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
@@ -42,18 +41,15 @@
}
private ObjectToOffsetMapping emptyObjectTObjectMapping() {
- AppView<AppInfo> appView =
+ return new ObjectToOffsetMapping(
AppView.createForD8(
AppInfo.createInitialAppInfo(
DexApplication.builder(
new InternalOptions(new DexItemFactory(), new Reporter()), null)
- .build()));
- return new ObjectToOffsetMapping(
- appView,
+ .build())),
GraphLens.getIdentityLens(),
NamingLens.getIdentityLens(),
InitClassLens.getDefault(),
- new LensCodeRewriterUtils(appView),
Collections.emptyList(),
Collections.emptyList(),
Collections.emptyList(),
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java
index f20603d..a97ecf1 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java
@@ -4,10 +4,12 @@
package com.android.tools.r8.repackage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
import static com.android.tools.r8.shaking.ProguardConfigurationParser.FLATTEN_PACKAGE_HIERARCHY;
import static com.android.tools.r8.shaking.ProguardConfigurationParser.REPACKAGE_CLASSES;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.CoreMatchers.containsString;
+import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
@@ -34,8 +36,6 @@
@RunWith(Parameterized.class)
public class RepackageCustomMethodHandleTest extends RepackageTestBase {
- private static final String EXPECTED = "InvokeCustom::foo";
-
@Parameters(name = "{1}, kind: {0}")
public static List<Object[]> data() {
return buildParameters(
@@ -59,25 +59,29 @@
.addProgramClassFileData(
transformer(Main.class).addClassTransformer(generateCallSiteInvoke()).transform())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(EXPECTED);
+ .assertSuccessWithOutputLines("InvokeCustom::foo");
}
- @Test()
+ @Test(expected = CompilationFailedException.class)
public void testR8() throws Exception {
testForR8(parameters.getBackend())
.addProgramClasses(InvokeCustom.class)
.addProgramClassFileData(
transformer(Main.class).addClassTransformer(generateCallSiteInvoke()).transform())
.addKeepMainRule(Main.class)
- .addKeepClassAndMembersRules(Main.class)
.apply(this::configureRepackaging)
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.setMinApi(parameters.getApiLevel())
- .compile()
- .inspect(inspector -> assertThat(InvokeCustom.class, isRepackaged(inspector)))
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(EXPECTED);
+ .compileWithExpectedDiagnostics(
+ diagnostics -> {
+ // TODO(b/176486859): This should not lookup the original packaged name.
+ diagnostics.assertErrorsMatch(
+ diagnosticMessage(
+ containsString(
+ "Failed lookup of non-missing type: "
+ + InvokeCustom.class.getTypeName())));
+ });
}
private ClassTransformer generateCallSiteInvoke() {
