Merge "Add (ignored) test for tree shaking"
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 63c1173..409c437 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.3.5-dev";
+ public static final String LABEL = "1.3.6-dev";
private Version() {
}
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 001e5b6..8de4a6e 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -110,6 +110,16 @@
public abstract DexField lookupField(DexField field);
+ // The method lookupMethod() maps a pair INVOKE=(method signature, invoke type) to a new pair
+ // INVOKE'=(method signature', invoke type'). This mapping can be context sensitive, meaning that
+ // the result INVOKE' depends on where the invocation INVOKE is in the program. This is, for
+ // example, used by the vertical class merger to translate invoke-super instructions that hit
+ // a method in the direct super class to invoke-direct instructions after class merging.
+ //
+ // This method can be used to determine if a graph lense is context sensitive. If a graph lense
+ // is context insensitive, it is safe to invoke lookupMethod() without a context (or to pass null
+ // as context). Trying to invoke a context sensitive graph lense without a context will lead to
+ // an assertion error.
public abstract boolean isContextFreeForMethods();
public boolean isContextFreeForMethod(DexMethod method) {
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 3288aa9..fffbc44 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
@@ -149,7 +149,8 @@
this.protoLiteRewriter =
options.forceProguardCompatibility ? null : new ProtoLitePruner(appInfo.withLiveness());
if (!appInfo.withLiveness().identifierNameStrings.isEmpty() && options.enableMinification) {
- this.identifierNameStringMarker = new IdentifierNameStringMarker(appInfo.withLiveness());
+ this.identifierNameStringMarker =
+ new IdentifierNameStringMarker(appInfo.withLiveness(), options);
} else {
this.identifierNameStringMarker = null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 1e9566a..073d6ae 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -12,6 +13,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.ir.optimize.Inliner.Constraint;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import java.util.Collection;
@@ -101,16 +103,37 @@
return Constraint.classIsVisible(invocationContext, type, appInfo);
}
- public Constraint forInvokeCustom() {
- return Constraint.NEVER;
- }
-
public Constraint forInstancePut(DexField field, DexType invocationContext) {
DexField lookup = graphLense.lookupField(field);
return forFieldInstruction(
lookup, appInfo.lookupInstanceTarget(lookup.clazz, lookup), invocationContext);
}
+ public Constraint forInvoke(DexMethod method, Type type, DexType invocationContext) {
+ switch (type) {
+ case DIRECT:
+ return forInvokeDirect(method, invocationContext);
+ case INTERFACE:
+ return forInvokeInterface(method, invocationContext);
+ case STATIC:
+ return forInvokeStatic(method, invocationContext);
+ case SUPER:
+ return forInvokeSuper(method, invocationContext);
+ case VIRTUAL:
+ return forInvokeVirtual(method, invocationContext);
+ case CUSTOM:
+ return forInvokeCustom();
+ case POLYMORPHIC:
+ return forInvokePolymorphic(method, invocationContext);
+ default:
+ throw new Unreachable("Unexpected type: " + type);
+ }
+ }
+
+ public Constraint forInvokeCustom() {
+ return Constraint.NEVER;
+ }
+
public Constraint forInvokeDirect(DexMethod method, DexType invocationContext) {
DexMethod lookup = graphLense.lookupMethod(method);
return forSingleTargetInvoke(lookup, appInfo.lookupDirectTarget(lookup), invocationContext);
diff --git a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
index 4807b89..459d7de 100644
--- a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
+++ b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.JarApplicationReader;
+import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.conversion.JarSourceCode;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
import com.android.tools.r8.ir.optimize.InliningConstraints;
@@ -34,6 +35,7 @@
private final JarApplicationReader application;
private final AppInfoWithLiveness appInfo;
+ private final GraphLense graphLense;
private final InliningConstraints inliningConstraints;
private final DexEncodedMethod method;
private final DexType invocationContext;
@@ -47,8 +49,10 @@
DexEncodedMethod method,
DexType invocationContext) {
super(ASM6);
+ assert graphLense.isContextFreeForMethods();
this.application = application;
this.appInfo = appInfo;
+ this.graphLense = graphLense;
this.inliningConstraints = new InliningConstraints(appInfo, graphLense);
this.method = method;
this.invocationContext = invocationContext;
@@ -116,48 +120,73 @@
public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
DexType ownerType = application.getTypeFromName(owner);
DexMethod target = application.getMethod(ownerType, name, desc);
+
+ // Find the DEX invocation type.
+ Invoke.Type type;
switch (opcode) {
case Opcodes.INVOKEDYNAMIC:
- if (JarSourceCode.isCallToPolymorphicSignatureMethod(owner, name)) {
- updateConstraint(inliningConstraints.forInvokePolymorphic(target, invocationContext));
- } else {
- updateConstraint(inliningConstraints.forInvokeCustom());
- }
+ type =
+ JarSourceCode.isCallToPolymorphicSignatureMethod(owner, name)
+ ? Invoke.Type.POLYMORPHIC
+ : Invoke.Type.CUSTOM;
+ assert noNeedToUseGraphLense(target, type);
break;
case Opcodes.INVOKEINTERFACE:
- updateConstraint(inliningConstraints.forInvokeInterface(target, invocationContext));
+ // Could have changed to an invoke-virtual instruction due to vertical class merging
+ // (if an interface is merged into a class).
+ type = graphLense.lookupMethod(target, null, Invoke.Type.INTERFACE).getType();
+ assert type == Invoke.Type.INTERFACE || type == Invoke.Type.VIRTUAL;
break;
case Opcodes.INVOKESPECIAL:
- if (name.equals(Constants.INSTANCE_INITIALIZER_NAME) || ownerType == invocationContext) {
- updateConstraint(inliningConstraints.forInvokeDirect(target, invocationContext));
+ if (name.equals(Constants.INSTANCE_INITIALIZER_NAME)) {
+ type = Invoke.Type.DIRECT;
+ assert noNeedToUseGraphLense(target, type);
+ } else if (ownerType == invocationContext) {
+ // The method could have been publicized.
+ type = graphLense.lookupMethod(target, null, Invoke.Type.DIRECT).getType();
+ assert type == Invoke.Type.DIRECT || type == Invoke.Type.VIRTUAL;
} else {
- updateConstraint(inliningConstraints.forInvokeSuper(target, invocationContext));
+ // This is a super call. Note that the vertical class merger translates some invoke-super
+ // instructions to invoke-direct. However, when that happens, the invoke instruction and
+ // the target method end up being in the same class, and therefore, we will allow inlining
+ // it. The result of using type=SUPER below will be the same, since it leads to the
+ // inlining constraint SAMECLASS.
+ // TODO(christofferqa): Consider using graphLense.lookupMethod (to do this, we need the
+ // context for the graph lense, though).
+ type = Invoke.Type.SUPER;
+ assert noNeedToUseGraphLense(target, type);
}
break;
case Opcodes.INVOKESTATIC:
- updateConstraint(inliningConstraints.forInvokeStatic(target, invocationContext));
+ type = Invoke.Type.STATIC;
+ assert noNeedToUseGraphLense(target, type);
break;
case Opcodes.INVOKEVIRTUAL:
- // Instructions that target a private method in the same class are translated to
- // invoke-direct.
+ type = Invoke.Type.VIRTUAL;
+ // Instructions that target a private method in the same class translates to invoke-direct.
if (target.holder == method.method.holder) {
DexClass clazz = appInfo.definitionFor(target.holder);
if (clazz != null && clazz.lookupDirectMethod(target) != null) {
- updateConstraint(inliningConstraints.forInvokeDirect(target, invocationContext));
- break;
+ type = Invoke.Type.DIRECT;
}
}
-
- updateConstraint(inliningConstraints.forInvokeVirtual(target, invocationContext));
+ assert noNeedToUseGraphLense(target, type);
break;
default:
throw new Unreachable("Unexpected opcode " + opcode);
}
+
+ updateConstraint(inliningConstraints.forInvoke(target, type, invocationContext));
+ }
+
+ private boolean noNeedToUseGraphLense(DexMethod method, Invoke.Type type) {
+ assert graphLense.lookupMethod(method, null, type).getType() == type;
+ return true;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
index 57e8514..581ac8a 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.naming.IdentifierNameStringUtils.identifyIdentiferNameString;
import static com.android.tools.r8.naming.IdentifierNameStringUtils.inferMemberOrTypeFromNameString;
import static com.android.tools.r8.naming.IdentifierNameStringUtils.isReflectionMethod;
+import static com.android.tools.r8.naming.IdentifierNameStringUtils.warnUndeterminedIdentifier;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexEncodedField;
@@ -31,7 +32,9 @@
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.StaticPut;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Streams;
import java.util.Arrays;
import java.util.List;
@@ -44,12 +47,14 @@
private final AppInfo appInfo;
private final DexItemFactory dexItemFactory;
private final Set<DexItem> identifierNameStrings;
+ private final InternalOptions options;
- public IdentifierNameStringMarker(AppInfoWithLiveness appInfo) {
+ public IdentifierNameStringMarker(AppInfoWithLiveness appInfo, InternalOptions options) {
this.appInfo = appInfo;
this.dexItemFactory = appInfo.dexItemFactory;
// Note that this info is only available at AppInfoWithLiveness.
this.identifierNameStrings = appInfo.identifierNameStrings;
+ this.options = options;
}
public void decoupleIdentifierNameStringsInFields() {
@@ -77,6 +82,7 @@
}
public void decoupleIdentifierNameStringsInMethod(DexEncodedMethod encodedMethod, IRCode code) {
+ Origin origin = appInfo.originFor(code.method.method.getHolder());
ListIterator<BasicBlock> blocks = code.listIterator();
while (blocks.hasNext()) {
BasicBlock block = blocks.next();
@@ -104,11 +110,17 @@
? instruction.asStaticPut().inValue()
: instruction.asInstancePut().value();
if (!in.isConstString()) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(options.reporter, field, origin, instruction, null);
+ }
continue;
}
DexString original = in.getConstInstruction().asConstString().getValue();
DexItemBasedString itemBasedString = inferMemberOrTypeFromNameString(appInfo, original);
if (itemBasedString == null) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(options.reporter, field, origin, instruction, original);
+ }
continue;
}
// Move the cursor back to $fieldPut
@@ -162,6 +174,10 @@
if (isReflectionMethod(dexItemFactory, invokedMethod)) {
DexItemBasedString itemBasedString = identifyIdentiferNameString(appInfo, invoke);
if (itemBasedString == null) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(
+ options.reporter, invokedMethod, origin, instruction, null);
+ }
continue;
}
DexType returnType = invoke.getReturnType();
@@ -206,12 +222,20 @@
for (int i = 0; i < ins.size(); i++) {
Value in = ins.get(i);
if (!in.isConstString()) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(
+ options.reporter, invokedMethod, origin, instruction, null);
+ }
continue;
}
DexString original = in.getConstInstruction().asConstString().getValue();
DexItemBasedString itemBasedString =
inferMemberOrTypeFromNameString(appInfo, original);
if (itemBasedString == null) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(
+ options.reporter, invokedMethod, origin, instruction, original);
+ }
continue;
}
// Move the cursor back to $invoke
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringUtils.java b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringUtils.java
index 4c64cf6..471b3f8 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringUtils.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringUtils.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexItemBasedString;
import com.android.tools.r8.graph.DexItemFactory;
@@ -23,6 +24,10 @@
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.TextPosition;
+import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.StringDiagnostic;
import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap;
import java.util.List;
import java.util.Map;
@@ -366,4 +371,27 @@
}
return new DexTypeList(types);
}
+
+ public static void warnUndeterminedIdentifier(
+ Reporter reporter,
+ DexItem member,
+ Origin origin,
+ Instruction instruction,
+ DexString original) {
+ assert member instanceof DexField || member instanceof DexMethod;
+ String kind = member instanceof DexField ? "field" : "method";
+ String originalMessage = original == null ? "what identifier string flows to "
+ : "what '" + original.toString() + "' refers to, which flows to ";
+ String message =
+ "Cannot determine " + originalMessage + member.toSourceString()
+ + " that is specified in -identifiernamestring rules."
+ + " Thus, not all identifier strings flowing to that " + kind
+ + " are renamed, which can cause resolution failures at runtime.";
+ StringDiagnostic diagnostic =
+ instruction.getPosition().line >= 1
+ ? new StringDiagnostic(message, origin,
+ new TextPosition(0L, instruction.getPosition().line, 1))
+ : new StringDiagnostic(message, origin);
+ reporter.warning(diagnostic);
+ }
}
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 a1cc440..061b9bf 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -5,6 +5,7 @@
import static com.android.tools.r8.naming.IdentifierNameStringUtils.identifyIdentiferNameString;
import static com.android.tools.r8.naming.IdentifierNameStringUtils.isReflectionMethod;
+import static com.android.tools.r8.naming.IdentifierNameStringUtils.warnUndeterminedIdentifier;
import static com.android.tools.r8.shaking.ProguardConfigurationUtils.buildIdentifierNameStringRule;
import com.android.tools.r8.Diagnostic;
@@ -39,6 +40,7 @@
import com.android.tools.r8.ir.code.Invoke.Type;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.shaking.protolite.ProtoLiteExtension;
@@ -1357,11 +1359,13 @@
}
private void handleProguardReflectiveBehavior(DexEncodedMethod method) {
- IRCode code = method.buildIR(appInfo, options, appInfo.originFor(method.method.holder));
- code.instructionIterator().forEachRemaining(this::handleProguardReflectiveBehavior);
+ Origin origin = appInfo.originFor(method.method.holder);
+ IRCode code = method.buildIR(appInfo, options, origin);
+ code.instructionIterator().forEachRemaining(instr ->
+ handleProguardReflectiveBehavior(instr, origin));
}
- private void handleProguardReflectiveBehavior(Instruction instruction) {
+ private void handleProguardReflectiveBehavior(Instruction instruction, Origin origin) {
if (!instruction.isInvokeMethod()) {
return;
}
@@ -1372,6 +1376,9 @@
}
DexItemBasedString itemBasedString = identifyIdentiferNameString(appInfo, invoke);
if (itemBasedString == null) {
+ if (options.proguardConfiguration.isObfuscating()) {
+ warnUndeterminedIdentifier(options.reporter, invokedMethod, origin, instruction, null);
+ }
return;
}
if (itemBasedString.basedOn instanceof DexType) {
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 2fbdab2..0840d91 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -146,8 +146,8 @@
private final Timing timing;
private Collection<DexMethod> invokes;
- // Set of merge candidates.
- private final Set<DexProgramClass> mergeCandidates = new HashSet<>();
+ // Set of merge candidates. Note that this must have a deterministic iteration order.
+ private final Set<DexProgramClass> mergeCandidates = new LinkedHashSet<>();
// Map from source class to target class.
private final Map<DexType, DexType> mergedClasses = new HashMap<>();
@@ -449,7 +449,7 @@
// different proto, it could be the case that a method with the given name is overloaded.
DexProto existing =
overloadingInfo.computeIfAbsent(signature.name, key -> signature.proto);
- if (!existing.equals(signature.proto)) {
+ if (existing != DexProto.SENTINEL && !existing.equals(signature.proto)) {
// Mark that this signature is overloaded by mapping it to SENTINEL.
overloadingInfo.put(signature.name, DexProto.SENTINEL);
}
@@ -815,25 +815,35 @@
mergeMethods(virtualMethods.values(), target.virtualMethods());
// Step 2: Merge fields
- Set<Wrapper<DexField>> existingFieldSignatures = new HashSet<>();
- addAll(existingFieldSignatures, target.fields(), FieldSignatureEquivalence.get());
+ Set<DexString> existingFieldNames = new HashSet<>();
+ for (DexEncodedField field : target.fields()) {
+ existingFieldNames.add(field.field.name);
+ }
+ // In principle, we could allow multiple fields with the same name, and then only rename the
+ // field in the end when we are done merging all the classes, if it it turns out that the two
+ // fields ended up having the same type. This would not be too expensive, since we visit the
+ // entire program using VerticalClassMerger.TreeFixer anyway.
+ //
+ // For now, we conservatively report that a signature is already taken if there is a field
+ // with the same name. If minification is used with -overloadaggressively, this is solved
+ // later anyway.
Predicate<DexField> availableFieldSignatures =
- field -> !existingFieldSignatures.contains(FieldSignatureEquivalence.get().wrap(field));
+ field -> !existingFieldNames.contains(field.name);
DexEncodedField[] mergedInstanceFields =
mergeFields(
source.instanceFields(),
target.instanceFields(),
availableFieldSignatures,
- existingFieldSignatures);
+ existingFieldNames);
DexEncodedField[] mergedStaticFields =
mergeFields(
source.staticFields(),
target.staticFields(),
availableFieldSignatures,
- existingFieldSignatures);
+ existingFieldNames);
// Step 3: Merge interfaces
Set<DexType> interfaces = mergeArrays(target.interfaces.values, source.interfaces.values);
@@ -1006,13 +1016,13 @@
DexEncodedField[] sourceFields,
DexEncodedField[] targetFields,
Predicate<DexField> availableFieldSignatures,
- Set<Wrapper<DexField>> existingFieldSignatures) {
+ Set<DexString> existingFieldNames) {
DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length];
// Add fields from source
int i = 0;
for (DexEncodedField field : sourceFields) {
DexEncodedField resultingField = renameFieldIfNeeded(field, availableFieldSignatures);
- existingFieldSignatures.add(FieldSignatureEquivalence.get().wrap(resultingField.field));
+ existingFieldNames.add(resultingField.field.name);
deferredRenamings.map(field.field, resultingField.field);
result[i] = resultingField;
i++;
@@ -1396,14 +1406,32 @@
@Override
public DexType lookupType(DexType type) {
- return type == source ? target : type;
+ return type == source ? target : mergedClasses.getOrDefault(type, type);
}
@Override
public GraphLenseLookupResult lookupMethod(
DexMethod method, DexEncodedMethod context, Type type) {
- return new GraphLenseLookupResult(
- renamedMembersLense.methodMap.getOrDefault(method, method), type);
+ // First look up the method using the existing graph lense (for example, the type will have
+ // changed if the method was publicized by ClassAndMemberPublicizer).
+ GraphLenseLookupResult lookup = graphLense.lookupMethod(method, context, type);
+ DexMethod previousMethod = lookup.getMethod();
+ Type previousType = lookup.getType();
+ // Then check if there is a renaming due to the vertical class merger.
+ DexMethod newMethod = renamedMembersLense.methodMap.get(previousMethod);
+ if (newMethod != null) {
+ if (previousType == Type.INTERFACE) {
+ // If an interface has been merged into a class, invoke-interface needs to be translated
+ // to invoke-virtual.
+ DexClass clazz = appInfo.definitionFor(newMethod.holder);
+ if (clazz != null && !clazz.accessFlags.isInterface()) {
+ assert appInfo.definitionFor(method.holder).accessFlags.isInterface();
+ return new GraphLenseLookupResult(newMethod, Type.VIRTUAL);
+ }
+ }
+ return new GraphLenseLookupResult(newMethod, previousType);
+ }
+ return new GraphLenseLookupResult(previousMethod, previousType);
}
@Override
diff --git a/src/test/examples/classmerging/FieldCollisionTest.java b/src/test/examples/classmerging/FieldCollisionTest.java
new file mode 100644
index 0000000..5bb2260
--- /dev/null
+++ b/src/test/examples/classmerging/FieldCollisionTest.java
@@ -0,0 +1,43 @@
+// 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 FieldCollisionTest {
+
+ private static final B SENTINEL_A = new B("A");
+ private static final B SENTINEL_B = new B("B");
+
+ public static void main(String[] args) {
+ B obj = new B();
+ System.out.println(obj.toString());
+ }
+
+ // Will be merged into B.
+ public static class A {
+
+ // After class merging, this field will have the same name and type as the field B.obj,
+ // unless we handle the collision.
+ protected final A obj = SENTINEL_A;
+ }
+
+ public static class B extends A {
+
+ protected final String message;
+ protected final B obj = SENTINEL_B;
+
+ public B() {
+ this(null);
+ }
+
+ public B(String message) {
+ this.message = message;
+ }
+
+ @Override
+ public String toString() {
+ return obj.message + System.lineSeparator() + ((B) super.obj).message;
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/MethodCollisionTest.java b/src/test/examples/classmerging/MethodCollisionTest.java
new file mode 100644
index 0000000..a9010a4
--- /dev/null
+++ b/src/test/examples/classmerging/MethodCollisionTest.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 classmerging;
+
+public class MethodCollisionTest {
+
+ public static void main(String[] args) {
+ new B().m();
+ new D().m();
+ }
+
+ public static class A {
+
+ // After class merging, this method will have the same signature as the method B.m,
+ // unless we handle the collision.
+ private A m() {
+ System.out.println("A.m");
+ return null;
+ }
+
+ public void invokeM() {
+ m();
+ }
+ }
+
+ public static class B extends A {
+
+ private B m() {
+ System.out.println("B.m");
+ invokeM();
+ return null;
+ }
+ }
+
+ public static class C {
+
+ // After class merging, this method will have the same signature as the method D.m,
+ // unless we handle the collision.
+ public C m() {
+ System.out.println("C.m");
+ return null;
+ }
+ }
+
+ public static class D extends C {
+
+ public D m() {
+ System.out.println("D.m");
+ super.m();
+ return null;
+ }
+ }
+
+
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index b3e0ad1..4ba014e 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -19,6 +19,12 @@
-keep public class classmerging.ExceptionTest {
public static void main(...);
}
+-keep public class classmerging.FieldCollisionTest {
+ public static void main(...);
+}
+-keep public class classmerging.MethodCollisionTest {
+ public static void main(...);
+}
-keep public class classmerging.RewritePinnedMethodTest {
public static void main(...);
}
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 e0d73e8..f28c86f 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -187,6 +187,22 @@
}
@Test
+ public void testFieldCollision() throws Exception {
+ String main = "classmerging.FieldCollisionTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("FieldCollisionTest.class"),
+ CF_DIR.resolve("FieldCollisionTest$A.class"),
+ CF_DIR.resolve("FieldCollisionTest$B.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.FieldCollisionTest",
+ "classmerging.FieldCollisionTest$B");
+ runTest(main, programFiles, preservedClassNames::contains);
+ }
+
+ @Test
public void testLambdaRewriting() throws Exception {
String main = "classmerging.LambdaRewritingTest";
Path[] programFiles =
@@ -210,6 +226,31 @@
}
@Test
+ public void testMethodCollision() throws Exception {
+ String main = "classmerging.MethodCollisionTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("MethodCollisionTest.class"),
+ CF_DIR.resolve("MethodCollisionTest$A.class"),
+ CF_DIR.resolve("MethodCollisionTest$B.class"),
+ CF_DIR.resolve("MethodCollisionTest$C.class"),
+ CF_DIR.resolve("MethodCollisionTest$D.class")
+ };
+ // TODO(christofferqa): Currently we do not allow merging A into B because we find a collision.
+ // However, we are free to change the names of private methods, so we should handle them similar
+ // to fields (i.e., we should allow merging A into B). This would also improve the performance
+ // of the collision detector, because it would only have to consider non-private methods.
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.MethodCollisionTest",
+ "classmerging.MethodCollisionTest$A",
+ "classmerging.MethodCollisionTest$B",
+ "classmerging.MethodCollisionTest$C",
+ "classmerging.MethodCollisionTest$D");
+ runTest(main, programFiles, preservedClassNames::contains);
+ }
+
+ @Test
public void testPinnedParameterTypes() throws Exception {
String main = "classmerging.PinnedParameterTypesTest";
Path[] programFiles =
diff --git a/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java b/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java
new file mode 100644
index 0000000..ddd0aa2
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java
@@ -0,0 +1,167 @@
+// 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 org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.DiagnosticsChecker;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.origin.Origin;
+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.KeepingDiagnosticHandler;
+import com.android.tools.r8.utils.Reporter;
+import com.google.common.collect.ImmutableList;
+import java.lang.reflect.Method;
+import java.nio.file.Path;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+class WarnReflectiveAccessTestMain {
+ int counter = 0;
+
+ WarnReflectiveAccessTestMain() {
+ }
+
+ public void foo() {
+ System.out.println("TestMain::foo(" + counter++ + ")");
+ }
+
+ public int boo() {
+ System.out.println("TestMain::boo(" + counter + ")");
+ return counter;
+ }
+
+ public static void main(String[] args) throws Exception {
+ WarnReflectiveAccessTestMain instance = new WarnReflectiveAccessTestMain();
+
+ StringBuilder builder = new StringBuilder();
+ builder.append("f");
+ builder.append("o").append("o");
+ Method foo = instance.getClass().getDeclaredMethod(builder.toString());
+ foo.invoke(instance);
+ }
+}
+
+@RunWith(VmTestRunner.class)
+public class WarnReflectiveAccessTest extends TestBase {
+
+ private KeepingDiagnosticHandler handler;
+ private Reporter reporter;
+
+ @Before
+ public void reset() {
+ handler = new KeepingDiagnosticHandler();
+ reporter = new Reporter(handler);
+ }
+
+ private AndroidApp runR8(
+ byte[][] classes,
+ Class main,
+ Path out,
+ boolean enableMinification,
+ boolean forceProguardCompatibility)
+ throws Exception {
+ String minificationModifier = enableMinification ? ",allowobfuscation " : " ";
+ String reflectionRules = forceProguardCompatibility ? ""
+ : "-identifiernamestring class java.lang.Class {\n"
+ + "static java.lang.Class forName(java.lang.String);\n"
+ + "java.lang.reflect.Method getDeclaredMethod(java.lang.String, java.lang.Class[]);\n"
+ + "}\n";
+ R8Command.Builder commandBuilder =
+ R8Command.builder(reporter)
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keep" + minificationModifier + "class " + main.getCanonicalName() + " {"
+ + " <methods>;"
+ + "}",
+ "-printmapping",
+ reflectionRules),
+ Origin.unknown())
+ .setOutput(out, OutputMode.DexIndexed);
+ for (byte[] clazz : classes) {
+ commandBuilder.addClassProgramData(clazz, Origin.unknown());
+ }
+ commandBuilder.addLibraryFiles(ToolHelper.getDefaultAndroidJar());
+ return ToolHelper.runR8(commandBuilder.build(), o -> {
+ o.enableInlining = false;
+ o.forceProguardCompatibility = forceProguardCompatibility;
+ });
+ }
+
+ private void reflectionWithBuildter(
+ boolean enableMinification,
+ boolean forceProguardCompatibility) throws Exception {
+ byte[][] classes = {
+ ToolHelper.getClassAsBytes(WarnReflectiveAccessTestMain.class)
+ };
+ Path out = temp.getRoot().toPath();
+ AndroidApp processedApp = runR8(classes, WarnReflectiveAccessTestMain.class, out,
+ enableMinification, forceProguardCompatibility);
+
+ String main = WarnReflectiveAccessTestMain.class.getCanonicalName();
+ DexInspector dexInspector = new DexInspector(processedApp);
+ ClassSubject mainSubject = dexInspector.clazz(main);
+ assertThat(mainSubject, isPresent());
+
+ ProcessResult javaOutput = runOnJavaRaw(main, classes);
+ assertEquals(0, javaOutput.exitCode);
+ assertThat(javaOutput.stdout, containsString("TestMain::foo"));
+
+ ProcessResult artOutput = runOnArtRaw(processedApp,
+ enableMinification ? mainSubject.getFinalName() : main);
+ if (enableMinification) {
+ assertNotEquals(0, artOutput.exitCode);
+ assertThat(artOutput.stderr, containsString("NoSuchMethodError"));
+ } else {
+ assertEquals(0, artOutput.exitCode);
+ assertThat(artOutput.stdout, containsString("TestMain::foo"));
+ }
+ }
+
+ @Test
+ public void test_minification_forceProguardCompatibility() throws Exception {
+ reflectionWithBuildter(true, true);
+ assertFalse(handler.warnings.isEmpty());
+ DiagnosticsChecker.checkDiagnostic(handler.warnings.get(0), (Path) null, 54, 1,
+ "Cannot determine", "getDeclaredMethod", "resolution failure");
+ }
+
+ @Test
+ public void test_noMinification_forceProguardCompatibility() throws Exception {
+ reflectionWithBuildter(false, true);
+ assertFalse(handler.warnings.isEmpty());
+ DiagnosticsChecker.checkDiagnostic(handler.warnings.get(0), (Path) null, 54, 1,
+ "Cannot determine", "getDeclaredMethod", "resolution failure");
+ }
+
+ @Test
+ public void test_minification_R8() throws Exception {
+ reflectionWithBuildter(true, false);
+ assertFalse(handler.warnings.isEmpty());
+ DiagnosticsChecker.checkDiagnostic(handler.warnings.get(0), (Path) null, 54, 1,
+ "Cannot determine", "getDeclaredMethod", "-identifiernamestring", "resolution failure");
+ }
+
+ @Test
+ public void test_noMinification_R8() throws Exception {
+ reflectionWithBuildter(false, false);
+ assertFalse(handler.warnings.isEmpty());
+ DiagnosticsChecker.checkDiagnostic(handler.warnings.get(0), (Path) null, 54, 1,
+ "Cannot determine", "getDeclaredMethod", "-identifiernamestring", "resolution failure");
+ }
+
+}
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 0b8de12..eba3be8 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -413,6 +413,8 @@
public abstract String getOriginalDescriptor();
+ public abstract String getFinalName();
+
public abstract String getFinalDescriptor();
public abstract boolean isMemberClass();
@@ -482,6 +484,11 @@
}
@Override
+ public String getFinalName() {
+ return null;
+ }
+
+ @Override
public String getFinalDescriptor() {
return null;
}
@@ -639,7 +646,7 @@
if (naming != null) {
return naming.originalName;
} else {
- return DescriptorUtils.descriptorToJavaType(getFinalDescriptor());
+ return getFinalName();
}
}
@@ -653,6 +660,11 @@
}
@Override
+ public String getFinalName() {
+ return DescriptorUtils.descriptorToJavaType(getFinalDescriptor());
+ }
+
+ @Override
public String getFinalDescriptor() {
return dexClass.type.descriptor.toString();
}
