Reland "Rewrite holder and arguments for DexCallSites in Writer"
This reverts commit b7282efcc6ebc85d017213c0070e33ebe308452b.
Bug: 176486859
Change-Id: I3abef156b08b404454af352d3861452c6243d1e5
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 bce34ff..917f0de 100644
--- a/src/main/java/com/android/tools/r8/dex/VirtualFile.java
+++ b/src/main/java/com/android/tools/r8/dex/VirtualFile.java
@@ -221,6 +221,7 @@
graphLens,
namingLens,
initClassLens,
+ transaction.rewriter,
indexedItems.classes,
indexedItems.protos,
indexedItems.types,
@@ -747,7 +748,7 @@
this.graphLens = graphLens;
this.initClassLens = initClassLens;
this.namingLens = namingLens;
- this.rewriter = new LensCodeRewriterUtils(appView);
+ this.rewriter = new LensCodeRewriterUtils(appView, true);
}
private <T extends DexItem> boolean maybeInsert(T item, Set<T> set, Set<T> baseSet) {
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 f315247..196bc31 100644
--- a/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java
+++ b/src/main/java/com/android/tools/r8/graph/ObjectToOffsetMapping.java
@@ -53,6 +53,7 @@
GraphLens graphLens,
NamingLens namingLens,
InitClassLens initClassLens,
+ LensCodeRewriterUtils lensCodeRewriter,
Collection<DexProgramClass> classes,
Collection<DexProto> protos,
Collection<DexType> types,
@@ -76,7 +77,7 @@
this.graphLens = graphLens;
this.namingLens = namingLens;
this.initClassLens = initClassLens;
- this.lensCodeRewriter = new LensCodeRewriterUtils(appView);
+ this.lensCodeRewriter = lensCodeRewriter;
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 ae5e3f9..4bc8714 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,16 +39,26 @@
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;
+
public LensCodeRewriterUtils(AppView<?> appView) {
+ this(appView, false);
+ }
+
+ public LensCodeRewriterUtils(AppView<?> appView, boolean enableCallSiteCaching) {
this.appView = appView;
this.definitions = appView;
this.graphLens = null;
+ this.rewrittenCallSiteCache = enableCallSiteCaching ? new ConcurrentHashMap<>() : null;
}
public LensCodeRewriterUtils(DexDefinitionSupplier definitions, GraphLens graphLens) {
this.appView = null;
this.definitions = definitions;
this.graphLens = graphLens;
+ this.rewrittenCallSiteCache = null;
}
private GraphLens graphLens() {
@@ -56,6 +66,14 @@
}
public DexCallSite rewriteCallSite(DexCallSite callSite, ProgramMethod context) {
+ if (rewrittenCallSiteCache == null) {
+ return rewriteCallSiteInternal(callSite, context);
+ }
+ return rewrittenCallSiteCache.computeIfAbsent(
+ callSite, ignored -> rewriteCallSiteInternal(callSite, context));
+ }
+
+ private DexCallSite rewriteCallSiteInternal(DexCallSite callSite, ProgramMethod context) {
DexItemFactory dexItemFactory = definitions.dexItemFactory();
DexProto newMethodProto = rewriteProto(callSite.methodProto);
DexMethodHandle newBootstrapMethod =
@@ -96,11 +114,15 @@
// 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.registerMethodHandle).
+ // with a keep rule (see Enqueuer.traceMethodHandle).
+ // Note that the member can be repackaged or minified.
actualTarget =
definitions
.dexItemFactory()
- .createMethod(invokedMethod.holder, rewrittenTarget.proto, rewrittenTarget.name);
+ .createMethod(
+ graphLens().lookupType(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 d2a5729..330535e 100644
--- a/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java
+++ b/src/test/java/com/android/tools/r8/dex/DebugByteCodeWriterTest.java
@@ -17,6 +17,7 @@
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;
@@ -41,15 +42,18 @@
}
private ObjectToOffsetMapping emptyObjectTObjectMapping() {
- return new ObjectToOffsetMapping(
+ AppView<AppInfo> appView =
AppView.createForD8(
AppInfo.createInitialAppInfo(
DexApplication.builder(
new InternalOptions(new DexItemFactory(), new Reporter()), null)
- .build())),
+ .build()));
+ return new ObjectToOffsetMapping(
+ appView,
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 a97ecf1..f20603d 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageCustomMethodHandleTest.java
@@ -4,12 +4,10 @@
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.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
@@ -36,6 +34,8 @@
@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,29 +59,25 @@
.addProgramClassFileData(
transformer(Main.class).addClassTransformer(generateCallSiteInvoke()).transform())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("InvokeCustom::foo");
+ .assertSuccessWithOutputLines(EXPECTED);
}
- @Test(expected = CompilationFailedException.class)
+ @Test()
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())
- .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())));
- });
+ .compile()
+ .inspect(inspector -> assertThat(InvokeCustom.class, isRepackaged(inspector)))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
private ClassTransformer generateCallSiteInvoke() {
