Merge "Add tests for Proguard's behavior regarding -allowaccessmodification."
diff --git a/LIBRARY-LICENSE b/LIBRARY-LICENSE
index bc69c40..12674e3 100644
--- a/LIBRARY-LICENSE
+++ b/LIBRARY-LICENSE
@@ -48,21 +48,3 @@
license: ASM license
licenseUrl: http://asm.ow2.org/license.html
url: http://asm.ow2.org/index.html
-- artifact: org.jetbrains.kotlin:kotlin-stdlib:+
- name: org.jetbrains.kotlin:kotlin-stdlib
- copyrightHolder: JetBrains s.r.o.
- license: The Apache License, Version 2.0
- licenseUrl: http://www.apache.org/licenses/LICENSE-2.0.txt
- url: https://kotlinlang.org/
-- artifact: org.jetbrains.kotlinx:kotlinx-metadata-jvm:+
- name: org.jetbrains.kotlinx:kotlinx-metadata-jvm
- copyrightHolder: JetBrains s.r.o.
- license: The Apache License, Version 2.0
- licenseUrl: http://www.apache.org/licenses/LICENSE-2.0.txt
- url: https://kotlinlang.org/
-- artifact: org.jetbrains:annotations:+
- name: IntelliJ IDEA Annotations
- copyrightHolder: JetBrains s.r.o.
- license: The Apache Software License, Version 2.0
- licenseUrl: http://www.apache.org/licenses/LICENSE-2.0.txt
- url: http://www.jetbrains.org
diff --git a/build.gradle b/build.gradle
index 7f41caa..92d7471 100644
--- a/build.gradle
+++ b/build.gradle
@@ -37,7 +37,6 @@
gsonVersion = '2.7'
junitVersion = '4.12'
kotlinVersion = '1.2.30'
- kotlinExtMetadataJVMVersion = '0.0.2'
protobufVersion = '3.0.0'
smaliVersion = '2.2b4'
}
@@ -75,7 +74,6 @@
repositories {
maven { url 'https://maven.google.com' }
- maven { url 'https://kotlin.bintray.com/kotlinx' }
mavenCentral()
}
@@ -225,7 +223,6 @@
exclude group: 'org.codehaus.mojo'
})
compile group: 'it.unimi.dsi', name: 'fastutil', version: fastutilVersion
- compile "org.jetbrains.kotlinx:kotlinx-metadata-jvm:$kotlinExtMetadataJVMVersion"
compile group: 'org.ow2.asm', name: 'asm', version: asmVersion
compile group: 'org.ow2.asm', name: 'asm-commons', version: asmVersion
compile group: 'org.ow2.asm', name: 'asm-tree', version: asmVersion
diff --git a/src/main/java/com/android/tools/r8/ArchiveClassFileProvider.java b/src/main/java/com/android/tools/r8/ArchiveClassFileProvider.java
index b3d59fb..af83158 100644
--- a/src/main/java/com/android/tools/r8/ArchiveClassFileProvider.java
+++ b/src/main/java/com/android/tools/r8/ArchiveClassFileProvider.java
@@ -17,6 +17,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
@@ -60,7 +61,7 @@
assert isArchive(archive);
origin = new PathOrigin(archive);
try {
- zipFile = new ZipFile(archive.toFile());
+ zipFile = new ZipFile(archive.toFile(), StandardCharsets.UTF_8);
} catch (IOException e) {
if (!Files.exists(archive)) {
throw new NoSuchFileException(archive.toString());
diff --git a/src/main/java/com/android/tools/r8/ArchiveProgramResourceProvider.java b/src/main/java/com/android/tools/r8/ArchiveProgramResourceProvider.java
index baac9bc..425b3c3 100644
--- a/src/main/java/com/android/tools/r8/ArchiveProgramResourceProvider.java
+++ b/src/main/java/com/android/tools/r8/ArchiveProgramResourceProvider.java
@@ -16,6 +16,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -60,7 +61,10 @@
public static ArchiveProgramResourceProvider fromArchive(
Path archive, Predicate<String> include) {
- return fromSupplier(new PathOrigin(archive), () -> new ZipFile(archive.toFile()), include);
+ return fromSupplier(
+ new PathOrigin(archive),
+ () -> new ZipFile(archive.toFile(), StandardCharsets.UTF_8),
+ include);
}
public static ArchiveProgramResourceProvider fromSupplier(
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 2e0b531..d587efe 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -42,8 +42,8 @@
import com.android.tools.r8.shaking.ReasonPrinter;
import com.android.tools.r8.shaking.RootSetBuilder;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
-import com.android.tools.r8.shaking.SimpleClassMerger;
import com.android.tools.r8.shaking.TreePruner;
+import com.android.tools.r8.shaking.VerticalClassMerger;
import com.android.tools.r8.shaking.protolite.ProtoLiteExtension;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
@@ -335,8 +335,8 @@
// Class merging requires inlining.
if (options.enableClassMerging && options.enableInlining) {
timing.begin("ClassMerger");
- SimpleClassMerger classMerger = new SimpleClassMerger(application,
- appInfo.withLiveness(), graphLense, timing);
+ VerticalClassMerger classMerger =
+ new VerticalClassMerger(application, appInfo.withLiveness(), graphLense, timing);
graphLense = classMerger.run();
timing.end();
@@ -344,11 +344,11 @@
.prunedCopyFrom(application, classMerger.getRemovedClasses());
}
if (options.proguardConfiguration.hasApplyMappingFile()) {
- SeedMapper seedMapper = SeedMapper.seedMapperFromFile(
- options.proguardConfiguration.getApplyMappingFile());
+ SeedMapper seedMapper =
+ SeedMapper.seedMapperFromFile(options.proguardConfiguration.getApplyMappingFile());
timing.begin("apply-mapping");
- graphLense = new ProguardMapApplier(appInfo.withLiveness(), graphLense, seedMapper)
- .run(timing);
+ graphLense =
+ new ProguardMapApplier(appInfo.withLiveness(), graphLense, seedMapper).run(timing);
timing.end();
}
application = application.asDirect().rewrittenWithLense(graphLense);
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index fb3b067..b775fd3 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 = "1.2.21-dev";
+ public static final String LABEL = "1.3.0-dev";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java b/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
index 90b863c..2621979 100644
--- a/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
+++ b/src/main/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilder.java
@@ -17,6 +17,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -127,7 +128,7 @@
List<ZipEntry> toDex = new ArrayList<>();
- try (ZipFile zipFile = new ZipFile(input)) {
+ try (ZipFile zipFile = new ZipFile(input, StandardCharsets.UTF_8)) {
final Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
diff --git a/src/main/java/com/android/tools/r8/compatdx/CompatDx.java b/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
index f512a34..871f6d8 100644
--- a/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
+++ b/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -572,7 +573,7 @@
// For each input archive file, add all class files within.
for (Path input : inputs) {
if (isArchive(input)) {
- try (ZipFile zipFile = new ZipFile(input.toFile())) {
+ try (ZipFile zipFile = new ZipFile(input.toFile(), StandardCharsets.UTF_8)) {
final Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index f8e82fb..026c2b5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -264,10 +264,9 @@
public void setCode(
IRCode ir,
- GraphLense graphLense,
RegisterAllocator registerAllocator,
InternalOptions options) {
- final DexBuilder builder = new DexBuilder(ir, graphLense, registerAllocator, options);
+ final DexBuilder builder = new DexBuilder(ir, registerAllocator, options);
code = builder.build(method.getArity());
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
index 06ea28d..cd3c093 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethodHandle.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.ir.code.Invoke.Type;
import com.android.tools.r8.naming.NamingLens;
import org.objectweb.asm.Handle;
import org.objectweb.asm.Opcodes;
@@ -159,6 +160,26 @@
public boolean isInvokeConstructor() {
return this == MethodHandleType.INVOKE_CONSTRUCTOR;
}
+
+ public Type toInvokeType() {
+ assert isMethodType();
+ switch (this) {
+ case INVOKE_STATIC:
+ return Type.STATIC;
+ case INVOKE_INSTANCE:
+ return Type.VIRTUAL;
+ case INVOKE_CONSTRUCTOR:
+ return Type.DIRECT;
+ case INVOKE_DIRECT:
+ return Type.DIRECT;
+ case INVOKE_INTERFACE:
+ return Type.INTERFACE;
+ case INVOKE_SUPER:
+ return Type.SUPER;
+ default:
+ throw new Unreachable("DexMethodHandle with unexpected type: " + this);
+ }
+ }
}
public MethodHandleType type;
diff --git a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
index 8d5f0d0..e4f1c8c 100644
--- a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
@@ -69,7 +69,6 @@
}
public DirectMappedDexApplication rewrittenWithLense(GraphLense graphLense) {
- assert graphLense.isContextFree();
assert mappingIsValid(graphLense, programClasses.getAllTypes());
assert mappingIsValid(graphLense, libraryClasses.keySet());
// As a side effect, this will rebuild the program classes and library classes maps.
@@ -81,7 +80,7 @@
// (e.g. relinking a type) or it might encode a type that was renamed, in which case the
// original type will point to a definition that was renamed.
for (DexType type : types) {
- DexType renamed = graphLense.lookupType(type, null);
+ DexType renamed = graphLense.lookupType(type);
if (renamed != type) {
if (definitionFor(type).type != renamed && definitionFor(renamed) == null) {
return false;
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index ed4106e..096de5a 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -3,8 +3,12 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.google.common.collect.ImmutableSet;
+import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Map;
+import java.util.Set;
/**
* A GraphLense implements a virtual view on top of the graph, used to delay global rewrites until
@@ -59,13 +63,33 @@
return new Builder();
}
- public abstract DexType lookupType(DexType type, DexEncodedMethod context);
+ public abstract DexType lookupType(DexType type);
- public abstract DexMethod lookupMethod(DexMethod method, DexEncodedMethod context);
+ // This overload can be used when the graph lense is known to be context insensitive.
+ public DexMethod lookupMethod(DexMethod method) {
+ assert isContextFreeForMethod(method);
+ return lookupMethod(method, null, null);
+ }
- public abstract DexField lookupField(DexField field, DexEncodedMethod context);
+ public abstract DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type);
- public abstract boolean isContextFree();
+ // Context sensitive graph lenses should override this method.
+ public Set<DexMethod> lookupMethodInAllContexts(DexMethod method) {
+ assert isContextFreeForMethod(method);
+ DexMethod result = lookupMethod(method);
+ if (result != null) {
+ return ImmutableSet.of(result);
+ }
+ return ImmutableSet.of();
+ }
+
+ public abstract DexField lookupField(DexField field);
+
+ public abstract boolean isContextFreeForMethods();
+
+ public boolean isContextFreeForMethod(DexMethod method) {
+ return isContextFreeForMethods();
+ }
public static GraphLense getIdentityLense() {
return new IdentityGraphLense();
@@ -78,22 +102,22 @@
private static class IdentityGraphLense extends GraphLense {
@Override
- public DexType lookupType(DexType type, DexEncodedMethod context) {
+ public DexType lookupType(DexType type) {
return type;
}
@Override
- public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context) {
+ public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
return method;
}
@Override
- public DexField lookupField(DexField field, DexEncodedMethod context) {
+ public DexField lookupField(DexField field) {
return field;
}
@Override
- public boolean isContextFree() {
+ public boolean isContextFreeForMethods() {
return true;
}
}
@@ -118,14 +142,14 @@
}
@Override
- public DexType lookupType(DexType type, DexEncodedMethod context) {
+ public DexType lookupType(DexType type) {
if (type.isArrayType()) {
synchronized (this) {
// This block need to be synchronized due to arrayTypeCache.
DexType result = arrayTypeCache.get(type);
if (result == null) {
DexType baseType = type.toBaseType(dexItemFactory);
- DexType newType = lookupType(baseType, context);
+ DexType newType = lookupType(baseType);
if (baseType == newType) {
result = type;
} else {
@@ -136,25 +160,39 @@
return result;
}
}
- DexType previous = previousLense.lookupType(type, context);
+ DexType previous = previousLense.lookupType(type);
return typeMap.getOrDefault(previous, previous);
}
@Override
- public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context) {
- DexMethod previous = previousLense.lookupMethod(method, context);
+ public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
+ DexMethod previous = previousLense.lookupMethod(method, context, type);
return methodMap.getOrDefault(previous, previous);
}
@Override
- public DexField lookupField(DexField field, DexEncodedMethod context) {
- DexField previous = previousLense.lookupField(field, context);
+ public Set<DexMethod> lookupMethodInAllContexts(DexMethod method) {
+ Set<DexMethod> result = new HashSet<>();
+ for (DexMethod previous : previousLense.lookupMethodInAllContexts(method)) {
+ result.add(methodMap.getOrDefault(previous, previous));
+ }
+ return result;
+ }
+
+ @Override
+ public DexField lookupField(DexField field) {
+ DexField previous = previousLense.lookupField(field);
return fieldMap.getOrDefault(previous, previous);
}
@Override
- public boolean isContextFree() {
- return previousLense.isContextFree();
+ public boolean isContextFreeForMethods() {
+ return previousLense.isContextFreeForMethods();
+ }
+
+ @Override
+ public boolean isContextFreeForMethod(DexMethod method) {
+ return previousLense.isContextFreeForMethod(method);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 113d522..819c6f2 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -6,10 +6,11 @@
import static com.android.tools.r8.ir.code.IRCode.INSTRUCTION_NUMBER_DELTA;
import com.android.tools.r8.errors.CompilationError;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DebugLocalInfo.PrintLevel;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.conversion.IRBuilder;
import com.android.tools.r8.utils.CfgPrinter;
@@ -20,10 +21,13 @@
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableList;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.IntArrayList;
+import it.unimi.dsi.fastutil.ints.IntList;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
@@ -173,7 +177,7 @@
public void removeSuccessor(BasicBlock block) {
int index = successors.indexOf(block);
assert index >= 0 : "removeSuccessor did not find the successor to remove";
- removeSuccessorsByIndex(Collections.singletonList(index));
+ removeSuccessorsByIndex(new IntArrayList(new int[] {index}));
}
public void removePredecessor(BasicBlock block) {
@@ -328,7 +332,7 @@
assert false : "replaceSuccessor did not find the predecessor to replace";
}
- public void removeSuccessorsByIndex(List<Integer> successorsToRemove) {
+ public void removeSuccessorsByIndex(IntList successorsToRemove) {
if (successorsToRemove.isEmpty()) {
return;
}
@@ -463,7 +467,7 @@
return instruction;
}
}
- throw new Unreachable();
+ return null;
}
public void clearUserInfo() {
@@ -678,6 +682,44 @@
catchHandlers = new CatchHandlers<>(guards, successorIndexes);
}
+ // Due to class merging, it is possible that two exception classes have been merged into one.
+ // This function renames the guards according to the given graph lense.
+ public void renameGuardsInCatchHandlers(GraphLense graphLense) {
+ assert hasCatchHandlers();
+ List<DexType> newGuards = new ArrayList<>(catchHandlers.getGuards().size());
+ for (DexType guard : catchHandlers.getGuards()) {
+ // The type may have changed due to class merging.
+ newGuards.add(graphLense.lookupType(guard));
+ }
+ this.catchHandlers = new CatchHandlers<>(newGuards, catchHandlers.getAllTargets());
+ }
+
+ public boolean consistentCatchHandlers() {
+ // Check that catch handlers are always the first successors of a block.
+ if (hasCatchHandlers()) {
+ assert exit().isGoto() || exit().isThrow();
+ CatchHandlers<Integer> catchHandlers = getCatchHandlersWithSuccessorIndexes();
+ // If there is a catch-all guard it must be the last.
+ List<DexType> guards = catchHandlers.getGuards();
+ int lastGuardIndex = guards.size() - 1;
+ for (int i = 0; i < guards.size(); i++) {
+ assert guards.get(i) != DexItemFactory.catchAllType || i == lastGuardIndex;
+ }
+ // Check that all successors except maybe the last are catch successors.
+ List<Integer> sortedHandlerIndices = new ArrayList<>(catchHandlers.getAllTargets());
+ sortedHandlerIndices.sort(Comparator.naturalOrder());
+ int firstIndex = sortedHandlerIndices.get(0);
+ int lastIndex = sortedHandlerIndices.get(sortedHandlerIndices.size() - 1);
+ assert firstIndex == 0;
+ assert lastIndex < sortedHandlerIndices.size();
+ int lastSuccessorIndex = getSuccessors().size() - 1;
+ assert lastIndex == lastSuccessorIndex // All successors are catch successors.
+ || lastIndex == lastSuccessorIndex - 1; // All but one successors are catch successors.
+ assert lastIndex == lastSuccessorIndex || !exit().isThrow();
+ }
+ return true;
+ }
+
public void clearCurrentDefinitions() {
currentDefinitions = null;
for (Phi phi : getPhis()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index a5d3e4a..c832991 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -5,8 +5,6 @@
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexItemFactory;
-import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
@@ -473,28 +471,7 @@
private boolean consistentCatchHandlers() {
for (BasicBlock block : blocks) {
- // Check that catch handlers are always the first successors of a block.
- if (block.hasCatchHandlers()) {
- assert block.exit().isGoto() || block.exit().isThrow();
- CatchHandlers<Integer> catchHandlers = block.getCatchHandlersWithSuccessorIndexes();
- // If there is a catch-all guard it must be the last.
- List<DexType> guards = catchHandlers.getGuards();
- int lastGuardIndex = guards.size() - 1;
- for (int i = 0; i < guards.size(); i++) {
- assert guards.get(i) != DexItemFactory.catchAllType || i == lastGuardIndex;
- }
- // Check that all successors except maybe the last are catch successors.
- List<Integer> sortedHandlerIndices = new ArrayList<>(catchHandlers.getAllTargets());
- sortedHandlerIndices.sort(Comparator.naturalOrder());
- int firstIndex = sortedHandlerIndices.get(0);
- int lastIndex = sortedHandlerIndices.get(sortedHandlerIndices.size() - 1);
- assert firstIndex == 0;
- assert lastIndex < sortedHandlerIndices.size();
- int lastSuccessorIndex = block.getSuccessors().size() - 1;
- assert lastIndex == lastSuccessorIndex // All successors are catch successors.
- || lastIndex == lastSuccessorIndex - 1; // All but one successors are catch successors.
- assert lastIndex == lastSuccessorIndex || !block.exit().isThrow();
- }
+ assert block.consistentCatchHandlers();
}
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
index 8330e06..21b8dc4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
@@ -363,7 +363,7 @@
private void processInvoke(Type type, DexMethod method) {
DexEncodedMethod source = caller.method;
- method = graphLense.lookupMethod(method, source);
+ method = graphLense.lookupMethod(method, source, type);
DexEncodedMethod definition = appInfo.lookup(type, method, source.method.holder);
if (definition != null) {
assert !source.accessFlags.isBridge() || definition != caller.method;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index e798fd2..de86cb4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.ir.code.Argument;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
@@ -118,13 +119,16 @@
}
public CfCode build(
- CodeRewriter rewriter, InternalOptions options, AppInfoWithSubtyping appInfo) {
+ CodeRewriter rewriter,
+ GraphLense graphLense,
+ InternalOptions options,
+ AppInfoWithSubtyping appInfo) {
computeInitializers();
types = new TypeVerificationHelper(code, factory, appInfo).computeVerificationTypes();
splitExceptionalBlocks();
LoadStoreHelper loadStoreHelper = new LoadStoreHelper(code, types);
loadStoreHelper.insertLoadsAndStores();
- DeadCodeRemover.removeDeadCode(code, rewriter, options);
+ DeadCodeRemover.removeDeadCode(code, rewriter, graphLense, options);
removeUnneededLoadsAndStores();
registerAllocator = new CfRegisterAllocator(code, options);
registerAllocator.allocateRegisters();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index ee7df48..8201562 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -43,7 +43,6 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.ir.code.Argument;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
@@ -82,10 +81,6 @@
// The IR representation of the code to build.
private final IRCode ir;
- // Graph lense for building the exception handlers. Needed since program classes that inherit
- // from Throwable may get merged into their subtypes during class merging.
- private final GraphLense graphLense;
-
// The register allocator providing register assignments for the code to build.
private final RegisterAllocator registerAllocator;
@@ -121,14 +116,11 @@
public DexBuilder(
IRCode ir,
- GraphLense graphLense,
RegisterAllocator registerAllocator,
InternalOptions options) {
assert ir != null;
- assert graphLense != null;
assert registerAllocator != null;
this.ir = ir;
- this.graphLense = graphLense;
this.registerAllocator = registerAllocator;
this.options = options;
}
@@ -714,12 +706,7 @@
assert i == handlerGroup.getGuards().size() - 1;
catchAllOffset = targetOffset;
} else {
- // The type may have changed due to class merging.
- // TODO(christofferqa): This assumes that the graph lense is context insensitive for the
- // given type (which is always the case). Consider removing the context-argument from
- // GraphLense.lookupType, since we do not currently have a use case for it.
- DexType actualType = graphLense.lookupType(type, null);
- pairs.add(new TypeAddrPair(actualType, targetOffset));
+ pairs.add(new TypeAddrPair(type, targetOffset));
}
}
TypeAddrPair[] pairsArray = pairs.toArray(new TypeAddrPair[pairs.size()]);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index df4e69c..7d7e5f3 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -46,6 +46,7 @@
import com.android.tools.r8.ir.optimize.NonNullTracker;
import com.android.tools.r8.ir.optimize.Outliner;
import com.android.tools.r8.ir.optimize.PeepholeOptimizer;
+import com.android.tools.r8.ir.optimize.RedundantFieldLoadElimination;
import com.android.tools.r8.ir.optimize.classinliner.ClassInliner;
import com.android.tools.r8.ir.optimize.lambda.LambdaMerger;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
@@ -140,7 +141,7 @@
this.memberValuePropagation =
options.enableValuePropagation ?
new MemberValuePropagation(appInfo.withLiveness()) : null;
- this.lensCodeRewriter = new LensCodeRewriter(graphLense, appInfo.withSubtyping());
+ this.lensCodeRewriter = new LensCodeRewriter(graphLense, appInfo.withSubtyping(), options);
if (appInfo.hasLiveness()) {
// When disabling the pruner here, also disable the ProtoLiteExtension in R8.java.
this.protoLiteRewriter =
@@ -482,7 +483,7 @@
// StringBuilder/StringBuffer method invocations, and removeDeadCode() to remove
// unused out-values.
codeRewriter.rewriteMoveResult(code);
- DeadCodeRemover.removeDeadCode(code, codeRewriter, options);
+ DeadCodeRemover.removeDeadCode(code, codeRewriter, graphLense, options);
consumer.accept(code, method);
return null;
}));
@@ -532,7 +533,7 @@
assert code.isConsistentSSA();
code.traceBlocks();
RegisterAllocator registerAllocator = performRegisterAllocation(code, method);
- method.setCode(code, graphLense, registerAllocator, options);
+ method.setCode(code, registerAllocator, options);
if (Log.ENABLED) {
Log.debug(getClass(), "Resulting dex code for %s:\n%s",
method.toSourceString(), logCode(options, method));
@@ -686,6 +687,7 @@
codeRewriter.rewriteSwitch(code);
codeRewriter.processMethodsNeverReturningNormally(code);
codeRewriter.simplifyIf(code, typeEnvironment);
+ new RedundantFieldLoadElimination(code).run();
if (options.testing.invertConditionals) {
invertConditionalsForTesting(code);
@@ -705,7 +707,7 @@
// Dead code removal. Performed after simplifications to remove code that becomes dead
// as a result of those simplifications. The following optimizations could reveal more
// dead code which is removed right before register allocation in performRegisterAllocation.
- DeadCodeRemover.removeDeadCode(code, codeRewriter, options);
+ DeadCodeRemover.removeDeadCode(code, codeRewriter, graphLense, options);
assert code.isConsistentSSA();
if (options.enableDesugaring && enableTryWithResourcesDesugaring()) {
@@ -776,7 +778,7 @@
private void finalizeToCf(DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
assert !method.getCode().isDexCode();
CfBuilder builder = new CfBuilder(method, code, options.itemFactory);
- CfCode result = builder.build(codeRewriter, options, appInfo.withSubtyping());
+ CfCode result = builder.build(codeRewriter, graphLense, options, appInfo.withSubtyping());
method.setCode(result);
markProcessed(method, code, feedback);
}
@@ -784,7 +786,7 @@
private void finalizeToDex(DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
// Perform register allocation.
RegisterAllocator registerAllocator = performRegisterAllocation(code, method);
- method.setCode(code, graphLense, registerAllocator, options);
+ method.setCode(code, registerAllocator, options);
updateHighestSortingStrings(method);
if (Log.ENABLED) {
Log.debug(getClass(), "Resulting dex code for %s:\n%s",
@@ -818,7 +820,7 @@
private RegisterAllocator performRegisterAllocation(IRCode code, DexEncodedMethod method) {
// Always perform dead code elimination before register allocation. The register allocator
// does not allow dead code (to make sure that we do not waste registers for unneeded values).
- DeadCodeRemover.removeDeadCode(code, codeRewriter, options);
+ DeadCodeRemover.removeDeadCode(code, codeRewriter, graphLense, options);
materializeInstructionBeforeLongOperationsWorkaround(code, options);
LinearScanRegisterAllocator registerAllocator = new LinearScanRegisterAllocator(code, options);
registerAllocator.allocateRegisters(options.debug);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index cdb419f..0e4be8b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -34,6 +34,7 @@
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.StaticPut;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.List;
import java.util.ListIterator;
import java.util.stream.Collectors;
@@ -42,10 +43,13 @@
private final GraphLense graphLense;
private final AppInfoWithSubtyping appInfo;
+ private final InternalOptions options;
- public LensCodeRewriter(GraphLense graphLense, AppInfoWithSubtyping appInfo) {
+ public LensCodeRewriter(
+ GraphLense graphLense, AppInfoWithSubtyping appInfo, InternalOptions options) {
this.graphLense = graphLense;
this.appInfo = appInfo;
+ this.options = options;
}
private Value makeOutValue(Instruction insn, IRCode code) {
@@ -96,8 +100,8 @@
if (!invokedHolder.isClassType()) {
continue;
}
- DexMethod actualTarget = graphLense.lookupMethod(invokedMethod, method);
- Invoke.Type invokeType = getInvokeType(invoke, actualTarget, invokedMethod);
+ DexMethod actualTarget = graphLense.lookupMethod(invokedMethod, method, invoke.getType());
+ Invoke.Type invokeType = getInvokeType(invoke, actualTarget, invokedMethod, method);
if (actualTarget != invokedMethod || invoke.getType() != invokeType) {
Invoke newInvoke = Invoke.create(invokeType, actualTarget, null,
invoke.outValue(), invoke.inValues());
@@ -107,10 +111,11 @@
&& newInvoke.outValue() != null) {
Value newValue = code.createValue(newInvoke.outType(), invoke.getLocalInfo());
newInvoke.outValue().replaceUsers(newValue);
- CheckCast cast = new CheckCast(
- newValue,
- newInvoke.outValue(),
- graphLense.lookupType(invokedMethod.proto.returnType, method));
+ CheckCast cast =
+ new CheckCast(
+ newValue,
+ newInvoke.outValue(),
+ graphLense.lookupType(invokedMethod.proto.returnType));
cast.setPosition(current.getPosition());
iterator.add(cast);
// If the current block has catch handlers split the check cast into its own block.
@@ -123,7 +128,7 @@
} else if (current.isInstanceGet()) {
InstanceGet instanceGet = current.asInstanceGet();
DexField field = instanceGet.getField();
- DexField actualField = graphLense.lookupField(field, method);
+ DexField actualField = graphLense.lookupField(field);
if (actualField != field) {
InstanceGet newInstanceGet =
new InstanceGet(
@@ -133,7 +138,7 @@
} else if (current.isInstancePut()) {
InstancePut instancePut = current.asInstancePut();
DexField field = instancePut.getField();
- DexField actualField = graphLense.lookupField(field, method);
+ DexField actualField = graphLense.lookupField(field);
if (actualField != field) {
InstancePut newInstancePut =
new InstancePut(
@@ -143,7 +148,7 @@
} else if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
DexField field = staticGet.getField();
- DexField actualField = graphLense.lookupField(field, method);
+ DexField actualField = graphLense.lookupField(field);
if (actualField != field) {
StaticGet newStaticGet =
new StaticGet(staticGet.getType(), staticGet.dest(), actualField);
@@ -152,7 +157,7 @@
} else if (current.isStaticPut()) {
StaticPut staticPut = current.asStaticPut();
DexField field = staticPut.getField();
- DexField actualField = graphLense.lookupField(field, method);
+ DexField actualField = graphLense.lookupField(field);
if (actualField != field) {
StaticPut newStaticPut =
new StaticPut(staticPut.getType(), staticPut.inValue(), actualField);
@@ -160,7 +165,7 @@
}
} else if (current.isCheckCast()) {
CheckCast checkCast = current.asCheckCast();
- DexType newType = graphLense.lookupType(checkCast.getType(), method);
+ DexType newType = graphLense.lookupType(checkCast.getType());
if (newType != checkCast.getType()) {
CheckCast newCheckCast =
new CheckCast(makeOutValue(checkCast, code), checkCast.object(), newType);
@@ -168,14 +173,14 @@
}
} else if (current.isConstClass()) {
ConstClass constClass = current.asConstClass();
- DexType newType = graphLense.lookupType(constClass.getValue(), method);
+ DexType newType = graphLense.lookupType(constClass.getValue());
if (newType != constClass.getValue()) {
ConstClass newConstClass = new ConstClass(makeOutValue(constClass, code), newType);
iterator.replaceCurrentInstruction(newConstClass);
}
} else if (current.isInstanceOf()) {
InstanceOf instanceOf = current.asInstanceOf();
- DexType newType = graphLense.lookupType(instanceOf.type(), method);
+ DexType newType = graphLense.lookupType(instanceOf.type());
if (newType != instanceOf.type()) {
InstanceOf newInstanceOf = new InstanceOf(makeOutValue(instanceOf, code),
instanceOf.value(), newType);
@@ -183,7 +188,7 @@
}
} else if (current.isInvokeNewArray()) {
InvokeNewArray newArray = current.asInvokeNewArray();
- DexType newType = graphLense.lookupType(newArray.getArrayType(), method);
+ DexType newType = graphLense.lookupType(newArray.getArrayType());
if (newType != newArray.getArrayType()) {
InvokeNewArray newNewArray = new InvokeNewArray(newType, makeOutValue(newArray, code),
newArray.inValues());
@@ -191,7 +196,7 @@
}
} else if (current.isNewArrayEmpty()) {
NewArrayEmpty newArrayEmpty = current.asNewArrayEmpty();
- DexType newType = graphLense.lookupType(newArrayEmpty.type, method);
+ DexType newType = graphLense.lookupType(newArrayEmpty.type);
if (newType != newArrayEmpty.type) {
NewArrayEmpty newNewArray = new NewArrayEmpty(makeOutValue(newArrayEmpty, code),
newArrayEmpty.size(), newType);
@@ -199,7 +204,7 @@
}
} else if (current.isNewInstance()) {
NewInstance newInstance= current.asNewInstance();
- DexType newClazz = graphLense.lookupType(newInstance.clazz, method);
+ DexType newClazz = graphLense.lookupType(newInstance.clazz);
if (newClazz != newInstance.clazz) {
NewInstance newNewInstance =
new NewInstance(newClazz, makeOutValue(newInstance, code));
@@ -215,7 +220,8 @@
DexEncodedMethod method, DexMethodHandle methodHandle) {
if (methodHandle.isMethodHandle()) {
DexMethod invokedMethod = methodHandle.asMethod();
- DexMethod actualTarget = graphLense.lookupMethod(invokedMethod, method);
+ DexMethod actualTarget =
+ graphLense.lookupMethod(invokedMethod, method, methodHandle.type.toInvokeType());
if (actualTarget != invokedMethod) {
DexClass clazz = appInfo.definitionFor(actualTarget.holder);
MethodHandleType newType = methodHandle.type;
@@ -229,7 +235,7 @@
}
} else {
DexField field = methodHandle.asField();
- DexField actualField = graphLense.lookupField(field, method);
+ DexField actualField = graphLense.lookupField(field);
if (actualField != field) {
return new DexMethodHandle(methodHandle.type, actualField);
}
@@ -240,7 +246,8 @@
private Type getInvokeType(
InvokeMethod invoke,
DexMethod actualTarget,
- DexMethod originalTarget) {
+ DexMethod originalTarget,
+ DexEncodedMethod invocationContext) {
// We might move methods from interfaces to classes and vice versa. So we have to support
// fixing the invoke kind, yet only if it was correct to start with.
if (invoke.isInvokeVirtual() || invoke.isInvokeInterface()) {
@@ -248,23 +255,42 @@
DexClass newTargetClass = appInfo.definitionFor(actualTarget.holder);
if (newTargetClass == null) {
return invoke.getType();
- } else {
- DexClass originalTargetClass = appInfo.definitionFor(originalTarget.holder);
- if (originalTargetClass != null
- && (originalTargetClass.isInterface() ^ (invoke.getType() == Type.INTERFACE))) {
- // The invoke was wrong to start with, so we keep it wrong. This is to ensure we get
- // the IncompatibleClassChangeError the original invoke would have triggered.
- return newTargetClass.accessFlags.isInterface()
- ? Type.VIRTUAL
- : Type.INTERFACE;
- } else {
- return newTargetClass.accessFlags.isInterface()
- ? Type.INTERFACE
- : Type.VIRTUAL;
+ }
+ DexClass originalTargetClass = appInfo.definitionFor(originalTarget.holder);
+ if (originalTargetClass != null
+ && (originalTargetClass.isInterface() ^ (invoke.getType() == Type.INTERFACE))) {
+ // The invoke was wrong to start with, so we keep it wrong. This is to ensure we get
+ // the IncompatibleClassChangeError the original invoke would have triggered.
+ return newTargetClass.accessFlags.isInterface() ? Type.VIRTUAL : Type.INTERFACE;
+ }
+ return newTargetClass.accessFlags.isInterface() ? Type.INTERFACE : Type.VIRTUAL;
+ }
+ if (options.enableClassMerging && invoke.isInvokeSuper()) {
+ if (actualTarget.getHolder() == invocationContext.method.getHolder()) {
+ DexClass targetClass = appInfo.definitionFor(actualTarget.holder);
+ if (targetClass == null) {
+ return invoke.getType();
+ }
+
+ // If the super class A of the enclosing class B (i.e., invocationContext.method.holder)
+ // has been merged into B during vertical class merging, and this invoke-super instruction
+ // was resolving to a method in A, then the target method has been changed to a direct
+ // method and moved into B, so that we need to use an invoke-direct instruction instead of
+ // invoke-super.
+ //
+ // At this point, we have an invoke-super instruction where the static target is the
+ // enclosing class. However, such an instruction could occur even if a subclass has never
+ // been merged into the enclosing class. Therefore, to determine if vertical class merging
+ // has been applied, we look if there is a direct method with the right signature, and only
+ // return Type.DIRECT in that case.
+ DexEncodedMethod method = targetClass.lookupDirectMethod(actualTarget);
+ if (method != null) {
+ // The target method has been moved from the super class into the sub class during class
+ // merging such that we now need to use an invoke-direct instruction.
+ return Type.DIRECT;
}
}
- } else {
- return invoke.getType();
}
+ return invoke.getType();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
index 531989e..8e587b9 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
import com.android.tools.r8.ir.code.IRCode;
@@ -11,15 +13,19 @@
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.utils.InternalOptions;
+import java.util.ArrayList;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.List;
import java.util.Queue;
+import java.util.Set;
public class DeadCodeRemover {
public static void removeDeadCode(
- IRCode code, CodeRewriter codeRewriter, InternalOptions options) {
- removeUnneededCatchHandlers(code);
+ IRCode code, CodeRewriter codeRewriter, GraphLense graphLense, InternalOptions options) {
+ removeUnneededCatchHandlers(code, graphLense, options);
Queue<BasicBlock> worklist = new LinkedList<>();
worklist.addAll(code.blocks);
for (BasicBlock block = worklist.poll(); block != null; block = worklist.poll()) {
@@ -98,15 +104,49 @@
}
}
- private static void removeUnneededCatchHandlers(IRCode code) {
+ private static void removeUnneededCatchHandlers(
+ IRCode code, GraphLense graphLense, InternalOptions options) {
for (BasicBlock block : code.blocks) {
- if (block.hasCatchHandlers() && !block.canThrow()) {
- CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
- for (BasicBlock target : handlers.getUniqueTargets()) {
- target.unlinkCatchHandler();
+ if (block.hasCatchHandlers()) {
+ if (block.canThrow()) {
+ if (options.enableClassMerging) {
+ // Handle the case where an exception class has been merged into its sub class.
+ block.renameGuardsInCatchHandlers(graphLense);
+ unlinkDeadCatchHandlers(block, graphLense);
+ }
+ } else {
+ CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
+ for (BasicBlock target : handlers.getUniqueTargets()) {
+ target.unlinkCatchHandler();
+ }
}
}
}
code.removeUnreachableBlocks();
}
+
+ // Due to class merging, it is possible that two exception classes have been merged into one. This
+ // function removes catch handlers where the guards ended up being the same as a previous one.
+ private static void unlinkDeadCatchHandlers(BasicBlock block, GraphLense graphLense) {
+ assert block.hasCatchHandlers();
+ CatchHandlers<BasicBlock> catchHandlers = block.getCatchHandlers();
+ List<DexType> guards = catchHandlers.getGuards();
+ List<BasicBlock> targets = catchHandlers.getAllTargets();
+
+ Set<DexType> previouslySeenGuards = new HashSet<>();
+ List<BasicBlock> deadCatchHandlers = new ArrayList<>();
+ for (int i = 0; i < guards.size(); i++) {
+ // The type may have changed due to class merging.
+ DexType guard = graphLense.lookupType(guards.get(i));
+ boolean guardSeenBefore = !previouslySeenGuards.add(guard);
+ if (guardSeenBefore) {
+ deadCatchHandlers.add(targets.get(i));
+ }
+ }
+ // Remove the guards that are guaranteed to be dead.
+ for (BasicBlock deadCatchHandler : deadCatchHandlers) {
+ deadCatchHandler.unlinkCatchHandler();
+ }
+ assert block.consistentCatchHandlers();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 25cd969..029b169 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -271,7 +271,7 @@
Origin origin = appInfo.originFor(target.method.holder);
IRCode code = target.buildInliningIR(appInfo, options, generator, callerPosition, origin);
if (!target.isProcessed()) {
- new LensCodeRewriter(graphLense, appInfo).rewrite(code, target);
+ new LensCodeRewriter(graphLense, appInfo, options).rewrite(code, target);
}
return code;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
new file mode 100644
index 0000000..24012b1
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
@@ -0,0 +1,193 @@
+// Copyright (c) 2018, 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.ir.optimize;
+
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.DominatorTree;
+import com.android.tools.r8.ir.code.FieldInstruction;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.Phi;
+import com.android.tools.r8.ir.code.Value;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+/**
+ * Eliminate redundant field loads.
+ *
+ * <p>Simple algorithm that goes through all blocks in one pass in dominator order and propagates
+ * active field sets across control-flow edges where the target has only one predecessor.
+ */
+// TODO(ager): Evaluate speed/size for computing active field sets in a fixed-point computation.
+public class RedundantFieldLoadElimination {
+
+ private final IRCode code;
+ private final DominatorTree dominatorTree;
+
+ // Maps keeping track of fields that have an already loaded value at basic block entry.
+ private final HashMap<BasicBlock, HashMap<FieldAndObject, Instruction>>
+ activeInstanceFieldsAtEntry = new HashMap<>();
+ private final HashMap<BasicBlock, HashMap<DexField, Instruction>> activeStaticFieldsAtEntry =
+ new HashMap<>();
+
+ // Maps keeping track of fields with already loaded values for the current block during
+ // elimination.
+ private HashMap<FieldAndObject, Instruction> activeInstanceFields;
+ private HashMap<DexField, Instruction> activeStaticFields;
+
+ public RedundantFieldLoadElimination(IRCode code) {
+ this.code = code;
+ dominatorTree = new DominatorTree(code);
+ }
+
+ private static class FieldAndObject {
+ private final DexField field;
+ private final Value object;
+
+ private FieldAndObject(DexField field, Value receiver) {
+ this.field = field;
+ this.object = receiver;
+ }
+
+ @Override
+ public int hashCode() {
+ return field.hashCode() * 7 + object.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (!(other instanceof FieldAndObject)) {
+ return false;
+ }
+ FieldAndObject o = (FieldAndObject) other;
+ return o.object == object && o.field == field;
+ }
+ }
+
+ public void run() {
+ for (BasicBlock block : dominatorTree.getSortedBlocks()) {
+ activeInstanceFields =
+ activeInstanceFieldsAtEntry.containsKey(block)
+ ? activeInstanceFieldsAtEntry.get(block)
+ : new HashMap<>();
+ activeStaticFields =
+ activeStaticFieldsAtEntry.containsKey(block)
+ ? activeStaticFieldsAtEntry.get(block)
+ : new HashMap<>();
+ InstructionListIterator it = block.listIterator();
+ while (it.hasNext()) {
+ Instruction instruction = it.next();
+ if (instruction.isFieldInstruction()) {
+ DexField field = instruction.asFieldInstruction().getField();
+ if (instruction.isInstancePut() || instruction.isStaticPut()) {
+ killActiveFields(instruction.asFieldInstruction());
+ } else if (instruction.isInstanceGet() && !instruction.outValue().hasLocalInfo()) {
+ Value object = instruction.asInstanceGet().object();
+ FieldAndObject fieldAndObject = new FieldAndObject(field, object);
+ if (activeInstanceFields.containsKey(fieldAndObject)) {
+ Instruction active = activeInstanceFields.get(fieldAndObject);
+ eliminateRedundantRead(it, instruction, active);
+ } else {
+ activeInstanceFields.put(fieldAndObject, instruction);
+ }
+ } else if (instruction.isStaticGet() && !instruction.outValue().hasLocalInfo()) {
+ if (activeStaticFields.containsKey(field)) {
+ Instruction active = activeStaticFields.get(field);
+ eliminateRedundantRead(it, instruction, active);
+ } else {
+ // A field get on a different class can cause <clinit> to run and change static
+ // field values.
+ killActiveFields(instruction.asFieldInstruction());
+ activeStaticFields.put(field, instruction);
+ }
+ }
+ }
+ if (instruction.isMonitor() || instruction.isInvokeMethod()) {
+ activeInstanceFields.clear();
+ activeStaticFields.clear();
+ }
+ }
+ propagateActiveFieldsFrom(block);
+ }
+ assert code.isConsistentSSA();
+ }
+
+ private void propagateActiveFieldsFrom(BasicBlock block) {
+ for (BasicBlock successor : block.getSuccessors()) {
+ // Allow propagation across exceptional edges, just be careful not to propagate if the
+ // throwing instruction is a field instruction.
+ if (successor.getPredecessors().size() == 1) {
+ if (block.hasCatchSuccessor(successor)) {
+ Instruction exceptionalExit = block.exceptionalExit();
+ if (exceptionalExit != null && exceptionalExit.isFieldInstruction()) {
+ killActiveFieldsForExceptionalExit(exceptionalExit.asFieldInstruction());
+ }
+ }
+ assert !activeInstanceFieldsAtEntry.containsKey(successor);
+ activeInstanceFieldsAtEntry.put(successor, new HashMap<>(activeInstanceFields));
+ assert !activeStaticFieldsAtEntry.containsKey(successor);
+ activeStaticFieldsAtEntry.put(successor, new HashMap<>(activeStaticFields));
+ }
+ }
+ }
+
+ private void killActiveFields(FieldInstruction instruction) {
+ DexField field = instruction.getField();
+ if (instruction.isInstancePut()) {
+ // Remove all the field/object pairs that refer to this field to make sure
+ // that we are conservative.
+ List<FieldAndObject> keysToRemove = new ArrayList<>();
+ for (FieldAndObject key : activeInstanceFields.keySet()) {
+ if (key.field == field) {
+ keysToRemove.add(key);
+ }
+ }
+ keysToRemove.forEach((k) -> activeInstanceFields.remove(k));
+ } else if (instruction.isInstanceGet()) {
+ Value object = instruction.asInstanceGet().object();
+ FieldAndObject fieldAndObject = new FieldAndObject(field, object);
+ activeInstanceFields.remove(fieldAndObject);
+ } else if (instruction.isStaticPut()) {
+ if (field.clazz != code.method.method.holder) {
+ // Accessing a static field on a different object could cause <clinit> to run which
+ // could modify any static field on any other object.
+ activeStaticFields.clear();
+ } else {
+ activeStaticFields.remove(field);
+ }
+ } else if (instruction.isStaticGet()) {
+ if (field.clazz != code.method.method.holder) {
+ // Accessing a static field on a different object could cause <clinit> to run which
+ // could modify any static field on any other object.
+ activeStaticFields.clear();
+ }
+ }
+ }
+
+ // If a field get instruction throws an exception it did not have an effect on the
+ // value of the field. Therefore, when propagating across exceptional edges for a
+ // field get instruction we have to exclude that field from the set of known
+ // field values.
+ private void killActiveFieldsForExceptionalExit(FieldInstruction instruction) {
+ DexField field = instruction.getField();
+ if (instruction.isInstanceGet()) {
+ Value object = instruction.asInstanceGet().object();
+ FieldAndObject fieldAndObject = new FieldAndObject(field, object);
+ activeInstanceFields.remove(fieldAndObject);
+ } else if (instruction.isStaticGet()) {
+ activeStaticFields.remove(field);
+ }
+ }
+
+ private void eliminateRedundantRead(
+ InstructionListIterator it, Instruction redundant, Instruction active) {
+ redundant.outValue().replaceUsers(active.outValue());
+ it.removeOrReplaceByDebugLocalRead();
+ active.outValue().uniquePhiUsers().forEach(Phi::removeTrivialPhi);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
index 0ba49d3..6ea7d60 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
@@ -45,6 +45,7 @@
assert checkSignatures();
switch (invokeType) {
+ case DIRECT:
case STATIC:
case SUPER:
case INTERFACE:
diff --git a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
index 0af41b1..cf2b2bc 100644
--- a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
+++ b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
@@ -5,11 +5,19 @@
package com.android.tools.r8.kotlin;
import com.android.tools.r8.DiagnosticsHandler;
+import com.android.tools.r8.graph.DexAnnotation;
+import com.android.tools.r8.graph.DexAnnotationElement;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueArray;
+import com.android.tools.r8.graph.DexValue.DexValueInt;
+import com.android.tools.r8.graph.DexValue.DexValueString;
+import com.android.tools.r8.kotlin.KotlinSyntheticClass.Flavour;
+import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.Sets;
import java.util.Set;
@@ -50,13 +58,10 @@
public final DexString kotlinStyleLambdaInstanceName = factory.createString("INSTANCE");
- public final DexType functionBase = factory.createType("Lkotlin/jvm/internal/FunctionBase;");
public final DexType lambdaType = factory.createType("Lkotlin/jvm/internal/Lambda;");
- public final DexMethod lambdaInitializerMethod = factory.createMethod(
- lambdaType,
- factory.createProto(factory.voidType, factory.intType),
- factory.constructorMethodName);
+ public final DexMethod lambdaInitializerMethod = factory.createMethod(lambdaType,
+ factory.createProto(factory.voidType, factory.intType), factory.constructorMethodName);
public boolean isFunctionInterface(DexType type) {
return functions.contains(type);
@@ -65,14 +70,9 @@
public final class Metadata {
public final DexType kotlinMetadataType = factory.createType("Lkotlin/Metadata;");
- public final DexString kind = factory.createString("k");
- public final DexString metadataVersion = factory.createString("mv");
- public final DexString bytecodeVersion = factory.createString("bv");
- public final DexString data1 = factory.createString("d1");
- public final DexString data2 = factory.createString("d2");
- public final DexString extraString = factory.createString("xs");
- public final DexString packageName = factory.createString("pn");
- public final DexString extraInt = factory.createString("xi");
+ public final DexString elementNameK = factory.createString("k");
+ public final DexString elementNameD1 = factory.createString("d1");
+ public final DexString elementNameD2 = factory.createString("d2");
}
// kotlin.jvm.internal.Intrinsics class
@@ -86,6 +86,98 @@
// Calculates kotlin info for a class.
public KotlinInfo getKotlinInfo(DexClass clazz, DiagnosticsHandler reporter) {
- return KotlinClassMetadataReader.getKotlinInfo(this, clazz, reporter);
+ if (clazz.annotations.isEmpty()) {
+ return null;
+ }
+ DexAnnotation meta = clazz.annotations.getFirstMatching(metadata.kotlinMetadataType);
+ if (meta != null) {
+ try {
+ return createKotlinInfo(clazz, meta);
+ } catch (MetadataError e) {
+ reporter.warning(
+ new StringDiagnostic("Class " + clazz.type.toSourceString() +
+ " has malformed kotlin.Metadata: " + e.getMessage()));
+ }
+ }
+ return null;
+ }
+
+ private KotlinInfo createKotlinInfo(DexClass clazz, DexAnnotation meta) {
+ DexAnnotationElement kindElement = getAnnotationElement(meta, metadata.elementNameK);
+ if (kindElement == null) {
+ throw new MetadataError("element 'k' is missing");
+ }
+
+ DexValue value = kindElement.value;
+ if (!(value instanceof DexValueInt)) {
+ throw new MetadataError("invalid 'k' value: " + value.toSourceString());
+ }
+
+ DexValueInt intValue = (DexValueInt) value;
+ switch (intValue.value) {
+ case 1:
+ return new KotlinClass();
+ case 2:
+ return new KotlinFile();
+ case 3:
+ return createSyntheticClass(clazz, meta);
+ case 4:
+ return new KotlinClassFacade();
+ case 5:
+ return new KotlinClassPart();
+ default:
+ throw new MetadataError("unsupported 'k' value: " + value.toSourceString());
+ }
+ }
+
+ private KotlinSyntheticClass createSyntheticClass(DexClass clazz, DexAnnotation meta) {
+ if (isKotlinStyleLambda(clazz)) {
+ return new KotlinSyntheticClass(Flavour.KotlinStyleLambda);
+ }
+ if (isJavaStyleLambda(clazz, meta)) {
+ return new KotlinSyntheticClass(Flavour.JavaStyleLambda);
+ }
+ return new KotlinSyntheticClass(Flavour.Unclassified);
+ }
+
+ private boolean isKotlinStyleLambda(DexClass clazz) {
+ // TODO: replace with direct hints from kotlin metadata when available.
+ return clazz.superType == this.functional.lambdaType;
+ }
+
+ private boolean isJavaStyleLambda(DexClass clazz, DexAnnotation meta) {
+ assert !isKotlinStyleLambda(clazz);
+ return clazz.superType == this.factory.objectType &&
+ clazz.interfaces.size() == 1 &&
+ isAnnotationElementNotEmpty(meta, metadata.elementNameD1);
+ }
+
+ private DexAnnotationElement getAnnotationElement(DexAnnotation annotation, DexString name) {
+ for (DexAnnotationElement element : annotation.annotation.elements) {
+ if (element.name == name) {
+ return element;
+ }
+ }
+ return null;
+ }
+
+ private boolean isAnnotationElementNotEmpty(DexAnnotation annotation, DexString name) {
+ for (DexAnnotationElement element : annotation.annotation.elements) {
+ if (element.name == name && element.value instanceof DexValueArray) {
+ DexValue[] values = ((DexValueArray) element.value).getValues();
+ if (values.length == 1 && values[0] instanceof DexValueString) {
+ return true;
+ }
+ // Must be broken metadata.
+ assert false;
+ }
+ }
+ return false;
+ }
+
+ private static class MetadataError extends RuntimeException {
+ MetadataError(String cause) {
+ super(cause);
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
index 74184a5..fc81994 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
@@ -4,31 +4,7 @@
package com.android.tools.r8.kotlin;
-import kotlinx.metadata.KmClassVisitor;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-public class KotlinClass extends KotlinInfo<KotlinClassMetadata.Class> {
-
- static KotlinClass fromKotlinClassMetadata(KotlinClassMetadata kotlinClassMetadata) {
- assert kotlinClassMetadata instanceof KotlinClassMetadata.Class;
- KotlinClassMetadata.Class kClass = (KotlinClassMetadata.Class) kotlinClassMetadata;
- return new KotlinClass(kClass);
- }
-
- private KotlinClass(KotlinClassMetadata.Class metadata) {
- super(metadata);
- }
-
- @Override
- void validateMetadata(KotlinClassMetadata.Class metadata) {
- ClassMetadataVisitor visitor = new ClassMetadataVisitor();
- // To avoid lazy parsing/verifying metadata.
- metadata.accept(visitor);
- }
-
- private static class ClassMetadataVisitor extends KmClassVisitor {
- }
-
+public class KotlinClass extends KotlinInfo {
@Override
public Kind getKind() {
return Kind.Class;
@@ -44,4 +20,6 @@
return this;
}
+ KotlinClass() {
+ }
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
index 62db3d1..e829a27 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
@@ -4,26 +4,7 @@
package com.android.tools.r8.kotlin;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-public final class KotlinClassFacade extends KotlinInfo<KotlinClassMetadata.MultiFileClassFacade> {
-
- static KotlinClassFacade fromKotlinClassMetadata(KotlinClassMetadata kotlinClassMetadata) {
- assert kotlinClassMetadata instanceof KotlinClassMetadata.MultiFileClassFacade;
- KotlinClassMetadata.MultiFileClassFacade multiFileClassFacade =
- (KotlinClassMetadata.MultiFileClassFacade) kotlinClassMetadata;
- return new KotlinClassFacade(multiFileClassFacade);
- }
-
- private KotlinClassFacade(KotlinClassMetadata.MultiFileClassFacade metadata) {
- super(metadata);
- }
-
- @Override
- void validateMetadata(KotlinClassMetadata.MultiFileClassFacade metadata) {
- // No worries about lazy parsing/verifying, since no API to explore metadata details.
- }
-
+public final class KotlinClassFacade extends KotlinInfo {
@Override
public Kind getKind() {
return Kind.Facade;
@@ -39,4 +20,7 @@
return this;
}
+ KotlinClassFacade() {
+ super();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
deleted file mode 100644
index 32aebff..0000000
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
+++ /dev/null
@@ -1,133 +0,0 @@
-// Copyright (c) 2018, 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.kotlin;
-
-import com.android.tools.r8.DiagnosticsHandler;
-import com.android.tools.r8.graph.DexAnnotation;
-import com.android.tools.r8.graph.DexAnnotationElement;
-import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexString;
-import com.android.tools.r8.graph.DexValue;
-import com.android.tools.r8.graph.DexValue.DexValueArray;
-import com.android.tools.r8.graph.DexValue.DexValueString;
-import com.android.tools.r8.utils.StringDiagnostic;
-import java.util.IdentityHashMap;
-import java.util.Map;
-import kotlinx.metadata.InconsistentKotlinMetadataException;
-import kotlinx.metadata.jvm.KotlinClassHeader;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-final class KotlinClassMetadataReader {
-
- static KotlinInfo getKotlinInfo(
- Kotlin kotlin,
- DexClass clazz,
- DiagnosticsHandler reporter) {
- if (clazz.annotations.isEmpty()) {
- return null;
- }
- DexAnnotation meta = clazz.annotations.getFirstMatching(kotlin.metadata.kotlinMetadataType);
- if (meta != null) {
- try {
- return createKotlinInfo(kotlin, clazz, meta);
- } catch (ClassCastException | InconsistentKotlinMetadataException | MetadataError e) {
- reporter.warning(
- new StringDiagnostic("Class " + clazz.type.toSourceString()
- + " has malformed kotlin.Metadata: " + e.getMessage()));
- } catch (Throwable e) {
- reporter.warning(
- new StringDiagnostic("Unexpected error while reading " + clazz.type.toSourceString()
- + "'s kotlin.Metadata: " + e.getMessage()));
- }
- }
- return null;
- }
-
- private static KotlinInfo createKotlinInfo(
- Kotlin kotlin,
- DexClass clazz,
- DexAnnotation meta) {
- Map<DexString, DexAnnotationElement> elementMap = new IdentityHashMap<>();
- for (DexAnnotationElement element : meta.annotation.elements) {
- elementMap.put(element.name, element);
- }
-
- DexAnnotationElement kind = elementMap.get(kotlin.metadata.kind);
- if (kind == null) {
- throw new MetadataError("element 'k' is missing.");
- }
- Integer k = (Integer) kind.value.getBoxedValue();
- DexAnnotationElement metadataVersion = elementMap.get(kotlin.metadata.metadataVersion);
- int[] mv = metadataVersion == null ? null : getUnboxedIntArray(metadataVersion.value, "mv");
- DexAnnotationElement bytecodeVersion = elementMap.get(kotlin.metadata.bytecodeVersion);
- int[] bv = bytecodeVersion == null ? null : getUnboxedIntArray(bytecodeVersion.value, "bv");
- DexAnnotationElement data1 = elementMap.get(kotlin.metadata.data1);
- String[] d1 = data1 == null ? null : getUnboxedStringArray(data1.value, "d1");
- DexAnnotationElement data2 = elementMap.get(kotlin.metadata.data2);
- String[] d2 = data2 == null ? null : getUnboxedStringArray(data2.value, "d2");
- DexAnnotationElement extraString = elementMap.get(kotlin.metadata.extraString);
- String xs = extraString == null ? null : getUnboxedString(extraString.value, "xs");
- DexAnnotationElement packageName = elementMap.get(kotlin.metadata.packageName);
- String pn = packageName == null ? null : getUnboxedString(packageName.value, "pn");
- DexAnnotationElement extraInt = elementMap.get(kotlin.metadata.extraInt);
- Integer xi = extraInt == null ? null : (Integer) extraInt.value.getBoxedValue();
-
- KotlinClassHeader header = new KotlinClassHeader(k, mv, bv, d1, d2, xs, pn, xi);
- KotlinClassMetadata kMetadata = KotlinClassMetadata.read(header);
-
- if (kMetadata instanceof KotlinClassMetadata.Class) {
- return KotlinClass.fromKotlinClassMetadata(kMetadata);
- } else if (kMetadata instanceof KotlinClassMetadata.FileFacade) {
- return KotlinFile.fromKotlinClassMetadata(kMetadata);
- } else if (kMetadata instanceof KotlinClassMetadata.MultiFileClassFacade) {
- return KotlinClassFacade.fromKotlinClassMetadata(kMetadata);
- } else if (kMetadata instanceof KotlinClassMetadata.MultiFileClassPart) {
- return KotlinClassPart.fromKotlinClassMetdata(kMetadata);
- } else if (kMetadata instanceof KotlinClassMetadata.SyntheticClass) {
- return KotlinSyntheticClass.fromKotlinClassMetadata(kMetadata, kotlin, clazz);
- } else {
- throw new MetadataError("unsupported 'k' value: " + k);
- }
- }
-
- private static int[] getUnboxedIntArray(DexValue v, String elementName) {
- if (!(v instanceof DexValueArray)) {
- throw new MetadataError("invalid '" + elementName + "' value: " + v.toSourceString());
- }
- DexValueArray intArrayValue = (DexValueArray) v;
- DexValue[] values = intArrayValue.getValues();
- int[] result = new int [values.length];
- for (int i = 0; i < values.length; i++) {
- result[i] = (Integer) values[i].getBoxedValue();
- }
- return result;
- }
-
- private static String[] getUnboxedStringArray(DexValue v, String elementName) {
- if (!(v instanceof DexValueArray)) {
- throw new MetadataError("invalid '" + elementName + "' value: " + v.toSourceString());
- }
- DexValueArray stringArrayValue = (DexValueArray) v;
- DexValue[] values = stringArrayValue.getValues();
- String[] result = new String [values.length];
- for (int i = 0; i < values.length; i++) {
- result[i] = getUnboxedString(values[i], elementName + "[" + i + "]");
- }
- return result;
- }
-
- private static String getUnboxedString(DexValue v, String elementName) {
- if (!(v instanceof DexValueString)) {
- throw new MetadataError("invalid '" + elementName + "' value: " + v.toSourceString());
- }
- return ((DexValueString) v).getValue().toString();
- }
-
- private static class MetadataError extends RuntimeException {
- MetadataError(String cause) {
- super(cause);
- }
- }
-
-}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
index da66e6c..d6da817 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
@@ -4,31 +4,7 @@
package com.android.tools.r8.kotlin;
-import kotlinx.metadata.KmPackageVisitor;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-public final class KotlinClassPart extends KotlinInfo<KotlinClassMetadata.MultiFileClassPart> {
-
- static KotlinClassPart fromKotlinClassMetdata(KotlinClassMetadata kotlinClassMetadata) {
- assert kotlinClassMetadata instanceof KotlinClassMetadata.MultiFileClassPart;
- KotlinClassMetadata.MultiFileClassPart multiFileClassPart =
- (KotlinClassMetadata.MultiFileClassPart) kotlinClassMetadata;
- return new KotlinClassPart(multiFileClassPart);
- }
-
- private KotlinClassPart(KotlinClassMetadata.MultiFileClassPart metadata) {
- super(metadata);
- }
-
- @Override
- void validateMetadata(KotlinClassMetadata.MultiFileClassPart metadata) {
- // To avoid lazy parsing/verifying metadata.
- metadata.accept(new MultiFileClassPartMetadataVisitor());
- }
-
- private static class MultiFileClassPartMetadataVisitor extends KmPackageVisitor {
- }
-
+public final class KotlinClassPart extends KotlinInfo {
@Override
public Kind getKind() {
return Kind.Part;
@@ -44,4 +20,6 @@
return this;
}
+ KotlinClassPart() {
+ }
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java b/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
index bcb70ed..38a77ad 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
@@ -4,31 +4,7 @@
package com.android.tools.r8.kotlin;
-import kotlinx.metadata.KmPackageVisitor;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-public final class KotlinFile extends KotlinInfo<KotlinClassMetadata.FileFacade> {
-
- static KotlinFile fromKotlinClassMetadata(KotlinClassMetadata kotlinClassMetadata) {
- assert kotlinClassMetadata instanceof KotlinClassMetadata.FileFacade;
- KotlinClassMetadata.FileFacade fileFacade =
- (KotlinClassMetadata.FileFacade) kotlinClassMetadata;
- return new KotlinFile(fileFacade);
- }
-
- private KotlinFile(KotlinClassMetadata.FileFacade metadata) {
- super(metadata);
- }
-
- @Override
- void validateMetadata(KotlinClassMetadata.FileFacade metadata) {
- // To avoid lazy parsing/verifying metadata.
- metadata.accept(new FileFacadeMetadataVisitor());
- }
-
- private static class FileFacadeMetadataVisitor extends KmPackageVisitor {
- }
-
+public final class KotlinFile extends KotlinInfo {
@Override
public Kind getKind() {
return Kind.File;
@@ -44,4 +20,6 @@
return this;
}
+ KotlinFile() {
+ }
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
index 702e0eb..4043bc6 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
@@ -4,23 +4,11 @@
package com.android.tools.r8.kotlin;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
// Provides access to kotlin information.
-public abstract class KotlinInfo<MetadataKind extends KotlinClassMetadata> {
- MetadataKind metadata;
-
+public abstract class KotlinInfo {
KotlinInfo() {
}
- KotlinInfo(MetadataKind metadata) {
- validateMetadata(metadata);
- this.metadata = metadata;
- }
-
- // Subtypes will define how to validate the given metadata.
- abstract void validateMetadata(MetadataKind metadata);
-
public enum Kind {
Class, File, Synthetic, Part, Facade
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
index 4660800..68721bb 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
@@ -4,11 +4,7 @@
package com.android.tools.r8.kotlin;
-import com.android.tools.r8.graph.DexClass;
-import kotlinx.metadata.KmLambdaVisitor;
-import kotlinx.metadata.jvm.KotlinClassMetadata;
-
-public final class KotlinSyntheticClass extends KotlinInfo<KotlinClassMetadata.SyntheticClass> {
+public final class KotlinSyntheticClass extends KotlinInfo {
public enum Flavour {
KotlinStyleLambda,
JavaStyleLambda,
@@ -17,40 +13,12 @@
private final Flavour flavour;
- static KotlinSyntheticClass fromKotlinClassMetadata(
- KotlinClassMetadata kotlinClassMetadata, Kotlin kotlin, DexClass clazz) {
- assert kotlinClassMetadata instanceof KotlinClassMetadata.SyntheticClass;
- KotlinClassMetadata.SyntheticClass syntheticClass =
- (KotlinClassMetadata.SyntheticClass) kotlinClassMetadata;
- if (isKotlinStyleLambda(syntheticClass, kotlin, clazz)) {
- return new KotlinSyntheticClass(Flavour.KotlinStyleLambda, syntheticClass);
- } else if (isJavaStyleLambda(syntheticClass, kotlin, clazz)) {
- return new KotlinSyntheticClass(Flavour.JavaStyleLambda, syntheticClass);
- } else {
- return new KotlinSyntheticClass(Flavour.Unclassified, syntheticClass);
- }
- }
-
- private KotlinSyntheticClass(Flavour flavour, KotlinClassMetadata.SyntheticClass metadata) {
+ KotlinSyntheticClass(Flavour flavour) {
this.flavour = flavour;
- validateMetadata(metadata);
- this.metadata = metadata;
- }
-
- @Override
- void validateMetadata(KotlinClassMetadata.SyntheticClass metadata) {
- if (metadata.isLambda()) {
- SyntheticClassMetadataVisitor visitor = new SyntheticClassMetadataVisitor();
- // To avoid lazy parsing/verifying metadata.
- metadata.accept(visitor);
- }
- }
-
- private static class SyntheticClassMetadataVisitor extends KmLambdaVisitor {
}
public boolean isLambda() {
- return isKotlinStyleLambda() || isJavaStyleLambda();
+ return flavour == Flavour.KotlinStyleLambda || flavour == Flavour.JavaStyleLambda;
}
public boolean isKotlinStyleLambda() {
@@ -75,31 +43,4 @@
public KotlinSyntheticClass asSyntheticClass() {
return this;
}
-
- /**
- * Returns {@code true} if the given {@link DexClass} is a Kotlin-style lambda:
- * a class that
- * 1) is recognized as lambda in its Kotlin metadata;
- * 2) directly extends kotlin.jvm.internal.Lambda
- */
- private static boolean isKotlinStyleLambda(
- KotlinClassMetadata.SyntheticClass metadata, Kotlin kotlin, DexClass clazz) {
- return metadata.isLambda()
- && clazz.superType == kotlin.functional.lambdaType;
- }
-
- /**
- * Returns {@code true} if the given {@link DexClass} is a Java-style lambda:
- * a class that
- * 1) is recognized as lambda in its Kotlin metadata;
- * 2) doesn't extend any other class;
- * 3) directly implements only one Java SAM.
- */
- private static boolean isJavaStyleLambda(
- KotlinClassMetadata.SyntheticClass metadata, Kotlin kotlin, DexClass clazz) {
- return metadata.isLambda()
- && clazz.superType == kotlin.factory.objectType
- && clazz.interfaces.size() == 1;
- }
-
}
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index e3ec746..01fff01 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -161,7 +161,7 @@
}
private void parseError(DexItem item, Origin origin, GenericSignatureFormatError e) {
- StringBuilder message = new StringBuilder("Invalid class signature for ");
+ StringBuilder message = new StringBuilder("Invalid signature for ");
if (item instanceof DexClass) {
message.append("class ");
message.append(((DexClass) item).getType().toSourceString());
@@ -183,12 +183,14 @@
clazz.annotations = rewriteGenericSignatures(clazz.annotations,
genericSignatureParser::parseClassSignature,
e -> parseError(clazz, clazz.getOrigin(), e));
- clazz.forEachField(field -> rewriteGenericSignatures(
- field.annotations, genericSignatureParser::parseFieldSignature,
- e -> parseError(field, clazz.getOrigin(), e)));
- clazz.forEachMethod(method -> rewriteGenericSignatures(
- method.annotations, genericSignatureParser::parseMethodSignature,
- e -> parseError(method, clazz.getOrigin(), e)));
+ clazz.forEachField(field ->
+ field.annotations = rewriteGenericSignatures(
+ field.annotations, genericSignatureParser::parseFieldSignature,
+ e -> parseError(field, clazz.getOrigin(), e)));
+ clazz.forEachMethod(method ->
+ method.annotations = rewriteGenericSignatures(
+ method.annotations, genericSignatureParser::parseMethodSignature,
+ e -> parseError(method, clazz.getOrigin(), e)));
}
}
@@ -506,11 +508,18 @@
String enclosingRenamedBinaryName =
getClassBinaryNameFromDescriptor(
renaming.getOrDefault(enclosingType, enclosingType.descriptor).toString());
- String renamed =
- getClassBinaryNameFromDescriptor(
- renaming.getOrDefault(type, type.descriptor).toString());
- String outName = renamed.substring(enclosingRenamedBinaryName.length() + 1);
- renamedSignature.append(outName);
+ DexString renamedDescriptor = renaming.get(type);
+ if (renamedDescriptor != null) {
+ // Pick the renamed inner class from the fully renamed binary name.
+ String fullRenamedBinaryName =
+ getClassBinaryNameFromDescriptor(renamedDescriptor.toString());
+ renamedSignature.append(
+ fullRenamedBinaryName.substring(enclosingRenamedBinaryName.length() + 1));
+ } else {
+ // Did not find the class - keep the inner class name as is.
+ // TODO(110085899): Warn about missing classes in signatures?
+ renamedSignature.append(name);
+ }
return type;
}
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
index 521229e..7dfd840 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
@@ -37,6 +37,7 @@
AppInfoWithLiveness appInfo,
GraphLense previousLense,
SeedMapper seedMapper) {
+ assert previousLense.isContextFreeForMethods();
this.appInfo = appInfo;
this.previousLense = previousLense;
this.seedMapper = seedMapper;
@@ -44,7 +45,7 @@
public GraphLense run(Timing timing) {
timing.begin("from-pg-map-to-lense");
- GraphLense lenseFromMap = new MapToLenseConverter().run(previousLense);
+ GraphLense lenseFromMap = new MapToLenseConverter().run();
timing.end();
timing.begin("fix-types-in-programs");
GraphLense typeFixedLense = new TreeFixer(lenseFromMap).run();
@@ -61,7 +62,7 @@
lenseBuilder = new ConflictFreeBuilder();
}
- private GraphLense run(GraphLense previousLense) {
+ private GraphLense run() {
// To handle inherited yet undefined methods in library classes, we are traversing types in
// a subtyping order. That also helps us detect conflicted mappings in a diamond case:
// LibItfA#foo -> a, LibItfB#foo -> b, while PrgA implements LibItfA and LibItfB.
@@ -308,7 +309,7 @@
}
for (int i = 0; i < methods.length; i++) {
DexEncodedMethod encodedMethod = methods[i];
- DexMethod appliedMethod = appliedLense.lookupMethod(encodedMethod.method, encodedMethod);
+ DexMethod appliedMethod = appliedLense.lookupMethod(encodedMethod.method);
DexType newHolderType = substituteType(appliedMethod.holder, encodedMethod);
DexProto newProto = substituteTypesIn(appliedMethod.proto, encodedMethod);
DexMethod newMethod;
@@ -331,7 +332,7 @@
}
for (int i = 0; i < fields.length; i++) {
DexEncodedField encodedField = fields[i];
- DexField appliedField = appliedLense.lookupField(encodedField.field, null);
+ DexField appliedField = appliedLense.lookupField(encodedField.field);
DexType newHolderType = substituteType(appliedField.clazz, null);
DexType newFieldType = substituteType(appliedField.type, null);
DexField newField;
@@ -427,7 +428,7 @@
return type.replaceBaseType(fixed, appInfo.dexItemFactory);
}
}
- return appliedLense.lookupType(type, context);
+ return appliedLense.lookupType(type);
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureParser.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureParser.java
index 5b549a3..c1f6a7d 100644
--- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureParser.java
+++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureParser.java
@@ -84,7 +84,7 @@
throw e;
} catch (Throwable t) {
Error e = new GenericSignatureFormatError(
- "Unknown error parsing generic signature: " + t.getMessage());
+ "Unknown error parsing class signature: " + t.getMessage());
e.addSuppressed(t);
throw e;
}
@@ -100,7 +100,7 @@
throw e;
} catch (Throwable t) {
Error e = new GenericSignatureFormatError(
- "Unknown error parsing generic signature: " + t.getMessage());
+ "Unknown error parsing method signature: " + t.getMessage());
e.addSuppressed(t);
throw e;
}
@@ -112,14 +112,14 @@
setInput(signature);
parseFieldTypeSignature();
actions.stop();
- } catch (GenericSignatureFormatError e) {
- throw e;
- } catch (Throwable t) {
- Error e = new GenericSignatureFormatError(
- "Unknown error parsing generic signature: " + t.getMessage());
- e.addSuppressed(t);
- throw e;
- }
+ } catch (GenericSignatureFormatError e) {
+ throw e;
+ } catch (Throwable t) {
+ Error e = new GenericSignatureFormatError(
+ "Unknown error parsing field signature: " + t.getMessage());
+ e.addSuppressed(t);
+ throw e;
+ }
}
private void setInput(String input) {
diff --git a/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java b/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
index c68122c..56089f4 100644
--- a/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.ir.code.Invoke.Type;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor.InvokeKind;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
@@ -43,16 +44,17 @@
InvokeKind kind = targetExtractor.getKind();
if (target != null && target.getArity() == method.method.getArity()) {
assert !method.accessFlags.isPrivate() && !method.accessFlags.isConstructor();
- target = lense.lookupMethod(target, method);
if (kind == InvokeKind.STATIC) {
assert method.accessFlags.isStatic();
- DexEncodedMethod targetMethod = appInfo.lookupStaticTarget(target);
+ DexMethod actualTarget = lense.lookupMethod(target, method, Type.STATIC);
+ DexEncodedMethod targetMethod = appInfo.lookupStaticTarget(actualTarget);
if (targetMethod != null) {
addForwarding(method, targetMethod);
}
} else if (kind == InvokeKind.VIRTUAL) {
// TODO(herhut): Add support for bridges with multiple targets.
- DexEncodedMethod targetMethod = appInfo.lookupSingleVirtualTarget(target);
+ DexMethod actualTarget = lense.lookupMethod(target, method, Type.VIRTUAL);
+ DexEncodedMethod targetMethod = appInfo.lookupSingleVirtualTarget(actualTarget);
if (targetMethod != null) {
addForwarding(method, targetMethod);
}
@@ -86,31 +88,29 @@
}
@Override
- public DexType lookupType(DexType type, DexEncodedMethod context) {
- return previousLense.lookupType(type, context);
+ public DexType lookupType(DexType type) {
+ return previousLense.lookupType(type);
}
@Override
- public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context) {
- DexMethod previous = previousLense.lookupMethod(method, context);
+ public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
+ DexMethod previous = previousLense.lookupMethod(method, context, type);
DexMethod bridge = bridgeTargetToBridgeMap.get(previous);
// Do not forward calls from a bridge method to itself while the bridge method is still
// a bridge.
- if (bridge == null
- || (context.accessFlags.isBridge() && bridge == context.method)) {
+ if (bridge == null || (context.accessFlags.isBridge() && bridge == context.method)) {
return previous;
- } else {
- return bridge;
}
+ return bridge;
}
@Override
- public DexField lookupField(DexField field, DexEncodedMethod context) {
- return previousLense.lookupField(field, context);
+ public DexField lookupField(DexField field) {
+ return previousLense.lookupField(field);
}
@Override
- public boolean isContextFree() {
+ public boolean isContextFreeForMethods() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index 488cf2f..fc408e3 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -29,7 +29,7 @@
private final GraphLense.Builder builder = GraphLense.builder();
public MemberRebindingAnalysis(AppInfoWithLiveness appInfo, GraphLense lense) {
- assert lense.isContextFree();
+ assert lense.isContextFreeForMethods();
this.appInfo = appInfo;
this.lense = lense;
}
@@ -115,7 +115,7 @@
private void computeMethodRebinding(Set<DexMethod> methods,
Function<DexMethod, DexEncodedMethod> lookupTarget) {
for (DexMethod method : methods) {
- method = lense.lookupMethod(method, null);
+ method = lense.lookupMethod(method);
// We can safely ignore array types, as the corresponding methods are defined in a library.
if (!method.getHolder().isClassType()) {
continue;
@@ -188,7 +188,7 @@
BiFunction<DexClass, DexField, DexEncodedField> lookupTargetOnClass) {
for (Map.Entry<DexField, Set<DexEncodedMethod>> entry : fields.entrySet()) {
DexField field = entry.getKey();
- field = lense.lookupField(field, null);
+ field = lense.lookupField(field);
DexEncodedField target = lookup.apply(field.getHolder(), field);
// Rebind to the lowest library class or program class. Do not rebind accesses to fields that
// are not visible from the access context.
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
index 5847a80..ebf110e 100644
--- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor.InvokeKind;
import com.google.common.collect.Sets;
@@ -28,7 +29,8 @@
}
private void identifyBridgeMethod(DexEncodedMethod method) {
- if (method.accessFlags.isBridge()) {
+ MethodAccessFlags accessFlags = method.accessFlags;
+ if (accessFlags.isBridge() && !accessFlags.isAbstract()) {
InvokeSingleTargetExtractor targetExtractor = new InvokeSingleTargetExtractor();
method.getCode().registerCodeReferences(targetExtractor);
DexMethod target = targetExtractor.getTarget();
@@ -36,11 +38,11 @@
// javac-generated visibility forward bridge method has same descriptor (name, signature and
// return type).
if (target != null && target.hasSameProtoAndName(method.method)) {
- assert !method.accessFlags.isPrivate() && !method.accessFlags.isConstructor();
+ assert !accessFlags.isPrivate() && !accessFlags.isConstructor();
if (kind == InvokeKind.SUPER) {
// This is a visibility forward, so check for the direct target.
- DexEncodedMethod targetMethod
- = appInfo.resolveMethod(target.getHolder(), target).asSingleTarget();
+ DexEncodedMethod targetMethod =
+ appInfo.resolveMethod(target.getHolder(), target).asSingleTarget();
if (targetMethod != null && targetMethod.accessFlags.isPublic()) {
if (Log.ENABLED) {
Log.info(getClass(), "Removing visibility forwarding %s -> %s", method.method,
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 51feb9d..2b51720 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -71,7 +71,6 @@
import java.util.SortedSet;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -1639,8 +1638,8 @@
this.liveTypes = rewriteItems(previous.liveTypes, lense::lookupType);
this.instantiatedTypes = rewriteItems(previous.instantiatedTypes, lense::lookupType);
this.instantiatedLambdas = rewriteItems(previous.instantiatedLambdas, lense::lookupType);
- this.targetedMethods = rewriteItems(previous.targetedMethods, lense::lookupMethod);
- this.liveMethods = rewriteItems(previous.liveMethods, lense::lookupMethod);
+ this.targetedMethods = rewriteMethodsConservatively(previous.targetedMethods, lense);
+ this.liveMethods = rewriteMethodsConservatively(previous.liveMethods, lense);
this.liveFields = rewriteItems(previous.liveFields, lense::lookupField);
this.instanceFieldReads =
rewriteKeysWhileMergingValues(previous.instanceFieldReads, lense::lookupField);
@@ -1653,11 +1652,11 @@
this.fieldsRead = rewriteItems(previous.fieldsRead, lense::lookupField);
this.fieldsWritten = rewriteItems(previous.fieldsWritten, lense::lookupField);
this.pinnedItems = rewriteMixedItems(previous.pinnedItems, lense);
- this.virtualInvokes = rewriteItems(previous.virtualInvokes, lense::lookupMethod);
- this.interfaceInvokes = rewriteItems(previous.interfaceInvokes, lense::lookupMethod);
- this.superInvokes = rewriteItems(previous.superInvokes, lense::lookupMethod);
- this.directInvokes = rewriteItems(previous.directInvokes, lense::lookupMethod);
- this.staticInvokes = rewriteItems(previous.staticInvokes, lense::lookupMethod);
+ this.virtualInvokes = rewriteMethodsConservatively(previous.virtualInvokes, lense);
+ this.interfaceInvokes = rewriteMethodsConservatively(previous.interfaceInvokes, lense);
+ this.superInvokes = rewriteMethodsConservatively(previous.superInvokes, lense);
+ this.directInvokes = rewriteMethodsConservatively(previous.directInvokes, lense);
+ this.staticInvokes = rewriteMethodsConservatively(previous.staticInvokes, lense);
this.prunedTypes = rewriteItems(previous.prunedTypes, lense::lookupType);
// TODO(herhut): Migrate these to Descriptors, as well.
assert assertNotModifiedByLense(previous.noSideEffects.keySet(), lense);
@@ -1739,16 +1738,16 @@
for (DexItem item : items) {
if (item instanceof DexClass) {
DexType type = ((DexClass) item).type;
- assert lense.lookupType(type, null) == type;
+ assert lense.lookupType(type) == type;
} else if (item instanceof DexEncodedMethod) {
DexEncodedMethod method = (DexEncodedMethod) item;
// We only allow changes to bridge methods, as these get retargeted even if they
// are kept.
assert method.accessFlags.isBridge()
- || lense.lookupMethod(method.method, null) == method.method;
+ || lense.lookupMethod(method.method) == method.method;
} else if (item instanceof DexEncodedField) {
DexField field = ((DexEncodedField) item).field;
- assert lense.lookupField(field, null) == field;
+ assert lense.lookupField(field) == field;
} else {
assert false;
}
@@ -1793,31 +1792,54 @@
return builder.build();
}
+ private static ImmutableSortedSet<DexMethod> rewriteMethodsConservatively(
+ Set<DexMethod> original, GraphLense lense) {
+ ImmutableSortedSet.Builder<DexMethod> builder =
+ new ImmutableSortedSet.Builder<>(PresortedComparable::slowCompare);
+ if (lense.isContextFreeForMethods()) {
+ for (DexMethod item : original) {
+ builder.add(lense.lookupMethod(item));
+ }
+ } else {
+ for (DexMethod item : original) {
+ // Avoid using lookupMethodInAllContexts when possible.
+ if (lense.isContextFreeForMethod(item)) {
+ builder.add(lense.lookupMethod(item));
+ } else {
+ // The lense is context sensitive, but we do not have the context here. Therefore, we
+ // conservatively look up the method in all contexts.
+ builder.addAll(lense.lookupMethodInAllContexts(item));
+ }
+ }
+ }
+ return builder.build();
+ }
+
private static <T extends PresortedComparable<T>> ImmutableSortedSet<T> rewriteItems(
- Set<T> original, BiFunction<T, DexEncodedMethod, T> rewrite) {
+ Set<T> original, Function<T, T> rewrite) {
ImmutableSortedSet.Builder<T> builder =
new ImmutableSortedSet.Builder<>(PresortedComparable::slowCompare);
for (T item : original) {
- builder.add(rewrite.apply(item, null));
+ builder.add(rewrite.apply(item));
}
return builder.build();
}
private static <T extends PresortedComparable<T>, S> ImmutableMap<T, S> rewriteKeys(
- Map<T, S> original, BiFunction<T, DexEncodedMethod, T> rewrite) {
+ Map<T, S> original, Function<T, T> rewrite) {
ImmutableMap.Builder<T, S> builder = new ImmutableMap.Builder<>();
for (T item : original.keySet()) {
- builder.put(rewrite.apply(item, null), original.get(item));
+ builder.put(rewrite.apply(item), original.get(item));
}
return builder.build();
}
- private static <T extends PresortedComparable<T>, S> Map<T, Set<S>>
- rewriteKeysWhileMergingValues(
- Map<T, Set<S>> original, BiFunction<T, DexEncodedMethod, T> rewrite) {
+ private static <T extends PresortedComparable<T>, S>
+ Map<T, Set<S>> rewriteKeysWhileMergingValues(
+ Map<T, Set<S>> original, Function<T, T> rewrite) {
Map<T, Set<S>> result = new IdentityHashMap<>();
for (T item : original.keySet()) {
- T rewrittenKey = rewrite.apply(item, null);
+ T rewrittenKey = rewrite.apply(item);
result.computeIfAbsent(rewrittenKey, k -> Sets.newIdentityHashSet())
.addAll(original.get(item));
}
@@ -1830,11 +1852,11 @@
for (DexItem item : original) {
// TODO(b/67934123) There should be a common interface to perform the dispatch.
if (item instanceof DexType) {
- builder.add(lense.lookupType((DexType) item, null));
+ builder.add(lense.lookupType((DexType) item));
} else if (item instanceof DexMethod) {
- builder.add(lense.lookupMethod((DexMethod) item, null));
+ builder.add(lense.lookupMethod((DexMethod) item));
} else if (item instanceof DexField) {
- builder.add(lense.lookupField((DexField) item, null));
+ builder.add(lense.lookupField((DexField) item));
} else {
throw new Unreachable();
}
@@ -1893,7 +1915,6 @@
public AppInfoWithLiveness rewrittenWithLense(DirectMappedDexApplication application,
GraphLense lense) {
- assert lense.isContextFree();
return new AppInfoWithLiveness(this, application, lense);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
similarity index 60%
rename from src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
rename to src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index a80e758..d54c95c 100644
--- a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -18,7 +19,12 @@
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.GraphLense.Builder;
import com.android.tools.r8.graph.KeyedDexItem;
+import com.android.tools.r8.graph.MethodAccessFlags;
+import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.graph.PresortedComparable;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.android.tools.r8.ir.synthetic.ForwardMethodSourceCode;
+import com.android.tools.r8.ir.synthetic.SynthesizedCode;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
@@ -32,13 +38,17 @@
import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
@@ -46,37 +56,46 @@
/**
* Merges Supertypes with a single implementation into their single subtype.
- * <p>
- * A common use-case for this is to merge an interface into its single implementation.
- * <p>
- * The class merger only fixes the structure of the graph but leaves the actual instructions
- * untouched. Fixup of instructions is deferred via a {@link GraphLense} to the Ir building phase.
+ *
+ * <p>A common use-case for this is to merge an interface into its single implementation.
+ *
+ * <p>The class merger only fixes the structure of the graph but leaves the actual instructions
+ * untouched. Fixup of instructions is deferred via a {@link GraphLense} to the IR building phase.
*/
-public class SimpleClassMerger {
+public class VerticalClassMerger {
private final DexApplication application;
private final AppInfoWithLiveness appInfo;
private final GraphLense graphLense;
- private final GraphLense.Builder renamedMembersLense = GraphLense.builder();
private final Map<DexType, DexType> mergedClasses = new IdentityHashMap<>();
private final Timing timing;
private Collection<DexMethod> invokes;
- private int numberOfMerges = 0;
- public SimpleClassMerger(DexApplication application, AppInfoWithLiveness appInfo,
- GraphLense graphLense, Timing timing) {
+ public VerticalClassMerger(
+ DexApplication application,
+ AppInfoWithLiveness appInfo,
+ GraphLense graphLense,
+ Timing timing) {
this.application = application;
this.appInfo = appInfo;
this.graphLense = graphLense;
this.timing = timing;
}
- private boolean isMergeCandidate(DexProgramClass clazz) {
+ // Returns a set of types that must not be merged into other types.
+ private Set<DexType> getPinnedTypes(Iterable<DexProgramClass> classes) {
+ // TODO(christofferqa): Compute types from [classes] that must be pinned in order for vertical
+ // class merging to work.
+ return new HashSet<>();
+ }
+
+ private boolean isMergeCandidate(DexProgramClass clazz, Set<DexType> pinnedTypes) {
// We can merge program classes if they are not instantiated, have a single subtype
// and we do not have to keep them.
return !clazz.isLibraryClass()
&& !appInfo.instantiatedTypes.contains(clazz.type)
&& !appInfo.isPinned(clazz.type)
+ && !pinnedTypes.contains(clazz.type)
&& clazz.type.getSingleSubtype() != null;
}
@@ -135,53 +154,123 @@
return result;
}
+ private void addAncestorsToWorklist(
+ DexProgramClass clazz, Deque<DexProgramClass> worklist, Set<DexProgramClass> seenBefore) {
+ if (seenBefore.contains(clazz)) {
+ return;
+ }
+
+ worklist.addFirst(clazz);
+
+ // Add super classes to worklist.
+ if (clazz.superType != null) {
+ DexClass definition = appInfo.definitionFor(clazz.superType);
+ if (definition != null && definition.isProgramClass()) {
+ addAncestorsToWorklist(definition.asProgramClass(), worklist, seenBefore);
+ }
+ }
+
+ // Add super interfaces to worklist.
+ for (DexType interfaceType : clazz.interfaces.values) {
+ DexClass definition = appInfo.definitionFor(interfaceType);
+ if (definition != null && definition.isProgramClass()) {
+ addAncestorsToWorklist(definition.asProgramClass(), worklist, seenBefore);
+ }
+ }
+ }
+
private GraphLense mergeClasses(GraphLense graphLense) {
- for (DexProgramClass clazz : application.classes()) {
- if (isMergeCandidate(clazz)) {
- DexClass targetClass = appInfo.definitionFor(clazz.type.getSingleSubtype());
- if (appInfo.isPinned(targetClass.type)) {
- // We have to keep the target class intact, so we cannot merge it.
+ Iterable<DexProgramClass> classes = application.classesWithDeterministicOrder();
+ Deque<DexProgramClass> worklist = new ArrayDeque<>();
+ Set<DexProgramClass> seenBefore = new HashSet<>();
+
+ int numberOfMerges = 0;
+
+ // Types that are pinned (in addition to those where appInfo.isPinned returns true).
+ Set<DexType> pinnedTypes = getPinnedTypes(classes);
+
+ // The resulting graph lense that should be used after class merging.
+ VerticalClassMergerGraphLense.Builder renamedMembersLense =
+ new VerticalClassMergerGraphLense.Builder();
+
+ Iterator<DexProgramClass> classIterator = classes.iterator();
+
+ // Visit the program classes in a top-down order according to the class hierarchy.
+ while (classIterator.hasNext() || !worklist.isEmpty()) {
+ if (worklist.isEmpty()) {
+ // Add the ancestors of this class (including the class itself) to the worklist in such a
+ // way that all super types of the class come before the class itself.
+ addAncestorsToWorklist(classIterator.next(), worklist, seenBefore);
+ if (worklist.isEmpty()) {
continue;
}
- if (mergedClasses.containsKey(targetClass.type)) {
- // TODO(herhut): Traverse top-down.
- continue;
- }
- if (clazz.hasClassInitializer() && targetClass.hasClassInitializer()) {
- // TODO(herhut): Handle class initializers.
- if (Log.ENABLED) {
- Log.info(getClass(), "Cannot merge %s into %s due to static initializers.",
- clazz.toSourceString(), targetClass.toSourceString());
- }
- continue;
- }
- // Guard against the case where we have two methods that may get the same signature
- // if we replace types. This is rare, so we approximate and err on the safe side here.
- if (new CollisionDetector(clazz.type, targetClass.type, getInvokes(), mergedClasses)
- .mayCollide()) {
- if (Log.ENABLED) {
- Log.info(getClass(), "Cannot merge %s into %s due to conflict.", clazz.toSourceString(),
- targetClass.toSourceString());
- }
- continue;
- }
- boolean merged = new ClassMerger(clazz, targetClass).merge();
+ }
+
+ DexProgramClass clazz = worklist.removeFirst();
+ if (!seenBefore.add(clazz) || !isMergeCandidate(clazz, pinnedTypes)) {
+ continue;
+ }
+
+ DexClass targetClass = appInfo.definitionFor(clazz.type.getSingleSubtype());
+ assert !mergedClasses.containsKey(targetClass.type);
+ if (appInfo.isPinned(targetClass.type)) {
+ // We have to keep the target class intact, so we cannot merge it.
+ continue;
+ }
+ if (clazz.hasClassInitializer() && targetClass.hasClassInitializer()) {
+ // TODO(herhut): Handle class initializers.
if (Log.ENABLED) {
- if (merged) {
- numberOfMerges++;
- Log.info(getClass(), "Merged class %s into %s.", clazz.toSourceString(),
- targetClass.toSourceString());
- } else {
- Log.info(getClass(), "Aborted merge for class %s into %s.",
- clazz.toSourceString(), targetClass.toSourceString());
- }
+ Log.info(
+ getClass(),
+ "Cannot merge %s into %s due to static initializers.",
+ clazz.toSourceString(),
+ targetClass.toSourceString());
+ }
+ continue;
+ }
+ // Guard against the case where we have two methods that may get the same signature
+ // if we replace types. This is rare, so we approximate and err on the safe side here.
+ if (new CollisionDetector(clazz.type, targetClass.type, getInvokes(), mergedClasses)
+ .mayCollide()) {
+ if (Log.ENABLED) {
+ Log.info(
+ getClass(),
+ "Cannot merge %s into %s due to conflict.",
+ clazz.toSourceString(),
+ targetClass.toSourceString());
+ }
+ continue;
+ }
+ ClassMerger merger = new ClassMerger(clazz, targetClass);
+ boolean merged = merger.merge();
+ if (merged) {
+ // Commit the changes to the graph lense.
+ renamedMembersLense.merge(merger.getRenamings());
+ // Do not allow merging the resulting class into its subclass.
+ // TODO(christofferqa): Get rid of this limitation.
+ pinnedTypes.add(targetClass.type);
+ }
+ if (Log.ENABLED) {
+ if (merged) {
+ numberOfMerges++;
+ Log.info(
+ getClass(),
+ "Merged class %s into %s.",
+ clazz.toSourceString(),
+ targetClass.toSourceString());
+ } else {
+ Log.info(
+ getClass(),
+ "Aborted merge for class %s into %s.",
+ clazz.toSourceString(),
+ targetClass.toSourceString());
}
}
}
if (Log.ENABLED) {
Log.debug(getClass(), "Merged %d classes.", numberOfMerges);
}
- return renamedMembersLense.build(application.dexItemFactory, graphLense);
+ return renamedMembersLense.build(graphLense);
}
private class ClassMerger {
@@ -190,7 +279,8 @@
private final DexClass source;
private final DexClass target;
- private final Map<DexEncodedMethod, DexEncodedMethod> deferredRenamings = new HashMap<>();
+ private final VerticalClassMergerGraphLense.Builder deferredRenamings =
+ new VerticalClassMergerGraphLense.Builder();
private boolean abortMerge = false;
private ClassMerger(DexClass source, DexClass target) {
@@ -210,25 +300,81 @@
Set<Wrapper<DexMethod>> existingMethods = new HashSet<>();
addAll(existingMethods, target.directMethods(), MethodSignatureEquivalence.get());
addAll(existingMethods, target.virtualMethods(), MethodSignatureEquivalence.get());
- Collection<DexEncodedMethod> mergedDirectMethods = mergeItems(
- Iterators.transform(Iterators.forArray(source.directMethods()), this::renameConstructors),
- target.directMethods(),
- MethodSignatureEquivalence.get(),
- existingMethods,
- this::renameMethod
- );
- Iterator<DexEncodedMethod> methods = Iterators.forArray(source.virtualMethods());
- if (source.accessFlags.isInterface()) {
- // If merging an interface, only merge methods that are not otherwise defined in the
- // target class.
- methods = Iterators.transform(methods, this::filterShadowedInterfaceMethods);
+
+ List<DexEncodedMethod> directMethods = new ArrayList<>();
+ for (DexEncodedMethod directMethod : source.directMethods()) {
+ if (directMethod.isInstanceInitializer()) {
+ directMethods.add(renameConstructor(directMethod));
+ } else {
+ directMethods.add(directMethod);
+ }
}
- Collection<DexEncodedMethod> mergedVirtualMethods = mergeItems(
- methods,
- target.virtualMethods(),
- MethodSignatureEquivalence.get(),
- existingMethods,
- this::abortOnNonAbstract);
+
+ List<DexEncodedMethod> virtualMethods = new ArrayList<>();
+ for (DexEncodedMethod virtualMethod : source.virtualMethods()) {
+ DexEncodedMethod shadowedBy = findMethodInTarget(virtualMethod);
+ if (shadowedBy != null) {
+ if (virtualMethod.accessFlags.isAbstract()) {
+ // Remove abstract/interface methods that are shadowed.
+ deferredRenamings.map(virtualMethod.method, shadowedBy.method);
+ continue;
+ }
+ } else {
+ // The method is shadowed. If it is abstract, we can simply move it to the subclass.
+ // Non-abstract methods are handled below (they cannot simply be moved to the subclass as
+ // a virtual method, because they might be the target of an invoke-super instruction).
+ if (virtualMethod.accessFlags.isAbstract()) {
+ DexEncodedMethod resultingVirtualMethod =
+ renameMethod(virtualMethod, target.type, false);
+ deferredRenamings.map(virtualMethod.method, resultingVirtualMethod.method);
+ virtualMethods.add(resultingVirtualMethod);
+ continue;
+ }
+ }
+
+ // This virtual method could be called directly from a sub class via an invoke-super
+ // instruction. Therefore, we translate this virtual method into a direct method, such that
+ // relevant invoke-super instructions can be rewritten into invoke-direct instructions.
+ DexEncodedMethod resultingDirectMethod = renameMethod(virtualMethod, target.type, true);
+ makePrivate(resultingDirectMethod);
+ directMethods.add(resultingDirectMethod);
+
+ // Record that invoke-super instructions in the target class should be redirected to the
+ // newly created direct method.
+ deferredRenamings.mapVirtualMethodToDirectInType(
+ virtualMethod.method, resultingDirectMethod.method, target.type);
+
+ if (shadowedBy == null) {
+ // In addition to the newly added direct method, create a virtual method such that we do
+ // not accidentally remove the method from the interface of this class.
+ // Note that this method is added independently of whether it will actually be used. If
+ // it turns out that the method is never used, it will be removed by the final round
+ // of tree shaking.
+ shadowedBy = buildBridgeMethod(virtualMethod, resultingDirectMethod.method);
+ virtualMethods.add(shadowedBy);
+ }
+
+ deferredRenamings.map(virtualMethod.method, shadowedBy.method);
+ }
+
+ Collection<DexEncodedMethod> mergedDirectMethods =
+ mergeItems(
+ directMethods.iterator(),
+ target.directMethods(),
+ MethodSignatureEquivalence.get(),
+ existingMethods,
+ (existing, method) -> {
+ DexEncodedMethod renamedMethod = renameMethod(method, target.type, true);
+ deferredRenamings.map(method.method, renamedMethod.method);
+ return renamedMethod;
+ });
+ Collection<DexEncodedMethod> mergedVirtualMethods =
+ mergeItems(
+ virtualMethods.iterator(),
+ target.virtualMethods(),
+ MethodSignatureEquivalence.get(),
+ existingMethods,
+ this::abortOnNonAbstract);
if (abortMerge) {
return false;
}
@@ -279,22 +425,46 @@
source.interfaces = DexTypeList.empty();
// Step 4: Record merging.
mergedClasses.put(source.type, target.type);
- // Step 5: Make deferred renamings final.
- deferredRenamings.forEach((from, to) -> renamedMembersLense.map(from.method, to.method));
return true;
}
- private DexEncodedMethod filterShadowedInterfaceMethods(DexEncodedMethod m) {
- DexEncodedMethod actual = appInfo.resolveMethod(target.type, m.method).asSingleTarget();
- assert actual != null;
- if (actual != m) {
- // We will drop a method here, so record it as a potential renaming.
- deferredRenamings.put(m, actual);
+ public VerticalClassMergerGraphLense.Builder getRenamings() {
+ return deferredRenamings;
+ }
+
+ private DexEncodedMethod buildBridgeMethod(
+ DexEncodedMethod signature, DexMethod invocationTarget) {
+ DexType holder = target.type;
+ DexProto proto = invocationTarget.proto;
+ DexString name = signature.method.name;
+ MethodAccessFlags accessFlags = signature.accessFlags.copy();
+ accessFlags.setBridge();
+ accessFlags.setSynthetic();
+ accessFlags.unsetAbstract();
+ return new DexEncodedMethod(
+ application.dexItemFactory.createMethod(holder, proto, name),
+ accessFlags,
+ DexAnnotationSet.empty(),
+ ParameterAnnotationsList.empty(),
+ new SynthesizedCode(
+ new ForwardMethodSourceCode(holder, proto, holder, invocationTarget, Type.DIRECT),
+ registry -> registry.registerInvokeDirect(invocationTarget)));
+ }
+
+ // Returns the method that shadows the given method, or null if method is not shadowed.
+ private DexEncodedMethod findMethodInTarget(DexEncodedMethod method) {
+ DexEncodedMethod actual = appInfo.resolveMethod(target.type, method.method).asSingleTarget();
+ if (actual == null) {
+ // May happen in case of missing classes.
+ abortMerge = true;
return null;
}
- // We will keep the method, so the class better be abstract.
- assert target.accessFlags.isAbstract();
- return m;
+ if (actual != method) {
+ return actual;
+ }
+ // We will keep the method, so the class better be abstract if there is no implementation.
+ assert !method.accessFlags.isAbstract() || target.accessFlags.isAbstract();
+ return null;
}
private <T extends KeyedDexItem<S>, S extends PresortedComparable<S>> void addAll(
@@ -350,80 +520,85 @@
}
}
- private DexString makeMergedName(String nameString, DexType holder) {
- return application.dexItemFactory
- .createString(nameString + "$" + holder.toSourceString().replace('.', '$'));
+ // Note that names returned by this function are not necessarily unique. However, class merging
+ // is aborted if it turns out that the returned name is not unique.
+ // TODO(christofferqa): Write a test that checks that class merging is aborted if this function
+ // generates a name that is not unique.
+ private DexString getFreshName(String nameString, DexType holder) {
+ String freshName = nameString + "$" + holder.toSourceString().replace('.', '$');
+ return application.dexItemFactory.createString(freshName);
}
private DexEncodedMethod abortOnNonAbstract(DexEncodedMethod existing,
DexEncodedMethod method) {
- if (existing == null) {
- // This is a conflict between a static and virtual method. Abort.
- abortMerge = true;
- return method;
- }
- // Ignore if we merge in an abstract method or if we override a bridge method that would
- // bridge to the superclasses method.
- if (method.accessFlags.isAbstract()) {
- // We make a method disappear here, so record the renaming so that calls to the previous
- // target get forwarded properly.
- deferredRenamings.put(method, existing);
- return existing;
- } else if (existing.accessFlags.isBridge()) {
+ // Ignore if we override a bridge method that would bridge to the superclasses method.
+ if (existing != null && existing.accessFlags.isBridge()) {
InvokeSingleTargetExtractor extractor = new InvokeSingleTargetExtractor();
existing.getCode().registerCodeReferences(extractor);
- if (extractor.getTarget() != method.method) {
- abortMerge = true;
+ if (extractor.getTarget() == method.method) {
+ return method;
}
- return method;
- } else {
- abortMerge = true;
- return existing;
}
+
+ // Abstract shadowed methods are removed prior to calling mergeItems.
+ assert !method.accessFlags.isAbstract();
+
+ // If [existing] is null, there is a conflict between a static and virtual method. Otherwise,
+ // there is a non-abstract method that is shadowed. We translate virtual shadowed methods into
+ // direct methods with a fresh name, so this situation should never happen. We currently get
+ // in this situation if getFreshName returns a name that is not unique, though.
+ abortMerge = true;
+ return method;
}
- private DexEncodedMethod renameConstructors(DexEncodedMethod method) {
- // Only rename instance initializers.
- if (!method.isInstanceInitializer()) {
- return method;
- }
+ private DexEncodedMethod renameConstructor(DexEncodedMethod method) {
+ assert method.isInstanceInitializer();
DexType holder = method.method.holder;
- DexEncodedMethod result = method
- .toRenamedMethod(makeMergedName(CONSTRUCTOR_NAME, holder), application.dexItemFactory);
+ DexEncodedMethod result =
+ method.toRenamedMethod(
+ getFreshName(CONSTRUCTOR_NAME, holder), application.dexItemFactory);
result.markForceInline();
- deferredRenamings.put(method, result);
+ deferredRenamings.map(method.method, result.method);
// Renamed constructors turn into ordinary private functions. They can be private, as
// they are only references from their direct subclass, which they were merged into.
result.accessFlags.unsetConstructor();
- result.accessFlags.unsetPublic();
- result.accessFlags.unsetProtected();
- result.accessFlags.setPrivate();
+ makePrivate(result);
return result;
}
- private DexEncodedMethod renameMethod(DexEncodedMethod existing, DexEncodedMethod method) {
+ private DexEncodedMethod renameMethod(
+ DexEncodedMethod method, DexType newHolder, boolean useFreshName) {
// We cannot handle renaming static initializers yet and constructors should have been
// renamed already.
assert !method.accessFlags.isConstructor();
- DexType holder = method.method.holder;
- String name = method.method.name.toSourceString();
- DexEncodedMethod result = method
- .toRenamedMethod(makeMergedName(name, holder), application.dexItemFactory);
- renamedMembersLense.map(method.method, result.method);
- return result;
+ DexType oldHolder = method.method.holder;
+ DexString oldName = method.method.name;
+ DexString newName =
+ useFreshName ? getFreshName(oldName.toSourceString(), oldHolder) : oldName;
+ DexMethod newSignature =
+ application.dexItemFactory.createMethod(newHolder, method.method.proto, newName);
+ return method.toTypeSubstitutedMethod(newSignature);
}
private DexEncodedField renameField(DexEncodedField existing, DexEncodedField field) {
DexString oldName = field.field.name;
- DexType holder = field.field.clazz;
- DexEncodedField result = field
- .toRenamedField(makeMergedName(oldName.toSourceString(), holder),
- application.dexItemFactory);
- renamedMembersLense.map(field.field, result.field);
+ DexType oldHolder = field.field.clazz;
+ DexString newName = getFreshName(oldName.toSourceString(), oldHolder);
+ DexField newSignature =
+ application.dexItemFactory.createField(target.type, field.field.type, newName);
+ DexEncodedField result = field.toTypeSubstitutedField(newSignature);
+ deferredRenamings.map(field.field, result.field);
return result;
}
}
+ private static void makePrivate(DexEncodedMethod method) {
+ assert !method.accessFlags.isAbstract();
+ method.accessFlags.unsetPublic();
+ method.accessFlags.unsetProtected();
+ method.accessFlags.setPrivate();
+ }
+
private class TreeFixer {
private final Builder lense = GraphLense.builder();
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
new file mode 100644
index 0000000..ea57d1d
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
@@ -0,0 +1,175 @@
+// Copyright (c) 2018, 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.shaking;
+
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+// This graph lense is instantiated during vertical class merging. The graph lense is context
+// sensitive in the enclosing class of a given invoke *and* the type of the invoke (e.g., invoke-
+// super vs invoke-virtual). This is illustrated by the following example.
+//
+// public class A {
+// public void m() { ... }
+// }
+// public class B extends A {
+// @Override
+// public void m() { invoke-super A.m(); ... }
+//
+// public void m2() { invoke-virtual A.m(); ... }
+// }
+//
+// Vertical class merging will merge class A into class B. Since class B already has a method with
+// the signature "void B.m()", the method A.m will be given a fresh name and moved to class B.
+// During this process, the method corresponding to A.m will be made private such that it can be
+// called via an invoke-direct instruction.
+//
+// For the invocation "invoke-super A.m()" in B.m, this graph lense will return the newly created,
+// private method corresponding to A.m (that is now in B.m with a fresh name), such that the
+// invocation will hit the same implementation as the original super.m() call.
+//
+// For the invocation "invoke-virtual A.m()" in B.m2, this graph lense will return the method B.m.
+public class VerticalClassMergerGraphLense extends GraphLense {
+ private final GraphLense previousLense;
+
+ private final Map<DexField, DexField> fieldMap;
+ private final Map<DexMethod, DexMethod> methodMap;
+ private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps;
+
+ public VerticalClassMergerGraphLense(
+ Map<DexField, DexField> fieldMap,
+ Map<DexMethod, DexMethod> methodMap,
+ Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps,
+ GraphLense previousLense) {
+ this.previousLense = previousLense;
+ this.fieldMap = fieldMap;
+ this.methodMap = methodMap;
+ this.contextualVirtualToDirectMethodMaps = contextualVirtualToDirectMethodMaps;
+ }
+
+ @Override
+ public DexType lookupType(DexType type) {
+ return previousLense.lookupType(type);
+ }
+
+ @Override
+ public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
+ assert isContextFreeForMethod(method) || (context != null && type != null);
+ DexMethod previous = previousLense.lookupMethod(method, context, type);
+ if (type == Type.SUPER) {
+ Map<DexMethod, DexMethod> virtualToDirectMethodMap =
+ contextualVirtualToDirectMethodMaps.get(context.method.holder);
+ if (virtualToDirectMethodMap != null) {
+ DexMethod directMethod = virtualToDirectMethodMap.get(previous);
+ if (directMethod != null) {
+ return directMethod;
+ }
+ }
+ }
+ return methodMap.getOrDefault(previous, previous);
+ }
+
+ @Override
+ public Set<DexMethod> lookupMethodInAllContexts(DexMethod method) {
+ ImmutableSet.Builder<DexMethod> builder = ImmutableSet.builder();
+ for (DexMethod previous : previousLense.lookupMethodInAllContexts(method)) {
+ builder.add(methodMap.getOrDefault(previous, previous));
+ for (Map<DexMethod, DexMethod> virtualToDirectMethodMap :
+ contextualVirtualToDirectMethodMaps.values()) {
+ DexMethod directMethod = virtualToDirectMethodMap.get(previous);
+ if (directMethod != null) {
+ builder.add(directMethod);
+ }
+ }
+ }
+ return builder.build();
+ }
+
+ @Override
+ public DexField lookupField(DexField field) {
+ DexField previous = previousLense.lookupField(field);
+ return fieldMap.getOrDefault(previous, previous);
+ }
+
+ @Override
+ public boolean isContextFreeForMethods() {
+ return contextualVirtualToDirectMethodMaps.isEmpty() && previousLense.isContextFreeForMethods();
+ }
+
+ @Override
+ public boolean isContextFreeForMethod(DexMethod method) {
+ if (!previousLense.isContextFreeForMethod(method)) {
+ return false;
+ }
+ DexMethod previous = previousLense.lookupMethod(method);
+ for (Map<DexMethod, DexMethod> virtualToDirectMethodMap :
+ contextualVirtualToDirectMethodMaps.values()) {
+ if (virtualToDirectMethodMap.containsKey(previous)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ public static class Builder {
+
+ private final ImmutableMap.Builder<DexField, DexField> fieldMapBuilder = ImmutableMap.builder();
+ private final ImmutableMap.Builder<DexMethod, DexMethod> methodMapBuilder =
+ ImmutableMap.builder();
+ private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps =
+ new HashMap<>();
+
+ public Builder() {}
+
+ public GraphLense build(GraphLense previousLense) {
+ Map<DexField, DexField> fieldMap = fieldMapBuilder.build();
+ Map<DexMethod, DexMethod> methodMap = methodMapBuilder.build();
+ if (fieldMap.isEmpty()
+ && methodMap.isEmpty()
+ && contextualVirtualToDirectMethodMaps.isEmpty()) {
+ return previousLense;
+ }
+ return new VerticalClassMergerGraphLense(
+ fieldMap, methodMap, contextualVirtualToDirectMethodMaps, previousLense);
+ }
+
+ public void map(DexField from, DexField to) {
+ fieldMapBuilder.put(from, to);
+ }
+
+ public void map(DexMethod from, DexMethod to) {
+ methodMapBuilder.put(from, to);
+ }
+
+ public void mapVirtualMethodToDirectInType(DexMethod from, DexMethod to, DexType type) {
+ Map<DexMethod, DexMethod> virtualToDirectMethodMap =
+ contextualVirtualToDirectMethodMaps.computeIfAbsent(type, key -> new HashMap<>());
+ virtualToDirectMethodMap.put(from, to);
+ }
+
+ public void merge(VerticalClassMergerGraphLense.Builder builder) {
+ fieldMapBuilder.putAll(builder.fieldMapBuilder.build());
+ methodMapBuilder.putAll(builder.methodMapBuilder.build());
+ for (DexType context : builder.contextualVirtualToDirectMethodMaps.keySet()) {
+ Map<DexMethod, DexMethod> current = contextualVirtualToDirectMethodMaps.get(context);
+ Map<DexMethod, DexMethod> other = builder.contextualVirtualToDirectMethodMaps.get(context);
+ if (current != null) {
+ current.putAll(other);
+ } else {
+ contextualVirtualToDirectMethodMaps.put(context, other);
+ }
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/utils/ArchiveResourceProvider.java b/src/main/java/com/android/tools/r8/utils/ArchiveResourceProvider.java
index 499ba47..0f06ca8 100644
--- a/src/main/java/com/android/tools/r8/utils/ArchiveResourceProvider.java
+++ b/src/main/java/com/android/tools/r8/utils/ArchiveResourceProvider.java
@@ -22,6 +22,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -49,7 +50,7 @@
private List<ProgramResource> readArchive() throws IOException {
List<ProgramResource> dexResources = new ArrayList<>();
List<ProgramResource> classResources = new ArrayList<>();
- try (ZipFile zipFile = new ZipFile(archive.getPath().toFile())) {
+ try (ZipFile zipFile = new ZipFile(archive.getPath().toFile(), StandardCharsets.UTF_8)) {
final Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
@@ -106,7 +107,7 @@
@Override
public void accept(Visitor resourceBrowser) throws ResourceException {
- try (ZipFile zipFile = new ZipFile(archive.getPath().toFile())) {
+ try (ZipFile zipFile = new ZipFile(archive.getPath().toFile(), StandardCharsets.UTF_8)) {
final Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
diff --git a/src/main/java/com/android/tools/r8/utils/ZipUtils.java b/src/main/java/com/android/tools/r8/utils/ZipUtils.java
index 1b032e5..849377b 100644
--- a/src/main/java/com/android/tools/r8/utils/ZipUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ZipUtils.java
@@ -10,6 +10,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Enumeration;
@@ -27,7 +28,7 @@
}
public static void iter(String zipFileStr, OnEntryHandler handler) throws IOException {
- try (ZipFile zipFile = new ZipFile(zipFileStr)) {
+ try (ZipFile zipFile = new ZipFile(zipFileStr, StandardCharsets.UTF_8)) {
final Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
diff --git a/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java b/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
index ad31005..2ef0123 100644
--- a/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
+++ b/src/test/apiUsageSample/com/android/tools/apiusagesample/D8ApiUsageSample.java
@@ -22,6 +22,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -387,7 +388,7 @@
for (Path file : files) {
if (isArchive(file)) {
Origin zipOrigin = new PathOrigin(file);
- ZipInputStream zip = new ZipInputStream(Files.newInputStream(file));
+ ZipInputStream zip = new ZipInputStream(Files.newInputStream(file), StandardCharsets.UTF_8);
ZipEntry entry;
while (null != (entry = zip.getNextEntry())) {
if (isClassFile(Paths.get(entry.getName()))) {
diff --git a/src/test/apiUsageSample/com/android/tools/apiusagesample/R8ApiUsageSample.java b/src/test/apiUsageSample/com/android/tools/apiusagesample/R8ApiUsageSample.java
index 7230f55..2c2e8d7 100644
--- a/src/test/apiUsageSample/com/android/tools/apiusagesample/R8ApiUsageSample.java
+++ b/src/test/apiUsageSample/com/android/tools/apiusagesample/R8ApiUsageSample.java
@@ -22,6 +22,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -378,7 +379,7 @@
for (Path file : files) {
if (isArchive(file)) {
Origin zipOrigin = new PathOrigin(file);
- ZipInputStream zip = new ZipInputStream(Files.newInputStream(file));
+ ZipInputStream zip = new ZipInputStream(Files.newInputStream(file), StandardCharsets.UTF_8);
ZipEntry entry;
while (null != (entry = zip.getNextEntry())) {
if (isClassFile(Paths.get(entry.getName()))) {
diff --git a/src/test/examples/classmerging/ConflictingInterfaceSignaturesTest.java b/src/test/examples/classmerging/ConflictingInterfaceSignaturesTest.java
new file mode 100644
index 0000000..223ff37
--- /dev/null
+++ b/src/test/examples/classmerging/ConflictingInterfaceSignaturesTest.java
@@ -0,0 +1,32 @@
+// Copyright (c) 2018, 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 classmerging;
+
+public class ConflictingInterfaceSignaturesTest {
+
+ public static void main(String[] args) {
+ A a = new InterfaceImpl();
+ a.foo();
+
+ B b = new InterfaceImpl();
+ b.foo();
+ }
+
+ public interface A {
+ void foo();
+ }
+
+ public interface B {
+ void foo();
+ }
+
+ public static final class InterfaceImpl implements A, B {
+
+ @Override
+ public void foo() {
+ System.out.println("In foo on InterfaceImpl");
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/ExceptionTest.java b/src/test/examples/classmerging/ExceptionTest.java
index b365b2a..28442f3 100644
--- a/src/test/examples/classmerging/ExceptionTest.java
+++ b/src/test/examples/classmerging/ExceptionTest.java
@@ -8,12 +8,27 @@
public static void main(String[] args) {
// The following will lead to a catch handler for ExceptionA, which is merged into ExceptionB.
try {
- throw new ExceptionB("Ouch!");
+ doSomethingThatMightThrowExceptionB();
+ doSomethingThatMightThrowException2();
+ } catch (ExceptionB exception) {
+ System.out.println("Caught exception: " + exception.getMessage());
} catch (ExceptionA exception) {
System.out.println("Caught exception: " + exception.getMessage());
+ } catch (Exception2 exception) {
+ System.out.println("Caught exception: " + exception.getMessage());
+ } catch (Exception1 exception) {
+ System.out.println("Caught exception: " + exception.getMessage());
}
}
+ private static void doSomethingThatMightThrowExceptionB() throws ExceptionB {
+ throw new ExceptionB("Ouch!");
+ }
+
+ private static void doSomethingThatMightThrowException2() throws Exception2 {
+ throw new Exception2("Ouch!");
+ }
+
// Will be merged into ExceptionB when class merging is enabled.
public static class ExceptionA extends Exception {
public ExceptionA(String message) {
@@ -26,4 +41,16 @@
super(message);
}
}
+
+ public static class Exception1 extends Exception {
+ public Exception1(String message) {
+ super(message);
+ }
+ }
+
+ public static class Exception2 extends Exception1 {
+ public Exception2(String message) {
+ super(message);
+ }
+ }
}
diff --git a/src/test/examples/classmerging/SimpleInterface.java b/src/test/examples/classmerging/SimpleInterface.java
new file mode 100644
index 0000000..e8ebcab
--- /dev/null
+++ b/src/test/examples/classmerging/SimpleInterface.java
@@ -0,0 +1,9 @@
+// Copyright (c) 2018, 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 classmerging;
+
+public interface SimpleInterface {
+ void foo();
+}
diff --git a/src/test/examples/classmerging/SimpleInterfaceAccessTest.java b/src/test/examples/classmerging/SimpleInterfaceAccessTest.java
new file mode 100644
index 0000000..04b5386
--- /dev/null
+++ b/src/test/examples/classmerging/SimpleInterfaceAccessTest.java
@@ -0,0 +1,16 @@
+// Copyright (c) 2018, 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 classmerging;
+
+import classmerging.pkg.SimpleInterfaceImplRetriever;
+
+public class SimpleInterfaceAccessTest {
+ public static void main(String[] args) {
+ // It is not possible to merge the interface SimpleInterface into SimpleInterfaceImpl, since
+ // this would lead to an illegal class access here.
+ SimpleInterface obj = SimpleInterfaceImplRetriever.getSimpleInterfaceImpl();
+ obj.foo();
+ }
+}
diff --git a/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java b/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
index c12804c..c09b11a 100644
--- a/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
+++ b/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
@@ -7,6 +7,9 @@
@Override
public String referencedMethod() {
- return "From sub: " + super.referencedMethod();
+ System.out.println("In referencedMethod on SubClassThatReferencesSuperMethod");
+ System.out.println("Calling referencedMethod on SuperClassWithReferencedMethod with super");
+ System.out.println("Got: " + super.referencedMethod());
+ return "SubClassThatReferencesSuperMethod.referencedMethod()";
}
}
diff --git a/src/test/examples/classmerging/SuperCallRewritingTest.java b/src/test/examples/classmerging/SuperCallRewritingTest.java
new file mode 100644
index 0000000..7a3d45c
--- /dev/null
+++ b/src/test/examples/classmerging/SuperCallRewritingTest.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, 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 classmerging;
+
+public class SuperCallRewritingTest {
+ public static void main(String[] args) {
+ System.out.println("Calling referencedMethod on SubClassThatReferencesSuperMethod");
+ System.out.println(new SubClassThatReferencesSuperMethod().referencedMethod());
+ }
+}
diff --git a/src/test/examples/classmerging/SuperClassWithReferencedMethod.java b/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
index 8d4e7b5..3bd5886c84 100644
--- a/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
+++ b/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
@@ -6,6 +6,7 @@
public class SuperClassWithReferencedMethod {
public String referencedMethod() {
- return "From Super";
+ System.out.println("In referencedMethod on SuperClassWithReferencedMethod");
+ return "SuperClassWithReferencedMethod.referencedMethod()";
}
}
diff --git a/src/test/examples/classmerging/TemplateMethodTest.java b/src/test/examples/classmerging/TemplateMethodTest.java
new file mode 100644
index 0000000..ac9de15
--- /dev/null
+++ b/src/test/examples/classmerging/TemplateMethodTest.java
@@ -0,0 +1,31 @@
+// Copyright (c) 2018, 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 classmerging;
+
+public class TemplateMethodTest {
+
+ public static void main(String[] args) {
+ AbstractClass obj = new AbstractClassImpl();
+ obj.foo();
+ }
+
+ private abstract static class AbstractClass {
+
+ public void foo() {
+ System.out.println("In foo on AbstractClass");
+ bar();
+ }
+
+ protected abstract void bar();
+ }
+
+ public static final class AbstractClassImpl extends AbstractClass {
+
+ @Override
+ public void bar() {
+ System.out.println("In bar on AbstractClassImpl");
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index f9e6d52..01e35f3 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -7,9 +7,21 @@
-keep public class classmerging.Test {
public static void main(...);
}
+-keep public class classmerging.ConflictingInterfaceSignaturesTest {
+ public static void main(...);
+}
-keep public class classmerging.ExceptionTest {
public static void main(...);
}
+-keep public class classmerging.SimpleInterfaceAccessTest {
+ public static void main(...);
+}
+-keep public class classmerging.SuperCallRewritingTest {
+ public static void main(...);
+}
+-keep public class classmerging.TemplateMethodTest {
+ public static void main(...);
+}
# TODO(herhut): Consider supporting merging of inner-class attributes.
# -keepattributes *
\ No newline at end of file
diff --git a/src/test/examples/classmerging/pkg/SimpleInterfaceImplRetriever.java b/src/test/examples/classmerging/pkg/SimpleInterfaceImplRetriever.java
new file mode 100644
index 0000000..5cfa11e
--- /dev/null
+++ b/src/test/examples/classmerging/pkg/SimpleInterfaceImplRetriever.java
@@ -0,0 +1,25 @@
+// Copyright (c) 2018, 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 classmerging.pkg;
+
+import classmerging.SimpleInterface;
+
+public class SimpleInterfaceImplRetriever {
+
+ public static SimpleInterface getSimpleInterfaceImpl() {
+ return new SimpleInterfaceImpl();
+ }
+
+ // This class is intentionally marked private. It is not possible to merge the interface
+ // SimpleInterface into SimpleInterfaceImpl, since this would lead to an illegal class access
+ // in SimpleInterfaceAccessTest.
+ private static class SimpleInterfaceImpl implements SimpleInterface {
+
+ @Override
+ public void foo() {
+ System.out.println("In foo on SimpleInterfaceImpl");
+ }
+ }
+}
diff --git a/src/test/examples/uninitializedfinal/UninitializedFinalFieldLeak.java b/src/test/examples/uninitializedfinal/UninitializedFinalFieldLeak.java
new file mode 100644
index 0000000..781d2b4
--- /dev/null
+++ b/src/test/examples/uninitializedfinal/UninitializedFinalFieldLeak.java
@@ -0,0 +1,57 @@
+// Copyright (c) 2018 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 uninitializedfinal;
+
+// Test that leaks an instance before its final field has been initialized to a thread that
+// reads that field. This tests that redundant field load elimination does not eliminate
+// field reads (even of final fields) that cross a monitor operation.
+public class UninitializedFinalFieldLeak {
+
+ public static class PollingThread extends Thread {
+ public int result = 0;
+ UninitializedFinalFieldLeak f;
+
+ PollingThread(UninitializedFinalFieldLeak f) {
+ this.f = f;
+ }
+
+ // Read the field a number of times. Then lock on the object to await field initialization.
+ public void run() {
+ result += f.i;
+ result += f.i;
+ result += f.i;
+ f.threadReadsDone = true;
+ synchronized (f) {
+ result += f.i;
+ }
+ // The right result is 42. Reading the uninitialized 0 three times and then
+ // reading the initialized value. It is safe to remove the two redundant loads
+ // before the monitor operation.
+ System.out.println(result);
+ }
+ }
+
+ public final int i;
+ public volatile boolean threadReadsDone = false;
+
+ public UninitializedFinalFieldLeak() throws InterruptedException {
+ // Leak the object to a thread and start the thread with the lock on the object taken.
+ // Then allow the other thread to run and read the uninitialized field.
+ // Finally, initialize the field and release the lock.
+ PollingThread t = new PollingThread(this);
+ synchronized (this) {
+ t.start();
+ while (!threadReadsDone) {
+ Thread.yield();
+ }
+ i = 42;
+ }
+ t.join();
+ }
+
+ public static void main(String[] args) throws InterruptedException {
+ new UninitializedFinalFieldLeak();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/D8CommandTest.java b/src/test/java/com/android/tools/r8/D8CommandTest.java
index 0c278f7..87453d0 100644
--- a/src/test/java/com/android/tools/r8/D8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/D8CommandTest.java
@@ -23,6 +23,7 @@
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -342,7 +343,7 @@
Path input = Paths.get(EXAMPLES_BUILD_DIR, "arithmetic.jar");
ProgramResourceProvider myProvider =
ArchiveProgramResourceProvider.fromSupplier(
- new MyOrigin(), () -> new ZipFile(input.toFile()));
+ new MyOrigin(), () -> new ZipFile(input.toFile(), StandardCharsets.UTF_8));
D8Command command =
D8Command.builder()
.setProgramConsumer(DexIndexedConsumer.emptyConsumer())
@@ -487,7 +488,7 @@
.setOutput(emptyZip, OutputMode.DexIndexed)
.build());
assertTrue(Files.exists(emptyZip));
- assertEquals(0, new ZipFile(emptyZip.toFile()).size());
+ assertEquals(0, new ZipFile(emptyZip.toFile(), StandardCharsets.UTF_8).size());
}
private D8Command parse(String... args) throws CompilationFailedException {
diff --git a/src/test/java/com/android/tools/r8/R8CommandTest.java b/src/test/java/com/android/tools/r8/R8CommandTest.java
index fbd88be..16bbae9 100644
--- a/src/test/java/com/android/tools/r8/R8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/R8CommandTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -459,7 +460,7 @@
.setOutput(emptyZip, OutputMode.DexIndexed)
.build());
assertTrue(Files.exists(emptyZip));
- assertEquals(0, new ZipFile(emptyZip.toFile()).size());
+ assertEquals(0, new ZipFile(emptyZip.toFile(), StandardCharsets.UTF_8).size());
}
private R8Command parse(String... args) throws CompilationFailedException {
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
index 087012c..36d924f 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
@@ -45,9 +45,11 @@
"instancevariable.InstanceVariable",
"instanceofstring.InstanceofString",
"invoke.Invoke",
+ "invokeempty.InvokeEmpty",
"jumbostring.JumboString",
"loadconst.LoadConst",
"loop.UdpServer",
+ "nestedtrycatches.NestedTryCatches",
"newarray.NewArray",
"regalloc.RegAlloc",
"returns.Returns",
@@ -58,9 +60,7 @@
"throwing.Throwing",
"trivial.Trivial",
"trycatch.TryCatch",
- "nestedtrycatches.NestedTryCatches",
"trycatchmany.TryCatchMany",
- "invokeempty.InvokeEmpty",
"regress.Regress",
"regress2.Regress2",
"regress_37726195.Regress",
@@ -81,6 +81,7 @@
"enclosingmethod_proguarded.Main",
"interfaceinlining.Main",
"switchmaps.Switches",
+ "uninitializedfinal.UninitializedFinalFieldLeak",
};
List<String[]> fullTestList = new ArrayList<>(tests.length * 2);
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
index 2c6a72a..b0876c9 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidOTest.java
@@ -27,6 +27,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -580,7 +581,7 @@
protected DexInspector getMainDexInspector(Path zip)
throws ZipException, IOException, ExecutionException {
- try (ZipFile zipFile = new ZipFile(zip.toFile())) {
+ try (ZipFile zipFile = new ZipFile(zip.toFile(), StandardCharsets.UTF_8)) {
try (InputStream in =
zipFile.getInputStream(zipFile.getEntry(ToolHelper.DEFAULT_DEX_FILENAME))) {
return new DexInspector(
diff --git a/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java b/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
index 182fe80..4701ac3 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesAndroidPTest.java
@@ -25,6 +25,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -256,7 +257,7 @@
protected DexInspector getMainDexInspector(Path zip)
throws ZipException, IOException, ExecutionException {
- try (ZipFile zipFile = new ZipFile(zip.toFile())) {
+ try (ZipFile zipFile = new ZipFile(zip.toFile(), StandardCharsets.UTF_8)) {
try (InputStream in =
zipFile.getInputStream(zipFile.getEntry(ToolHelper.DEFAULT_DEX_FILENAME))) {
return new DexInspector(
diff --git a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
index 284ed68..9560c1f 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
@@ -25,6 +25,7 @@
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -309,7 +310,7 @@
protected DexInspector getMainDexInspector(Path zip)
throws ZipException, IOException, ExecutionException {
- try (ZipFile zipFile = new ZipFile(zip.toFile())) {
+ try (ZipFile zipFile = new ZipFile(zip.toFile(), StandardCharsets.UTF_8)) {
try (InputStream in =
zipFile.getInputStream(zipFile.getEntry(ToolHelper.DEFAULT_DEX_FILENAME))) {
return new DexInspector(
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/EmptyBridgeTest.java b/src/test/java/com/android/tools/r8/bridgeremoval/EmptyBridgeTest.java
new file mode 100644
index 0000000..160ad59
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/EmptyBridgeTest.java
@@ -0,0 +1,63 @@
+// Copyright (c) 2018, 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.bridgeremoval;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class EmptyBridgeTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+
+ ClassBuilder abs = jasminBuilder.addClass("Abs");
+ abs.setAccess("public abstract");
+ abs.addMethod("public bridge abstract", "emptyBridge", ImmutableList.of(), "V");
+
+ ClassBuilder cls = jasminBuilder.addClass("Main");
+ cls.addVirtualMethod("bogus", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"foo\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+ cls.addMainMethod(
+ ".limit stack 4",
+ ".limit locals 2",
+ "new " + cls.name,
+ "dup",
+ "invokespecial " + cls.name + "/<init>()V",
+ "astore_0",
+ "aload_0",
+ "invokevirtual " + cls.name + "/bogus()V",
+ "return");
+
+ String absClassName = abs.name;
+ String mainClassName = cls.name;
+ String proguardConfig =
+ "-allowaccessmodification" + System.lineSeparator()
+ + "-keep class " + mainClassName + "{ *; }" + System.lineSeparator()
+ + "-keep class " + absClassName + "{ *; }";
+ AndroidApp processedApp = compileWithR8(jasminBuilder.build(), proguardConfig);
+
+ DexInspector inspector = new DexInspector(processedApp);
+ ClassSubject classSubject = inspector.clazz(absClassName);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method("void", "emptyBridge", ImmutableList.of());
+ assertThat(methodSubject, isPresent());
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 69f925c..52e2313 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -3,8 +3,10 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.classmerging;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.CompilationFailedException;
@@ -12,21 +14,33 @@
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.code.MoveException;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.smali.SmaliBuilder;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.FoundClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
+import org.junit.Ignore;
import org.junit.Test;
+// TODO(christofferqa): Add tests to check that statically typed invocations on method handles
+// continue to work after class merging. Rewriting of method handles should be carried out by
+// LensCodeRewriter.rewriteDexMethodHandle.
public class ClassMergingTest extends TestBase {
private static final Path CF_DIR =
@@ -96,9 +110,94 @@
@Test
public void testSuperCallWasDetected() throws Exception {
- runR8(EXAMPLE_KEEP, this::configure);
- assertTrue(inspector.clazz("classmerging.SuperClassWithReferencedMethod").isPresent());
- assertTrue(inspector.clazz("classmerging.SubClassThatReferencesSuperMethod").isPresent());
+ String main = "classmerging.SuperCallRewritingTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SubClassThatReferencesSuperMethod.class"),
+ CF_DIR.resolve("SuperClassWithReferencedMethod.class"),
+ CF_DIR.resolve("SuperCallRewritingTest.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperCallRewritingTest");
+ runTest(main, programFiles, preservedClassNames);
+ }
+
+ // When a subclass A has been merged into its subclass B, we rewrite invoke-super calls that hit
+ // methods in A to invoke-direct calls. However, we should be careful not to transform invoke-
+ // super instructions into invoke-direct instructions simply because the static target is a method
+ // in the enclosing class.
+ //
+ // This test hand-crafts an invoke-super instruction in SubClassThatReferencesSuperMethod that
+ // targets SubClassThatReferencesSuperMethod.referencedMethod. When running without class
+ // merging, R8 should not rewrite the invoke-super instruction into invoke-direct.
+ @Test
+ public void testSuperCallNotRewrittenToDirect() throws Exception {
+ String main = "classmerging.SuperCallRewritingTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SuperClassWithReferencedMethod.class"),
+ CF_DIR.resolve("SuperCallRewritingTest.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperClassWithReferencedMethod",
+ "classmerging.SuperCallRewritingTest");
+
+ // Build SubClassThatReferencesMethod.
+ SmaliBuilder smaliBuilder =
+ new SmaliBuilder(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperClassWithReferencedMethod");
+ smaliBuilder.addInitializer(
+ ImmutableList.of(),
+ 0,
+ "invoke-direct {p0}, Lclassmerging/SuperClassWithReferencedMethod;-><init>()V",
+ "return-void");
+ smaliBuilder.addInstanceMethod(
+ "java.lang.String",
+ "referencedMethod",
+ ImmutableList.of(),
+ 2,
+ "sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream;",
+ "const-string v1, \"In referencedMethod on SubClassThatReferencesSuperMethod\"",
+ "invoke-virtual {v0, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V",
+ "invoke-super {p0}, Lclassmerging/SubClassThatReferencesSuperMethod;->referencedMethod()Ljava/lang/String;",
+ "move-result-object v1",
+ "return-object v1");
+
+ // Build app.
+ AndroidApp.Builder builder = AndroidApp.builder();
+ builder.addProgramFiles(programFiles);
+ builder.addDexProgramData(smaliBuilder.compile(), Origin.unknown());
+
+ // Run test.
+ runTestOnInput(
+ main,
+ builder.build(),
+ preservedClassNames,
+ // Prevent class merging, such that the generated code would be invalid if we rewrite the
+ // invoke-super instruction into an invoke-direct instruction.
+ getProguardConfig(EXAMPLE_KEEP, "-keep class *"));
+ }
+
+ @Test
+ public void testConflictingInterfaceSignatures() throws Exception {
+ String main = "classmerging.ConflictingInterfaceSignaturesTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("ConflictingInterfaceSignaturesTest.class"),
+ CF_DIR.resolve("ConflictingInterfaceSignaturesTest$A.class"),
+ CF_DIR.resolve("ConflictingInterfaceSignaturesTest$B.class"),
+ CF_DIR.resolve("ConflictingInterfaceSignaturesTest$InterfaceImpl.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.ConflictingInterfaceSignaturesTest",
+ "classmerging.ConflictingInterfaceSignaturesTest$InterfaceImpl");
+ runTest(main, programFiles, preservedClassNames);
}
// If an exception class A is merged into another exception class B, then all exception tables
@@ -110,17 +209,100 @@
new Path[] {
CF_DIR.resolve("ExceptionTest.class"),
CF_DIR.resolve("ExceptionTest$ExceptionA.class"),
- CF_DIR.resolve("ExceptionTest$ExceptionB.class")
+ CF_DIR.resolve("ExceptionTest$ExceptionB.class"),
+ CF_DIR.resolve("ExceptionTest$Exception1.class"),
+ CF_DIR.resolve("ExceptionTest$Exception2.class")
};
Set<String> preservedClassNames =
- ImmutableSet.of("classmerging.ExceptionTest", "classmerging.ExceptionTest$ExceptionB");
+ ImmutableSet.of(
+ "classmerging.ExceptionTest",
+ "classmerging.ExceptionTest$ExceptionB",
+ "classmerging.ExceptionTest$Exception2");
+ DexInspector inspector = runTest(main, programFiles, preservedClassNames);
+
+ ClassSubject mainClass = inspector.clazz(main);
+ assertThat(mainClass, isPresent());
+
+ MethodSubject mainMethod =
+ mainClass.method("void", "main", ImmutableList.of("java.lang.String[]"));
+ assertThat(mainMethod, isPresent());
+
+ // Check that the second catch handler has been removed.
+ DexCode code = mainMethod.getMethod().getCode().asDexCode();
+ int numberOfMoveExceptionInstructions = 0;
+ for (Instruction instruction : code.instructions) {
+ if (instruction instanceof MoveException) {
+ numberOfMoveExceptionInstructions++;
+ }
+ }
+ assertEquals(2, numberOfMoveExceptionInstructions);
+ }
+
+ @Ignore("b/73958515")
+ @Test
+ public void testNoIllegalClassAccess() throws Exception {
+ String main = "classmerging.SimpleInterfaceAccessTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SimpleInterface.class"),
+ CF_DIR.resolve("SimpleInterfaceAccessTest.class"),
+ CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever.class"),
+ CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever$SimpleInterfaceImpl.class")
+ };
+ // SimpleInterface cannot be merged into SimpleInterfaceImpl because SimpleInterfaceImpl
+ // is in a different package and is not public.
+ ImmutableSet<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SimpleInterface",
+ "classmerging.SimpleInterfaceAccessTest",
+ "classmerging.pkg.SimpleInterfaceImplRetriever",
+ "classmerging.pkg.SimpleInterfaceImplRetriever$SimpleInterfaceImpl");
+ runTest(main, programFiles, preservedClassNames);
+ // If access modifications are allowed then SimpleInterface should be merged into
+ // SimpleInterfaceImpl.
+ preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SimpleInterfaceAccessTest",
+ "classmerging.pkg.SimpleInterfaceImplRetriever",
+ "classmerging.pkg.SimpleInterfaceImplRetriever$SimpleInterfaceImpl");
+ runTest(
+ main,
+ programFiles,
+ preservedClassNames,
+ getProguardConfig(EXAMPLE_KEEP, "-allowaccessmodification"));
+ }
+
+ @Test
+ public void testTemplateMethodPattern() throws Exception {
+ String main = "classmerging.TemplateMethodTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("TemplateMethodTest.class"),
+ CF_DIR.resolve("TemplateMethodTest$AbstractClass.class"),
+ CF_DIR.resolve("TemplateMethodTest$AbstractClassImpl.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.TemplateMethodTest", "classmerging.TemplateMethodTest$AbstractClassImpl");
runTest(main, programFiles, preservedClassNames);
}
- private void runTest(String main, Path[] programFiles, Set<String> preservedClassNames)
+ private DexInspector runTest(String main, Path[] programFiles, Set<String> preservedClassNames)
throws Exception {
- AndroidApp input = readProgramFiles(programFiles);
- AndroidApp output = compileWithR8(input, EXAMPLE_KEEP, this::configure);
+ return runTest(main, programFiles, preservedClassNames, getProguardConfig(EXAMPLE_KEEP));
+ }
+
+ private DexInspector runTest(
+ String main, Path[] programFiles, Set<String> preservedClassNames, String proguardConfig)
+ throws Exception {
+ return runTestOnInput(
+ main, readProgramFiles(programFiles), preservedClassNames, proguardConfig);
+ }
+
+ private DexInspector runTestOnInput(
+ String main, AndroidApp input, Set<String> preservedClassNames, String proguardConfig)
+ throws Exception {
+ AndroidApp output = compileWithR8(input, proguardConfig, this::configure);
DexInspector inspector = new DexInspector(output);
// Check that all classes in [preservedClassNames] are in fact preserved.
for (String className : preservedClassNames) {
@@ -135,5 +317,18 @@
}
// Check that the R8-generated code produces the same result as D8-generated code.
assertEquals(runOnArt(compileWithD8(input), main), runOnArt(output, main));
+ return inspector;
+ }
+
+ private String getProguardConfig(Path path, String... additionalRules) throws IOException {
+ StringBuilder builder = new StringBuilder();
+ for (String line : Files.readAllLines(path)) {
+ builder.append(line);
+ builder.append(System.lineSeparator());
+ }
+ for (String rule : additionalRules) {
+ builder.append(rule);
+ }
+ return builder.toString();
}
}
diff --git a/src/test/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilderTests.java b/src/test/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilderTests.java
index e49c740..341fb67 100644
--- a/src/test/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilderTests.java
+++ b/src/test/java/com/android/tools/r8/compatdexbuilder/CompatDexBuilderTests.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Enumeration;
import java.util.HashSet;
@@ -65,7 +66,7 @@
for (String className : CLASS_NAMES) {
expectedNames.add(SUBDIR + "/" + className + ".class.dex");
}
- try (ZipFile zipFile = new ZipFile(outputZip.toFile())) {
+ try (ZipFile zipFile = new ZipFile(outputZip.toFile(), StandardCharsets.UTF_8)) {
for (Enumeration<? extends ZipEntry> e = zipFile.entries(); e.hasMoreElements(); ) {
ZipEntry ze = e.nextElement();
expectedNames.remove(ze.getName());
diff --git a/src/test/java/com/android/tools/r8/compatdx/CompatDxTests.java b/src/test/java/com/android/tools/r8/compatdx/CompatDxTests.java
index c55428f..7d181bc 100644
--- a/src/test/java/com/android/tools/r8/compatdx/CompatDxTests.java
+++ b/src/test/java/com/android/tools/r8/compatdx/CompatDxTests.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.net.URI;
+import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
@@ -197,8 +198,8 @@
}
private void compareArchiveFiles(Path d8File, Path dxFile) throws IOException {
- ZipFile d8Zip = new ZipFile(d8File.toFile());
- ZipFile dxZip = new ZipFile(dxFile.toFile());
+ ZipFile d8Zip = new ZipFile(d8File.toFile(), StandardCharsets.UTF_8);
+ ZipFile dxZip = new ZipFile(dxFile.toFile(), StandardCharsets.UTF_8);
// TODO(zerny): This should test resource containment too once supported.
Set<String> d8Content = d8Zip.stream().map(ZipEntry::getName).collect(Collectors.toSet());
Set<String> dxContent = dxZip.stream().map(ZipEntry::getName).collect(Collectors.toSet());
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
index eb6e943..ba04454 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
@@ -195,7 +195,7 @@
return addStaticMethod("main", ImmutableList.of("[Ljava/lang/String;"), "V", lines);
}
- private MethodSignature addMethod(
+ public MethodSignature addMethod(
String access,
String name,
List<String> argumentTypes,
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
index d6e3612..4214a67 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
@@ -38,7 +38,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.DirectMappedDexApplication;
-import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.ir.code.CatchHandlers;
@@ -653,7 +652,7 @@
code);
IRCode ir = code.buildIR(method, null, options, Origin.unknown());
RegisterAllocator allocator = new LinearScanRegisterAllocator(ir, options);
- method.setCode(ir, GraphLense.getIdentityLense(), allocator, options);
+ method.setCode(ir, allocator, options);
directMethods[i] = method;
}
builder.addProgramClass(
diff --git a/src/test/java/com/android/tools/r8/naming/GenericSignatureParserTest.java b/src/test/java/com/android/tools/r8/naming/GenericSignatureParserTest.java
index 1d1604b..957bc1d 100644
--- a/src/test/java/com/android/tools/r8/naming/GenericSignatureParserTest.java
+++ b/src/test/java/com/android/tools/r8/naming/GenericSignatureParserTest.java
@@ -350,7 +350,8 @@
forMethodSignatures(this::parseMethodSignature);
}
- private void failingParseAction(Consumer<GenericSignatureParser<String>> parse)
+ private void failingParseAction(
+ Consumer<GenericSignatureParser<String>> parse, String errorMessageType)
throws Exception {
class ThrowsInParserActionBase<E extends Error> extends ReGenerateGenericSignatureRewriter {
protected Supplier<? extends E> exceptionSupplier;
@@ -452,7 +453,8 @@
assertEquals("ERROR", e.getMessage());
} else {
plainErrorCount++;
- assertEquals("Unknown error parsing generic signature: ERROR", e.getMessage());
+ assertEquals("Unknown error parsing "
+ + errorMessageType + " signature: ERROR", e.getMessage());
}
}
}
@@ -463,10 +465,10 @@
public void failingParseAction() throws Exception {
// These signatures hits all action callbacks.
failingParseAction(parser -> parser.parseClassSignature(
- "<U:Ljava/lang/Object;>LOuter<TT;>.Inner;Ljava/util/List<TU;>;"));
+ "<U:Ljava/lang/Object;>LOuter<TT;>.Inner;Ljava/util/List<TU;>;"), "class");
failingParseAction(
- parser -> parser.parseFieldSignature("LOuter$InnerInterface<TU;>.Inner;"));
+ parser -> parser.parseFieldSignature("LOuter$InnerInterface<TU;>.Inner;"), "field");
failingParseAction(
- parser -> parser.parseMethodSignature("(LOuter$InnerInterface<TU;>.Inner;)V"));
+ parser -> parser.parseMethodSignature("(LOuter$InnerInterface<TU;>.Inner;)V"), "method");
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
index 68cd36f..b9ff7ea 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
@@ -424,7 +424,7 @@
@Test
public void classSignatureOuter_classNotFound() throws Exception {
- String signature = "<T:LNotFound;>LNotFound;";
+ String signature = "<T:LNotFound;>LAlsoNotFound;";
testSingleClass("Outer", signature, this::noWarnings, inspector -> {
assertThat(inspector.clazz("NotFound"), not(isPresent()));
ClassSubject outer = inspector.clazz("Outer");
@@ -433,11 +433,51 @@
}
@Test
+ public void classSignatureExtendsInner_innerClassNotFound() throws Exception {
+ String signature = "LOuter<TT;>.NotFound;";
+ testSingleClass("Outer$ExtendsInner", signature, this::noWarnings, inspector -> {
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ ClassSubject outer = inspector.clazz("Outer$ExtendsInner");
+ assertEquals(signature, outer.getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
+ public void classSignatureExtendsInner_outerAndInnerClassNotFound() throws Exception {
+ String signature = "LNotFound<TT;>.AlsoNotFound;";
+ testSingleClass("Outer$ExtendsInner", signature, this::noWarnings, inspector -> {
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ ClassSubject outer = inspector.clazz("Outer$ExtendsInner");
+ assertEquals(signature, outer.getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
+ public void classSignatureExtendsInner_nestedInnerClassNotFound() throws Exception {
+ String signature = "LOuter<TT;>.Inner.NotFound;";
+ testSingleClass("Outer$ExtendsInner", signature, this::noWarnings, inspector -> {
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ ClassSubject outer = inspector.clazz("Outer$ExtendsInner");
+ assertEquals(signature, outer.getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
+ public void classSignatureExtendsInner_multipleMestedInnerClassesNotFound() throws Exception {
+ String signature = "LOuter<TT;>.NotFound.AlsoNotFound;";
+ testSingleClass("Outer$ExtendsInner", signature, this::noWarnings, inspector -> {
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ ClassSubject outer = inspector.clazz("Outer$ExtendsInner");
+ assertEquals(signature, outer.getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
public void classSignatureOuter_invalid() throws Exception {
testSingleClass("Outer", "X", diagnostics -> {
assertEquals(1, diagnostics.warnings.size());
DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
- "Invalid class signature for class Outer", "Expected L at position 1");
+ "Invalid signature for class Outer", "Expected L at position 1");
}, inspector -> noSignatureAttribute(inspector.clazz("Outer")));
}
@@ -446,7 +486,7 @@
testSingleClass("Outer", "<L", diagnostics -> {
assertEquals(1, diagnostics.warnings.size());
DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
- "Invalid class signature for class Outer", "Unexpected end of signature at position 3");
+ "Invalid signature for class Outer", "Unexpected end of signature at position 3");
}, inspector -> noSignatureAttribute(inspector.clazz("Outer")));
}
@@ -455,7 +495,7 @@
testSingleClass("Outer$ExtendsInner", "X", diagnostics -> {
assertEquals(1, diagnostics.warnings.size());
DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
- "Invalid class signature for class Outer$ExtendsInner", "Expected L at position 1");
+ "Invalid signature for class Outer$ExtendsInner", "Expected L at position 1");
}, inspector -> noSignatureAttribute(inspector.clazz("Outer$ExtendsInner")));
}
@@ -464,7 +504,7 @@
testSingleClass("Outer$Inner$ExtendsInnerInner", "X", diagnostics -> {
assertEquals(1, diagnostics.warnings.size());
DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
- "Invalid class signature for class Outer$Inner$ExtendsInnerInner",
+ "Invalid signature for class Outer$Inner$ExtendsInnerInner",
"Expected L at position 1");
}, inspector -> noSignatureAttribute(inspector.clazz("Outer$Inner$ExtendsInnerInner")));
}
@@ -488,7 +528,7 @@
testSingleClass("Outer$ExtendsInner", signature, diagnostics -> {
assertEquals(1, diagnostics.warnings.size());
DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
- "Invalid class signature for class Outer$ExtendsInner", "Expected ; at position 16");
+ "Invalid signature for class Outer$ExtendsInner", "Expected ; at position 16");
}, inspector -> {
noSignatureAttribute(inspector.clazz("Outer$ExtendsInner"));
});
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java
new file mode 100644
index 0000000..e48ce56
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java
@@ -0,0 +1,289 @@
+// Copyright (c) 2018, 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.naming;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isRenamed;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_SUPER;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.PUTFIELD;
+import static org.objectweb.asm.Opcodes.RETURN;
+import static org.objectweb.asm.Opcodes.V1_8;
+
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.DiagnosticsChecker;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.StringConsumer;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.graph.invokesuper.Consumer;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.FieldSubject;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.MethodVisitor;
+
+
+public class MinifierFieldSignatureTest extends TestBase {
+ /*
+
+ class Fields<X extends String> {
+ class Inner {
+ }
+ public X anX;
+ public X[] anArrayOfX;
+ public Fields<X> aFieldsOfX;
+ public Fields<X>.Inner aFieldsOfXInner;
+ }
+
+ */
+
+ private String anXSignature = "TX;";
+ private String anArrayOfXSignature = "[TX;";
+ private String aFieldsOfXSignature = "LFields<TX;>;";
+ private String aFieldsOfXInnerSignature = "LFields<TX;>.Inner;";
+
+ public byte[] dumpFields(Map<String, String> signatures) throws Exception {
+
+ ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
+ MethodVisitor mv;
+ String signature;
+
+ cw.visit(V1_8, ACC_SUPER, "Fields", "<X:Ljava/lang/String;>Ljava/lang/Object;",
+ "java/lang/Object", null);
+
+ cw.visitInnerClass("Fields$Inner", "Fields", "Inner", 0);
+
+ {
+ signature = signatures.get("anX");
+ signature = signature == null ? anXSignature : signature;
+ fv = cw.visitField(ACC_PUBLIC, "anX", "Ljava/lang/String;", signature, null);
+ fv.visitEnd();
+ }
+ {
+ signature = signatures.get("anArrayOfX");
+ signature = signature == null ? anArrayOfXSignature : signature;
+ fv = cw.visitField(
+ ACC_PUBLIC, "anArrayOfX", "[Ljava/lang/String;", signature, null);
+ fv.visitEnd();
+ }
+ {
+ signature = signatures.get("aFieldsOfX");
+ signature = signature == null ? aFieldsOfXSignature : signature;
+ fv = cw.visitField(ACC_PUBLIC, "aFieldsOfX", "LFields;", signature, null);
+ fv.visitEnd();
+ }
+ {
+ signature = signatures.get("aFieldsOfXInner");
+ signature = signature == null ? aFieldsOfXInnerSignature : signature;
+ fv = cw.visitField(
+ ACC_PUBLIC, "aFieldsOfXInner", "LFields$Inner;", signature, null);
+ fv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(0, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+
+ public byte[] dumpInner() throws Exception {
+
+ ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
+ MethodVisitor mv;
+
+ cw.visit(V1_8, ACC_SUPER, "Fields$Inner", null, "java/lang/Object", null);
+
+ cw.visitInnerClass("Fields$Inner", "Fields", "Inner", 0);
+
+ {
+ fv = cw.visitField(ACC_FINAL + ACC_SYNTHETIC, "this$0", "LFields;", null, null);
+ fv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(0, "<init>", "(LFields;)V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitVarInsn(ALOAD, 1);
+ mv.visitFieldInsn(PUTFIELD, "Fields$Inner", "this$0", "LFields;");
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(2, 2);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+
+ private FieldSubject lookupAnX(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Fields");
+ return clazz.field("java.lang.String", "anX");
+ }
+
+ private FieldSubject lookupAnArrayOfX(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Fields");
+ return clazz.field("java.lang.String[]", "anArrayOfX");
+ }
+
+ private FieldSubject lookupAFieldsOfX(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Fields");
+ return clazz.field("Fields", "aFieldsOfX");
+ }
+
+ public void runTest(
+ ImmutableMap<String, String> signatures,
+ Consumer<DiagnosticsChecker> diagnostics,
+ Consumer<DexInspector> inspect)
+ throws Exception {
+ DiagnosticsChecker checker = new DiagnosticsChecker();
+ DexInspector inspector = new DexInspector(
+ ToolHelper.runR8(R8Command.builder(checker)
+ .addClassProgramData(dumpFields(signatures), Origin.unknown())
+ .addClassProgramData(dumpInner(), Origin.unknown())
+ .addProguardConfiguration(ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod,Signature",
+ "-keep,allowobfuscation class ** { *; }"
+ ), Origin.unknown())
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .setProguardMapConsumer(StringConsumer.emptyConsumer())
+ .build()));
+ // All classes are kept, and renamed.
+ ClassSubject clazz = inspector.clazz("Fields");
+ assertThat(clazz, isRenamed());
+ assertThat(inspector.clazz("Fields$Inner"), isRenamed());
+
+ FieldSubject anX = lookupAnX(inspector);
+ FieldSubject anArrayOfX = lookupAnArrayOfX(inspector);
+ FieldSubject aFieldsOfX =lookupAFieldsOfX(inspector);
+ FieldSubject aFieldsOfXInner = clazz.field("Fields$Inner", "aFieldsOfXInner");
+
+ // Check that all fields have been renamed
+ assertThat(anX, isRenamed());
+ assertThat(anArrayOfX, isRenamed());
+ assertThat(aFieldsOfX, isRenamed());
+ assertThat(aFieldsOfXInner, isRenamed());
+
+ //System.out.println(generic.getFinalSignatureAttribute());
+ //System.out.println(generic.getOriginalSignatureAttribute());
+
+ // Test that methods have their original signature if the default was provided.
+ if (!signatures.containsKey("anX")) {
+ assertEquals(anXSignature, anX.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("anArrayOfX")) {
+ assertEquals(anArrayOfXSignature, anArrayOfX.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("aFieldsOfX")) {
+ assertEquals(
+ aFieldsOfXSignature, aFieldsOfX.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("aFieldsOfXInner")) {
+ assertEquals(
+ aFieldsOfXInnerSignature, aFieldsOfXInner.getOriginalSignatureAttribute());
+ }
+
+ diagnostics.accept(checker);
+ inspect.accept(inspector);
+ }
+
+ private void testSingleField(String name, String signature,
+ Consumer<DiagnosticsChecker> diagnostics,
+ Consumer<DexInspector> inspector)
+ throws Exception {
+ ImmutableMap<String, String> signatures = ImmutableMap.of(name, signature);
+ runTest(signatures, diagnostics, inspector);
+ }
+
+ private void isOriginUnknown(Origin origin) {
+ assertSame(Origin.unknown(), origin);
+ }
+
+ private void noWarnings(DiagnosticsChecker checker) {
+ assertEquals(0, checker.warnings.size());
+ }
+
+ private void noInspection(DexInspector inspector) {
+ }
+
+ private void noSignatureAttribute(FieldSubject field) {
+ assertThat(field, isPresent());
+ assertNull(field.getFinalSignatureAttribute());
+ assertNull(field.getOriginalSignatureAttribute());
+ }
+
+ @Test
+ public void originalJavacSignatures() throws Exception {
+ // Test using the signatures generated by javac.
+ runTest(ImmutableMap.of(), this::noWarnings, this::noInspection);
+ }
+
+ @Test
+ public void signatureEmpty() throws Exception {
+ testSingleField("anX", "", this::noWarnings,
+ inspector -> noSignatureAttribute(lookupAnX(inspector)));
+ }
+
+ @Test
+ public void signatureInvalid() throws Exception {
+ testSingleField("anX", "X", diagnostics -> {
+ assertEquals(1, diagnostics.warnings.size());
+ // TODO(sgjesse): The position 2 reported here is one off.
+ DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
+ "Invalid signature for field",
+ "java.lang.String Fields.anX",
+ "Expected L, [ or T at position 2");
+ }, inspector -> noSignatureAttribute(lookupAnX(inspector)));
+ }
+
+ @Test
+ public void classNotFound() throws Exception {
+ String signature = "LNotFound<TX;>.InnerNotFound.InnerAlsoNotFound;";
+ testSingleField("anX", signature, this::noWarnings, inspector -> {
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ assertEquals(signature, lookupAnX(inspector).getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
+ public void multipleWarnings() throws Exception {
+ runTest(ImmutableMap.of(
+ "anX", "X",
+ "anArrayOfX", "X",
+ "aFieldsOfX", "X"
+ ), diagnostics -> {
+ assertEquals(3, diagnostics.warnings.size());
+ }, inspector -> {
+ noSignatureAttribute(lookupAnX(inspector));
+ noSignatureAttribute(lookupAnArrayOfX(inspector));
+ noSignatureAttribute(lookupAFieldsOfX(inspector));
+ });
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java
new file mode 100644
index 0000000..41750ae
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java
@@ -0,0 +1,314 @@
+// Copyright (c) 2018, 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.naming;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isRenamed;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_SUPER;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
+import static org.objectweb.asm.Opcodes.ACONST_NULL;
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.ARETURN;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.PUTFIELD;
+import static org.objectweb.asm.Opcodes.RETURN;
+import static org.objectweb.asm.Opcodes.V1_8;
+
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.DiagnosticsChecker;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.StringConsumer;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.graph.invokesuper.Consumer;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.MethodVisitor;
+
+public class MinifierMethodSignatureTest extends TestBase {
+ /*
+
+ class Methods<X extends Throwable> {
+ class Inner {
+ }
+ public static <T extends Throwable> T generic(T a, Methods<T>.Inner b) { return null; }
+ public Methods<X>.Inner parameterizedReturn() { return null; }
+ public void parameterizedArguments(X a, Methods<X>.Inner b) { }
+ public void parametrizedThrows() throws X { }
+ }
+
+ */
+
+ private String genericSignature = "<T:Ljava/lang/Throwable;>(TT;LMethods<TT;>.Inner;)TT;";
+ private String parameterizedReturnSignature = "()LMethods<TX;>.Inner;";
+ private String parameterizedArgumentsSignature = "(TX;LMethods<TX;>.Inner;)V";
+ private String parametrizedThrowsSignature = "()V^TX;";
+
+ private byte[] dumpMethods(Map<String, String> signatures) throws Exception {
+
+ ClassWriter cw = new ClassWriter(0);
+ MethodVisitor mv;
+ String signature;
+
+ cw.visit(V1_8, ACC_SUPER, "Methods", "<X:Ljava/lang/Throwable;>Ljava/lang/Object;",
+ "java/lang/Object", null);
+
+ cw.visitInnerClass("Methods$Inner", "Methods", "Inner", 0);
+
+ {
+ mv = cw.visitMethod(0, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ signature = signatures.get("generic");
+ signature = signature == null ? genericSignature : signature;
+ mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "generic",
+ "(Ljava/lang/Throwable;LMethods$Inner;)Ljava/lang/Throwable;",
+ signature, null);
+ mv.visitCode();
+ mv.visitInsn(ACONST_NULL);
+ mv.visitInsn(ARETURN);
+ mv.visitMaxs(1, 2);
+ mv.visitEnd();
+ }
+ {
+ signature = signatures.get("parameterizedReturn");
+ signature = signature == null ? parameterizedReturnSignature : signature;
+ mv = cw.visitMethod(ACC_PUBLIC, "parameterizedReturn", "()LMethods$Inner;",
+ signature, null);
+ mv.visitCode();
+ mv.visitInsn(ACONST_NULL);
+ mv.visitInsn(ARETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ signature = signatures.get("parameterizedArguments");
+ signature = signature == null ? parameterizedArgumentsSignature : signature;
+ mv = cw.visitMethod(ACC_PUBLIC, "parameterizedArguments",
+ "(Ljava/lang/Throwable;LMethods$Inner;)V", signature, null);
+ mv.visitCode();
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(0, 3);
+ mv.visitEnd();
+ }
+ {
+ signature = signatures.get("parametrizedThrows");
+ signature = signature == null ? parametrizedThrowsSignature : signature;
+ mv = cw.visitMethod(ACC_PUBLIC, "parametrizedThrows", "()V", signature,
+ new String[] { "java/lang/Throwable" });
+ mv.visitCode();
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(0, 1);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+
+ private byte[] dumpInner() throws Exception {
+
+ ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
+ MethodVisitor mv;
+
+ cw.visit(V1_8, ACC_SUPER, "Methods$Inner", null, "java/lang/Object", null);
+
+ cw.visitInnerClass("Methods$Inner", "Methods", "Inner", 0);
+
+ {
+ fv = cw.visitField(ACC_FINAL + ACC_SYNTHETIC, "this$0", "LMethods;", null, null);
+ fv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(0, "<init>", "(LMethods;)V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitVarInsn(ALOAD, 1);
+ mv.visitFieldInsn(PUTFIELD, "Methods$Inner", "this$0", "LMethods;");
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(2, 2);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+
+ private MethodSubject lookupGeneric(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Methods");
+ return clazz.method(
+ "java.lang.Throwable", "generic", ImmutableList.of("java.lang.Throwable", "Methods$Inner"));
+ }
+
+ private MethodSubject lookupParameterizedReturn(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Methods");
+ return clazz.method(
+ "Methods$Inner", "parameterizedReturn", ImmutableList.of());
+ }
+
+ private MethodSubject lookupParameterizedArguments(DexInspector inspector) {
+ ClassSubject clazz = inspector.clazz("Methods");
+ return clazz.method(
+ "void", "parameterizedArguments", ImmutableList.of("java.lang.Throwable", "Methods$Inner"));
+ }
+
+ public void runTest(
+ ImmutableMap<String, String> signatures,
+ Consumer<DiagnosticsChecker> diagnostics,
+ Consumer<DexInspector> inspect)
+ throws Exception {
+ DiagnosticsChecker checker = new DiagnosticsChecker();
+ DexInspector inspector = new DexInspector(
+ ToolHelper.runR8(R8Command.builder(checker)
+ .addClassProgramData(dumpMethods(signatures), Origin.unknown())
+ .addClassProgramData(dumpInner(), Origin.unknown())
+ .addProguardConfiguration(ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod,Signature",
+ "-keep,allowobfuscation class ** { *; }"
+ ), Origin.unknown())
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .setProguardMapConsumer(StringConsumer.emptyConsumer())
+ .build()));
+ // All classes are kept, and renamed.
+ ClassSubject clazz = inspector.clazz("Methods");
+ assertThat(clazz, isRenamed());
+ assertThat(inspector.clazz("Methods$Inner"), isRenamed());
+
+ MethodSubject generic = lookupGeneric(inspector);
+ MethodSubject parameterizedReturn = lookupParameterizedReturn(inspector);
+ MethodSubject parameterizedArguments = lookupParameterizedArguments(inspector);
+ MethodSubject parametrizedThrows =
+ clazz.method("void", "parametrizedThrows", ImmutableList.of());
+
+ // Check that all methods have been renamed
+ assertThat(generic, isRenamed());
+ assertThat(parameterizedReturn, isRenamed());
+ assertThat(parameterizedArguments, isRenamed());
+ assertThat(parametrizedThrows, isRenamed());
+
+ // Test that methods have their original signature if the default was provided.
+ if (!signatures.containsKey("generic")) {
+ assertEquals(genericSignature, generic.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("parameterizedReturn")) {
+ assertEquals(
+ parameterizedReturnSignature, parameterizedReturn.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("parameterizedArguments")) {
+ assertEquals(
+ parameterizedArgumentsSignature, parameterizedArguments.getOriginalSignatureAttribute());
+ }
+ if (!signatures.containsKey("parametrizedThrows")) {
+ assertEquals(
+ parametrizedThrowsSignature, parametrizedThrows.getOriginalSignatureAttribute());
+ }
+
+ diagnostics.accept(checker);
+ inspect.accept(inspector);
+ }
+
+ private void testSingleMethod(String name, String signature,
+ Consumer<DiagnosticsChecker> diagnostics,
+ Consumer<DexInspector> inspector)
+ throws Exception {
+ ImmutableMap<String, String> signatures = ImmutableMap.of(name, signature);
+ runTest(signatures, diagnostics, inspector);
+ }
+
+ private void isOriginUnknown(Origin origin) {
+ assertSame(Origin.unknown(), origin);
+ }
+
+ private void noWarnings(DiagnosticsChecker checker) {
+ assertEquals(0, checker.warnings.size());
+ }
+
+ private void noInspection(DexInspector inspector) {
+ }
+
+ private void noSignatureAttribute(MethodSubject method) {
+ assertThat(method, isPresent());
+ assertNull(method.getFinalSignatureAttribute());
+ assertNull(method.getOriginalSignatureAttribute());
+ }
+
+ @Test
+ public void originalJavacSignatures() throws Exception {
+ // Test using the signatures generated by javac.
+ runTest(ImmutableMap.of(), this::noWarnings, this::noInspection);
+ }
+
+ @Test
+ public void signatureEmpty() throws Exception {
+ testSingleMethod("generic", "", this::noWarnings, inspector -> {
+ noSignatureAttribute(lookupGeneric(inspector));
+ });
+ }
+
+ @Test
+ public void signatureInvalid() throws Exception {
+ testSingleMethod("generic", "X", diagnostics -> {
+ assertEquals(1, diagnostics.warnings.size());
+ DiagnosticsChecker.checkDiagnostic(diagnostics.warnings.get(0), this::isOriginUnknown,
+ "Invalid signature for method",
+ "java.lang.Throwable Methods.generic(java.lang.Throwable, Methods$Inner)",
+ "Expected ( at position 1");
+ }, inspector -> noSignatureAttribute(lookupGeneric(inspector)));
+ }
+
+ @Test
+ public void classNotFound() throws Exception {
+ String signature = "<T:LNotFound;>(TT;LAlsoNotFound<TT;>.InnerNotFound.InnerAlsoNotFound;)TT;";
+ testSingleMethod("generic", signature, this::noWarnings,
+ inspector -> {
+ ClassSubject methods = inspector.clazz("Methods");
+ MethodSubject method =
+ methods.method("java.lang.Throwable", "generic",
+ ImmutableList.of("java.lang.Throwable", "Methods$Inner"));
+ assertThat(inspector.clazz("NotFound"), not(isPresent()));
+ assertEquals(signature, method.getOriginalSignatureAttribute());
+ });
+ }
+
+ @Test
+ public void multipleWarnings() throws Exception {
+ runTest(ImmutableMap.of(
+ "generic", "X",
+ "parameterizedReturn", "X",
+ "parameterizedArguments", "X"
+ ), diagnostics -> {
+ assertEquals(3, diagnostics.warnings.size());
+ }, inspector -> {
+ noSignatureAttribute(lookupGeneric(inspector));
+ noSignatureAttribute(lookupParameterizedReturn(inspector));
+ noSignatureAttribute(lookupParameterizedArguments(inspector));
+ });
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspector.java b/src/test/java/com/android/tools/r8/utils/DexInspector.java
index 38de25e..fd32ec7 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -56,6 +56,7 @@
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationElement;
+import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexCode;
@@ -92,6 +93,7 @@
import java.util.List;
import java.util.NoSuchElementException;
import java.util.concurrent.ExecutionException;
+import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -198,6 +200,50 @@
}
}
+ DexAnnotation findAnnotation(String name, DexAnnotationSet annotations) {
+ for (DexAnnotation annotation : annotations.annotations) {
+ DexType type = annotation.annotation.type;
+ String original = mapping == null ? type.toSourceString() : mapping.originalNameOf(type);
+ if (original.equals(name)) {
+ return annotation;
+ }
+ }
+ return null;
+ }
+
+ public String getFinalSignatureAttribute(DexAnnotationSet annotations) {
+ DexAnnotation annotation =
+ findAnnotation("dalvik.annotation.Signature", annotations);
+ if (annotation == null) {
+ return null;
+ }
+ assert annotation.annotation.elements.length == 1;
+ DexAnnotationElement element = annotation.annotation.elements[0];
+ assert element.value instanceof DexValueArray;
+ StringBuilder builder = new StringBuilder();
+ DexValueArray valueArray = (DexValueArray) element.value;
+ for (DexValue value : valueArray.getValues()) {
+ assertTrue(value instanceof DexValueString);
+ DexValueString s = (DexValueString) value;
+ builder.append(s.getValue());
+ }
+ return builder.toString();
+ }
+
+ public String getOriginalSignatureAttribute(
+ DexAnnotationSet annotations, BiConsumer<GenericSignatureParser, String> parse) {
+ String finalSignature = getFinalSignatureAttribute(annotations);
+ if (finalSignature == null || mapping == null) {
+ return finalSignature;
+ }
+
+ GenericSignatureGenerater rewriter = new GenericSignatureGenerater();
+ GenericSignatureParser<String> parser = new GenericSignatureParser<>(rewriter);
+ parse.accept(parser, finalSignature);
+ return rewriter.getSignature();
+ }
+
+
public ClassSubject clazz(Class clazz) {
return clazz(clazz.getTypeName());
}
@@ -582,23 +628,12 @@
assert !name.endsWith("EnclosingClass")
&& !name.endsWith("EnclosingMethod")
&& !name.endsWith("InnerClass");
- DexAnnotation annotation = findAnnotation(name);
+ DexAnnotation annotation = findAnnotation(name, dexClass.annotations);
return annotation == null
? new AbsentAnnotationSubject()
: new FoundAnnotationSubject(annotation);
}
- private DexAnnotation findAnnotation(String name) {
- for (DexAnnotation annotation : dexClass.annotations.annotations) {
- DexType type = annotation.annotation.type;
- String original = mapping == null ? type.toSourceString() : mapping.originalNameOf(type);
- if (original.equals(name)) {
- return annotation;
- }
- }
- return null;
- }
-
@Override
public String getOriginalName() {
if (naming != null) {
@@ -663,94 +698,13 @@
@Override
public String getOriginalSignatureAttribute() {
- // Build the generic signature using the current mapping if any.
- class GenericSignatureGenerater implements GenericSignatureAction<String> {
-
- private StringBuilder signature;
-
- public String getSignature() {
- return signature.toString();
- }
-
- @Override
- public void parsedSymbol(char symbol) {
- signature.append(symbol);
- }
-
- @Override
- public void parsedIdentifier(String identifier) {
- signature.append(identifier);
- }
-
- @Override
- public String parsedTypeName(String name) {
- String type = name;
- if (originalToObfuscatedMapping != null) {
- String original = originalToObfuscatedMapping.inverse().get(name);
- type = original != null ? original : name;
- }
- signature.append(type);
- return type;
- }
-
- @Override
- public String parsedInnerTypeName(String enclosingType, String name) {
- if (originalToObfuscatedMapping != null) {
- // The enclosingType has already been mapped if a mapping is present.
- String minifiedEnclosing = originalToObfuscatedMapping.get(enclosingType);
- String type = originalToObfuscatedMapping.inverse().get(minifiedEnclosing + "$" + name);
- if (type != null) {
- assert type.startsWith(enclosingType + "$");
- name = type.substring(enclosingType.length() + 1);
- }
- signature.append(name);
- return type;
- } else {
- String type = enclosingType + "$" + name;
- signature.append(name);
- return type;
- }
- }
-
- @Override
- public void start() {
- signature = new StringBuilder();
- }
-
- @Override
- public void stop() {
- // nothing to do
- }
- }
-
- String finalSignature = getFinalSignatureAttribute();
- if (finalSignature == null || mapping == null) {
- return finalSignature;
- }
-
- GenericSignatureGenerater rewriter = new GenericSignatureGenerater();
- GenericSignatureParser<String> parser = new GenericSignatureParser<>(rewriter);
- parser.parseClassSignature(finalSignature);
- return rewriter.getSignature();
+ return DexInspector.this.getOriginalSignatureAttribute(
+ dexClass.annotations, GenericSignatureParser::parseClassSignature);
}
@Override
public String getFinalSignatureAttribute() {
- DexAnnotation annotation = findAnnotation("dalvik.annotation.Signature");
- if (annotation == null) {
- return null;
- }
- assert annotation.annotation.elements.length == 1;
- DexAnnotationElement element = annotation.annotation.elements[0];
- assert element.value instanceof DexValueArray;
- StringBuilder builder = new StringBuilder();
- DexValueArray valueArray = (DexValueArray) element.value;
- for (DexValue value : valueArray.getValues()) {
- assertTrue(value instanceof DexValueString);
- DexValueString s = (DexValueString) value;
- builder.append(s.getValue());
- }
- return builder.toString();
+ return DexInspector.this.getFinalSignatureAttribute(dexClass.annotations);
}
@Override
@@ -790,6 +744,10 @@
public abstract boolean isClassInitializer();
+ public abstract String getOriginalSignatureAttribute();
+
+ public abstract String getFinalSignatureAttribute();
+
public abstract DexEncodedMethod getMethod();
public Iterator<InstructionSubject> iterateInstructions() {
@@ -858,6 +816,16 @@
public Signature getFinalSignature() {
return null;
}
+
+ @Override
+ public String getOriginalSignatureAttribute() {
+ return null;
+ }
+
+ @Override
+ public String getFinalSignatureAttribute() {
+ return null;
+ }
}
public class FoundMethodSubject extends MethodSubject {
@@ -918,7 +886,31 @@
@Override
public MethodSignature getOriginalSignature() {
MethodSignature signature = getFinalSignature();
- MemberNaming memberNaming = clazz.naming != null ? clazz.naming.lookup(signature) : null;
+ if (clazz.naming == null) {
+ return signature;
+ }
+
+ // Map the parameters and return type to original names. This is needed as the in the
+ // Proguard map the names on the left side are the original names. E.g.
+ //
+ // X -> a
+ // X method(X) -> a
+ //
+ // whereas the final signature is for X.a is "a (a)"
+ String[] OriginalParameters = new String[signature.parameters.length];
+ for (int i = 0; i < OriginalParameters.length; i++) {
+ String obfuscated = signature.parameters[i];
+ String original = originalToObfuscatedMapping.inverse().get(obfuscated);
+ OriginalParameters[i] = original != null ? original : obfuscated;
+ }
+ String obfuscatedReturnType = signature.type;
+ String originalReturnType = originalToObfuscatedMapping.inverse().get(obfuscatedReturnType);
+ String returnType = originalReturnType != null ? originalReturnType : obfuscatedReturnType;
+
+ MethodSignature lookupSignature =
+ new MethodSignature(signature.name, returnType, OriginalParameters);
+
+ MemberNaming memberNaming = clazz.naming.lookup(lookupSignature);
return memberNaming != null
? (MethodSignature) memberNaming.getOriginalSignature()
: signature;
@@ -930,6 +922,17 @@
}
@Override
+ public String getOriginalSignatureAttribute() {
+ return DexInspector.this.getOriginalSignatureAttribute(
+ dexMethod.annotations, GenericSignatureParser::parseMethodSignature);
+ }
+
+ @Override
+ public String getFinalSignatureAttribute() {
+ return DexInspector.this.getFinalSignatureAttribute(dexMethod.annotations);
+ }
+
+ @Override
public Iterator<InstructionSubject> iterateInstructions() {
return new InstructionIterator(this);
}
@@ -954,6 +957,10 @@
public abstract DexValue getStaticValue();
public abstract boolean isRenamed();
+
+ public abstract String getOriginalSignatureAttribute();
+
+ public abstract String getFinalSignatureAttribute();
}
public class AbsentFieldSubject extends FieldSubject {
@@ -1002,6 +1009,16 @@
public DexEncodedField getField() {
return null;
}
+
+ @Override
+ public String getOriginalSignatureAttribute() {
+ return null;
+ }
+
+ @Override
+ public String getFinalSignatureAttribute() {
+ return null;
+ }
}
public class FoundFieldSubject extends FieldSubject {
@@ -1042,7 +1059,24 @@
@Override
public FieldSignature getOriginalSignature() {
FieldSignature signature = getFinalSignature();
- MemberNaming memberNaming = clazz.naming != null ? clazz.naming.lookup(signature) : null;
+ if (clazz.naming == null) {
+ return signature;
+ }
+
+ // Map the type to the original name. This is needed as the in the Proguard map the
+ // names on the left side are the original names. E.g.
+ //
+ // X -> a
+ // X field -> a
+ //
+ // whereas the final signature is for X.a is "a a"
+ String obfuscatedType = signature.type;
+ String originalType = originalToObfuscatedMapping.inverse().get(obfuscatedType);
+ String fieldType = originalType != null ? originalType : obfuscatedType;
+
+ FieldSignature lookupSignature = new FieldSignature(signature.name, fieldType);
+
+ MemberNaming memberNaming = clazz.naming.lookup(lookupSignature);
return memberNaming != null
? (FieldSignature) memberNaming.getOriginalSignature()
: signature;
@@ -1069,6 +1103,17 @@
}
@Override
+ public String getOriginalSignatureAttribute() {
+ return DexInspector.this.getOriginalSignatureAttribute(
+ dexField.annotations, GenericSignatureParser::parseFieldSignature);
+ }
+
+ @Override
+ public String getFinalSignatureAttribute() {
+ return DexInspector.this.getFinalSignatureAttribute(dexField.annotations);
+ }
+
+ @Override
public String toString() {
return dexField.toSourceString();
}
@@ -1523,4 +1568,63 @@
return result;
}
}
+
+ // Build the generic signature using the current mapping if any.
+ class GenericSignatureGenerater implements GenericSignatureAction<String> {
+
+ private StringBuilder signature;
+
+ public String getSignature() {
+ return signature.toString();
+ }
+
+ @Override
+ public void parsedSymbol(char symbol) {
+ signature.append(symbol);
+ }
+
+ @Override
+ public void parsedIdentifier(String identifier) {
+ signature.append(identifier);
+ }
+
+ @Override
+ public String parsedTypeName(String name) {
+ String type = name;
+ if (originalToObfuscatedMapping != null) {
+ String original = originalToObfuscatedMapping.inverse().get(name);
+ type = original != null ? original : name;
+ }
+ signature.append(type);
+ return type;
+ }
+
+ @Override
+ public String parsedInnerTypeName(String enclosingType, String name) {
+ String type;
+ if (originalToObfuscatedMapping != null) {
+ // The enclosingType has already been mapped if a mapping is present.
+ String minifiedEnclosing = originalToObfuscatedMapping.get(enclosingType);
+ type = originalToObfuscatedMapping.inverse().get(minifiedEnclosing + "$" + name);
+ if (type != null) {
+ assert type.startsWith(enclosingType + "$");
+ name = type.substring(enclosingType.length() + 1);
+ }
+ } else {
+ type = enclosingType + "$" + name;
+ }
+ signature.append(name);
+ return type;
+ }
+
+ @Override
+ public void start() {
+ signature = new StringBuilder();
+ }
+
+ @Override
+ public void stop() {
+ // nothing to do
+ }
+ }
}
diff --git a/tools/r8lib_size_compare.py b/tools/r8lib_size_compare.py
index 4bb49eb..84dd1a2 100755
--- a/tools/r8lib_size_compare.py
+++ b/tools/r8lib_size_compare.py
@@ -87,7 +87,7 @@
'-injar', utils.R8_JAR,
'-printmapping', pg_map,
'-outjar', pg_output]
- for library_name, relocated_package in utils.R8_RELOCATIONS:
+ for library_name, relocated_package in R8_RELOCATIONS:
pg_args.extend(['-dontwarn', relocated_package + '.**',
'-dontnote', relocated_package + '.**'])
check_call(pg_args)
