Merge "Ensure that we can merge desugared java8 classes"
diff --git a/src/main/java/com/android/tools/r8/ir/code/Dup.java b/src/main/java/com/android/tools/r8/ir/code/Dup.java
index 4ef2370..89542a7 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Dup.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Dup.java
@@ -30,6 +30,7 @@
assert outValue == null || !outValue.hasUsersInfo() || !outValue.isUsed() ||
value instanceof StackValues;
this.outValue = value;
+ this.outValue.definition = this;
for (StackValue val : ((StackValues)value).getStackValues()) {
val.definition = this;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Dup2.java b/src/main/java/com/android/tools/r8/ir/code/Dup2.java
index 6d61f58..fcb5b8e 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Dup2.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Dup2.java
@@ -44,6 +44,7 @@
assert outValue == null || !outValue.hasUsersInfo() || !outValue.isUsed() ||
value instanceof StackValues;
this.outValue = value;
+ this.outValue.definition = this;
for (StackValue val : ((StackValues)value).getStackValues()) {
val.definition = this;
}
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 cb93a5e..bef2765 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
@@ -20,6 +20,7 @@
import com.google.common.collect.Sets;
import java.util.ArrayDeque;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Deque;
@@ -34,6 +35,7 @@
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
public class IRCode {
@@ -575,7 +577,10 @@
}
private boolean verifyDefinition(Value value) {
- assert value.definition.outValue() == value;
+ Value outValue = value.definition.outValue();
+ assert outValue == value
+ || (value instanceof StackValue
+ && Arrays.asList(((StackValues) outValue).getStackValues()).contains(value));
return true;
}
@@ -662,11 +667,15 @@
}
// After the throwing instruction only debug instructions and the final jump
// instruction is allowed.
+ // TODO(mkroghj) Temporarily allow stack-operations to be after throwing instructions.
if (seenThrowing) {
assert instruction.isDebugInstruction()
|| instruction.isJumpInstruction()
|| instruction.isStore()
- || instruction.isPop();
+ || instruction.isPop()
+ || instruction.isDup()
+ || instruction.isDup2()
+ || instruction.isSwap();
}
}
}
@@ -675,7 +684,7 @@
}
public boolean verifyNoImpreciseOrBottomTypes() {
- return verifySSATypeLattice(
+ Predicate<Value> verifyValue =
v -> {
assert v.getTypeLattice().isPreciseType();
assert !v.getTypeLattice().isFineGrainedType();
@@ -685,6 +694,16 @@
assert !(v.definition instanceof ImpreciseMemberTypeInstruction)
|| ((ImpreciseMemberTypeInstruction) v.definition).getMemberType().isPrecise();
return true;
+ };
+ return verifySSATypeLattice(
+ v -> {
+ // StackValues is an artificial type created to allow returning multiple values from an
+ // instruction.
+ if (v instanceof StackValues) {
+ return Stream.of(((StackValues) v).getStackValues()).allMatch(verifyValue);
+ } else {
+ return verifyValue(v);
+ }
});
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewArrayEmpty.java b/src/main/java/com/android/tools/r8/ir/code/NewArrayEmpty.java
index c010973..e235682 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NewArrayEmpty.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NewArrayEmpty.java
@@ -64,6 +64,24 @@
}
@Override
+ public boolean instructionInstanceCanThrow() {
+ return !(size().definition != null
+ && size().definition.isConstNumber()
+ && size().definition.asConstNumber().getRawValue() >= 0
+ && size().definition.asConstNumber().getRawValue() < Integer.MAX_VALUE);
+ }
+
+ @Override
+ public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
+ if (instructionInstanceCanThrow()) {
+ return false;
+ }
+ // This would belong better in instructionInstanceCanThrow, but that is not passed an appInfo.
+ DexType baseType = type.toBaseType(appInfo.dexItemFactory);
+ return baseType.isPrimitiveType() || appInfo.definitionFor(baseType) != null;
+ }
+
+ @Override
public boolean identicalNonValueNonPositionParts(Instruction other) {
return other.isNewArrayEmpty() && other.asNewArrayEmpty().type == type;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java b/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
index ed51913..f534fb9 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.code.FillArrayDataPayload;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.conversion.CfBuilder;
import com.android.tools.r8.ir.conversion.DexBuilder;
@@ -73,6 +74,19 @@
}
@Override
+ public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
+ if (!src().getTypeLattice().isNullable() && src().numberOfAllUsers() == 1) {
+ // The NewArrayFilledData instruction is only inserted by an R8 optimization following
+ // a NewArrayEmpty when there are more than one entry.
+ assert src().uniqueUsers().iterator().next() == this;
+ assert src().definition != null;
+ assert src().definition.isNewArrayEmpty();
+ return true;
+ }
+ return false;
+ }
+
+ @Override
public boolean instructionTypeCanThrow() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Swap.java b/src/main/java/com/android/tools/r8/ir/code/Swap.java
index 80edc80..3c623d6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Swap.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Swap.java
@@ -33,6 +33,7 @@
assert outValue == null || !outValue.hasUsersInfo() || !outValue.isUsed() ||
value instanceof StackValues;
this.outValue = value;
+ this.outValue.definition = this;
for (StackValue val : ((StackValues)value).getStackValues()) {
val.definition = this;
}
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 4dcc839..6b5f6c4 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
@@ -166,6 +166,7 @@
reachedFixpoint = !phiOptimizations.optimize(code);
}
}
+ assert code.isConsistentSSA();
registerAllocator = new CfRegisterAllocator(code, options, typeVerificationHelper);
registerAllocator.allocateRegisters();
loadStoreHelper.insertPhiMoves(registerAllocator);
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 be171c9..fa259c2 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
@@ -115,16 +115,15 @@
continue;
}
Value outValue = current.outValue();
- // Instructions with no out value cannot be dead code by the current definition
- // (unused out value). They typically side-effect input values or deals with control-flow.
- assert outValue != null;
- if (!outValue.isDead(appInfo)) {
+ if (outValue != null && !outValue.isDead(appInfo)) {
continue;
}
updateWorklist(worklist, current);
// All users will be removed for this instruction. Eagerly clear them so further inspection
// of this instruction during dead code elimination will terminate here.
- outValue.clearUsers();
+ if (outValue != null) {
+ outValue.clearUsers();
+ }
iterator.removeOrReplaceByDebugLocalRead();
}
}
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 ad084e4..b02801c 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -473,6 +473,11 @@
.toCharArray();
this.packageDictionaryIterator = packageDictionary.iterator();
this.classDictionaryIterator = classDictionary.iterator();
+
+ // R.class in Android, which contains constant IDs to assets, can be bundled at any time.
+ // Insert `R` immediately so that the class name minifier can skip that name by default.
+ StringBuilder rBuilder = new StringBuilder().append(packagePrefix).append("R;");
+ usedTypeNames.add(appInfo.dexItemFactory.createString(rBuilder.toString()));
}
public String getPackageName() {
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 adc6979..2babc02 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1736,6 +1736,14 @@
markInstantiated(holderClass.type, KeepReason.reflectiveUseIn(method));
}
markFieldAsKept(encodedField, KeepReason.reflectiveUseIn(method));
+ // Fields accessed by reflection is marked as both read and written.
+ if (encodedField.isStatic()) {
+ registerItemWithTargetAndContext(staticFieldsRead, encodedField.field, method);
+ registerItemWithTargetAndContext(staticFieldsWritten, encodedField.field, method);
+ } else {
+ registerItemWithTargetAndContext(instanceFieldsRead, encodedField.field, method);
+ registerItemWithTargetAndContext(instanceFieldsWritten, encodedField.field, method);
+ }
}
} else {
assert identifierItem.isDexMethod();
diff --git a/src/test/examples/classmerging/ArrayTypeCollisionTest.java b/src/test/examples/classmerging/ArrayTypeCollisionTest.java
index 9e4c9fd..c422946 100644
--- a/src/test/examples/classmerging/ArrayTypeCollisionTest.java
+++ b/src/test/examples/classmerging/ArrayTypeCollisionTest.java
@@ -12,11 +12,11 @@
}
private static void method(A[] obj) {
- System.out.println("In method(A[])");
+ System.out.println("In method(A[]), length: " + obj.length);
}
private static void method(B[] obj) {
- System.out.println("In method(B[])");
+ System.out.println("In method(B[]), length: " + obj.length);
}
// A cannot be merged into B because that would lead to a collision.
diff --git a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
index 3ad2482..0fd0354 100644
--- a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
@@ -101,6 +101,9 @@
// Ordered list of injar entries.
private List<Path> injars = new ArrayList<>();
+ // Ordered list of libraryjar entries.
+ private List<Path> libraryjars = new ArrayList<>();
+
// Proguard configuration file lines.
private List<String> config = new ArrayList<>();
@@ -138,10 +141,16 @@
command.add("-injars");
command.add(injar.toString());
}
- command.add("-libraryjars");
- // TODO(sgjesse): Add support for running with Android Jar.
- // command.add(ToolHelper.getAndroidJar(AndroidApiLevel.P).toString());
- command.add(ToolHelper.getJava8RuntimeJar().toString());
+ for (Path libraryjar : libraryjars) {
+ command.add("-libraryjars");
+ command.add(libraryjar.toString());
+ }
+ if (libraryjars.isEmpty()) {
+ command.add("-libraryjars");
+ // TODO(sgjesse): Add support for running with Android Jar.
+ // command.add(ToolHelper.getAndroidJar(AndroidApiLevel.P).toString());
+ command.add(ToolHelper.getJava8RuntimeJar().toString());
+ }
command.add("-include");
command.add(configFile.toString());
for (Path proguardConfigFile : proguardConfigFiles) {
@@ -198,7 +207,7 @@
injars.add(file);
} else {
throw new Unimplemented(
- "No support for adding paths directly (we need to compute the descriptor)");
+ "No support for adding class files directly (we need to compute the descriptor)");
}
}
return self();
@@ -207,7 +216,7 @@
@Override
public ProguardTestBuilder addProgramClassFileData(Collection<byte[]> classes) {
throw new Unimplemented(
- "No support for adding classfile data directly (we need to compute the descriptor)");
+ "No support for adding class files directly (we need to compute the descriptor)");
}
@Override
@@ -223,7 +232,15 @@
}
@Override
public ProguardTestBuilder addLibraryFiles(Collection<Path> files) {
- throw new Unimplemented("No support for adding library files");
+ for (Path file : files) {
+ if (FileUtils.isJarFile(file)) {
+ libraryjars.add(file);
+ } else {
+ throw new Unimplemented(
+ "No support for adding class files directly (we need to compute the descriptor)");
+ }
+ }
+ return self();
}
@Override
diff --git a/src/test/java/com/android/tools/r8/R8TestRunResult.java b/src/test/java/com/android/tools/r8/R8TestRunResult.java
index 42870fa..55730eb 100644
--- a/src/test/java/com/android/tools/r8/R8TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/R8TestRunResult.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.utils.graphinspector.GraphInspector;
import java.io.IOException;
import java.util.concurrent.ExecutionException;
+import java.util.function.Consumer;
public class R8TestRunResult extends TestRunResult<R8TestRunResult> {
@@ -50,6 +51,13 @@
return graphInspector.get();
}
+ public R8TestRunResult inspectGraph(Consumer<GraphInspector> consumer)
+ throws IOException, ExecutionException {
+ consumer.accept(graphInspector());
+ return self();
+ }
+
+
public String proguardMap() {
return proguardMap;
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index bab857c..e4a809c 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -614,10 +614,6 @@
}
public static Path getAndroidJar(AndroidApiLevel apiLevel) {
- if (apiLevel == AndroidApiLevel.P) {
- // TODO(mikaelpeltier) Android P does not yet have his android.jar use the O version
- apiLevel = AndroidApiLevel.O;
- }
String jar = String.format(
ANDROID_JAR_PATTERN,
(apiLevel == AndroidApiLevel.getDefault() ? DEFAULT_MIN_SDK : apiLevel).getLevel());
diff --git a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
index c4151d1..688c91c 100644
--- a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
@@ -206,7 +206,8 @@
" public static void main(...);",
"}",
"-neverinline class " + main + " {",
- " static void method(...);",
+ " static classmerging.A[] method(...);",
+ " static classmerging.B[] method(...);",
"}"));
}
diff --git a/src/test/java/com/android/tools/r8/deadcode/RemoveDeadArray.java b/src/test/java/com/android/tools/r8/deadcode/RemoveDeadArray.java
new file mode 100644
index 0000000..5de7de0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/deadcode/RemoveDeadArray.java
@@ -0,0 +1,92 @@
+// Copyright (c) 2019, 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.deadcode;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.code.Aput;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.Arrays;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RemoveDeadArray extends TestBase {
+
+ public static class TestClassWithCatch {
+ static {
+ try {
+ int[] foobar = new int[]{42, 42, 42, 42};
+ } catch (Exception ex) {
+ System.out.println("foobar");
+ }
+ }
+ }
+
+ public static class TestClass {
+ private static int[] foobar = new int[]{42, 42, 42, 42};
+ public static void main(java.lang.String[] args) {
+ ShouldGoAway[] willBeRemoved = new ShouldGoAway[4];
+ ShouldGoAway[] willAlsoBeRemoved = new ShouldGoAway[0];
+ System.out.println("foobar");
+ }
+
+ public static class ShouldGoAway { }
+ }
+
+ private Backend backend;
+
+ public RemoveDeadArray(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ // Todo(ricow): enable unused array removal for cf backend.
+ return new Backend[]{Backend.DEX};
+ }
+
+
+ @Test
+ public void testDeadArraysRemoved() throws Exception {
+ R8TestCompileResult result =
+ testForR8(backend)
+ .addProgramClasses(TestClass.class, TestClass.ShouldGoAway.class)
+ .addKeepMainRule(TestClass.class)
+ .compile();
+ CodeInspector inspector = result.inspector();
+ assertFalse(inspector.clazz(TestClass.class).clinit().isPresent());
+
+ MethodSubject main = inspector.clazz(TestClass.class).mainMethod();
+ main.streamInstructions().noneMatch(instructionSubject -> instructionSubject.isNewArray());
+ assertFalse(main.getMethod().getCode().asDexCode().toString().contains("NewArray"));
+ runOnArt(result.app, TestClass.class.getName());
+ }
+
+ @Test
+ public void testNotRemoveStaticCatch() throws Exception {
+ R8TestCompileResult result =
+ testForR8(backend)
+ .addProgramClasses(TestClassWithCatch.class)
+ .addKeepAllClassesRule()
+ .compile();
+ CodeInspector inspector = result.inspector();
+ MethodSubject clinit = inspector.clazz(TestClassWithCatch.class).clinit();
+ assertTrue(clinit.isPresent());
+ // Ensure that our optimization does not hit, we should still have 4 Aput instructions.
+ long aPutCount = Arrays.stream(clinit.getMethod().getCode().asDexCode().instructions)
+ .filter(instruction -> instruction instanceof Aput)
+ .count();
+ assertEquals(4, aPutCount);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/AvoidRTest.java b/src/test/java/com/android/tools/r8/naming/AvoidRTest.java
new file mode 100644
index 0000000..4633b36
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/AvoidRTest.java
@@ -0,0 +1,144 @@
+// Copyright (c) 2019, 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.DescriptorUtils.getSimpleClassNameFromDescriptor;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.R8TestBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminTestBase;
+import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableSet;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AvoidRTest extends JasminTestBase {
+ private Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public AvoidRTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test_withObfuscationDictionary() throws Exception {
+ Path dictionary = temp.newFile("dictionary.txt").toPath();
+ FileUtils.writeTextFile(dictionary, StringUtils.lines("P", "Q", "R", "S", "T"));
+ Set<String> expectedNames = ImmutableSet.of("P", "Q", "S", "T");
+
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ R8TestBuilder builder = testForR8(backend);
+ for (int i = 0; i < 4; i++) {
+ jasminBuilder.addClass("TopLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 4; i++) {
+ jasminBuilder.addClass("p1/SecondLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 4; i++) {
+ jasminBuilder.addClass("p1/p2/ThirdLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 4; i++) {
+ jasminBuilder.addClass("p2/SecondLevel" + Integer.toString(i));
+ }
+ builder.addProgramClassFileData(jasminBuilder.buildClasses());
+ Set<String> usedDescriptors = new HashSet<>();
+ builder.noTreeShaking()
+ .addKeepRules("-classobfuscationdictionary " + dictionary)
+ .compile()
+ .inspect(codeInspector -> {
+ codeInspector.forAllClasses(classSubject -> {
+ assertThat(classSubject, isRenamed());
+ String renamedDescriptor = classSubject.getFinalDescriptor();
+ assertTrue(usedDescriptors.add(renamedDescriptor));
+ assertNotEquals("R", getSimpleClassNameFromDescriptor(renamedDescriptor));
+ assertTrue(expectedNames.contains(getSimpleClassNameFromDescriptor(renamedDescriptor)));
+ });
+ });
+ }
+
+ @Test
+ public void test_withoutPackageHierarchy() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ R8TestBuilder builder = testForR8(backend);
+ for (int i = 0; i < 26 * 2; i++) {
+ jasminBuilder.addClass("TestClass" + Integer.toString(i));
+ }
+ builder.addProgramClassFileData(jasminBuilder.buildClasses());
+ Set<String> usedNames = new HashSet<>();
+ builder.noTreeShaking()
+ .compile()
+ .inspect(codeInspector -> {
+ codeInspector.forAllClasses(classSubject -> {
+ assertThat(classSubject, isRenamed());
+ assertTrue(usedNames.add(classSubject.getFinalName()));
+ assertNotEquals("R", classSubject.getFinalName());
+ });
+ });
+ assertTrue(usedNames.contains("Q"));
+ assertTrue(usedNames.contains("S"));
+ }
+
+ private void test_withPackageHierarchy(String keepRule) throws Exception {
+ R8TestBuilder builder = testForR8(backend);
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ for (int i = 0; i < 26 * 2; i++) {
+ jasminBuilder.addClass("TopLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 26 * 2; i++) {
+ jasminBuilder.addClass("p1/SecondLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 26 * 2; i++) {
+ jasminBuilder.addClass("p1/p2/ThirdLevel" + Integer.toString(i));
+ }
+ for (int i = 0; i < 26 * 2; i++) {
+ jasminBuilder.addClass("p2/SecondLevel" + Integer.toString(i));
+ }
+ builder.addProgramClassFileData(jasminBuilder.buildClasses());
+ Set<String> usedDescriptors = new HashSet<>();
+ builder.noTreeShaking()
+ .addKeepRules(keepRule)
+ .compile()
+ .inspect(codeInspector -> {
+ codeInspector.forAllClasses(classSubject -> {
+ assertThat(classSubject, isRenamed());
+ String renamedDescriptor = classSubject.getFinalDescriptor();
+ assertTrue(usedDescriptors.add(renamedDescriptor));
+ assertNotEquals("R", getSimpleClassNameFromDescriptor(renamedDescriptor));
+ });
+ });
+ }
+
+ @Test
+ public void test_withPackageHierarchy_default() throws Exception {
+ test_withPackageHierarchy("");
+ }
+
+ @Test
+ public void test_withPackageHierarchy_repackage() throws Exception {
+ // Repackage every class to the top-level.
+ test_withPackageHierarchy("-repackageclasses");
+ }
+
+ @Test
+ public void test_withPackageHierarchy_flatten() throws Exception {
+ // Repackage every package to the top-level.
+ test_withPackageHierarchy("-flattenpackagehierarchy");
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
index e8eb416..e518181 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
@@ -8,7 +8,9 @@
@Keep
public class KeptByFieldReflectionTest {
- public final int foo = 42;
+ // TODO(b/123262024): This field must be kept un-initialized. Otherwise the "-whyareyoukeeping"
+ // output tested will hit the initialization in <init> and not the reflective access.
+ public int foo;
public static void main(String[] args) throws Exception {
// Due to b/123210548 the object cannot be created by a reflective newInstance call.
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
index 6feaa35..62dcebe 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
@@ -19,7 +19,6 @@
import java.io.PrintStream;
import java.util.Arrays;
import java.util.Collection;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -31,7 +30,7 @@
private static final Class<?> CLASS = KeptByFieldReflectionTest.class;
private static final Collection<Class<?>> CLASSES = Arrays.asList(CLASS);
- private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 42");
+ private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 0");
private final String EXPECTED_WHYAREYOUKEEPING =
StringUtils.lines(
@@ -53,7 +52,6 @@
}
@Test
- @Ignore("b/123215165")
public void test() throws Exception {
MethodReference mainMethod = methodFromMethod(CLASS.getDeclaredMethod("main", String[].class));
FieldReference fooField = fieldFromField(CLASS.getDeclaredField("foo"));
diff --git a/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java b/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java
new file mode 100644
index 0000000..7aa573e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/reflection/FieldAccessTest.java
@@ -0,0 +1,161 @@
+// Copyright (c) 2019, 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.reflection;
+import static com.android.tools.r8.references.Reference.fieldFromField;
+import static com.android.tools.r8.references.Reference.methodFromMethod;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.FieldReference;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.graphinspector.GraphInspector.QueryNode;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class FieldAccessTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "{0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public FieldAccessTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ private boolean isInvokeGetField(InstructionSubject instruction) {
+ return
+ instruction.isInvoke()
+ && instruction.getMethod().qualifiedName().equals("java.lang.Class.getField");
+ }
+
+ private void runTest(Class<?> testClass) throws Exception {
+ MethodReference mainMethod =
+ methodFromMethod(testClass.getDeclaredMethod("main", String[].class));
+ FieldReference fooField = fieldFromField(testClass.getDeclaredField("foo"));
+
+ testForR8(Backend.DEX)
+ .enableGraphInspector()
+ .addProgramClasses(testClass)
+ .addKeepMainRule(testClass)
+ .run(testClass)
+ .inspectGraph(inspector -> {
+ // The only root should be the keep annotation rule.
+ assertEquals(1, inspector.getRoots().size());
+ QueryNode root = inspector.rule(Origin.unknown(), 1, 1).assertRoot();
+
+ inspector.method(mainMethod).assertNotRenamed().assertKeptBy(root);
+ inspector.field(fooField).assertRenamed();
+ })
+ .inspect(inspector -> {
+ Assert.assertTrue(
+ inspector
+ .clazz(testClass)
+ .uniqueMethodWithName("main")
+ .streamInstructions()
+ .anyMatch(this::isInvokeGetField));
+ })
+ .assertSuccessWithOutput("42");
+ }
+
+ @Test
+ public void reflectiveGet() throws Exception {
+ runTest(FieldAccessTestGet.class);
+ }
+
+ @Test
+ public void reflectiveGetStatic() throws Exception {
+ runTest(FieldAccessTestGetStatic.class);
+ }
+
+ @Test
+ public void reflectivePut() throws Exception {
+ runTest(FieldAccessTestPut.class);
+ }
+
+ @Test
+ public void reflectivePutStatic() throws Exception {
+ runTest(FieldAccessTestPutStatic.class);
+ }
+
+ @Test
+ public void reflectivePutGet() throws Exception {
+ runTest(FieldAccessTestPutGet.class);
+ }
+
+ @Test
+ public void reflectivePutGetStatic() throws Exception {
+ runTest(FieldAccessTestPutGetStatic.class);
+ }
+}
+
+class FieldAccessTestGet {
+
+ public int foo = 42;
+
+ public static void main(String[] args) throws Exception {
+ FieldAccessTestGet obj = new FieldAccessTestGet();
+ System.out.print(FieldAccessTestGet.class.getField("foo").getInt(obj));
+ }
+}
+
+class FieldAccessTestGetStatic {
+
+ public static int foo = 42;
+
+ public static void main(String[] args) throws Exception {
+ System.out.print(FieldAccessTestGetStatic.class.getField("foo").getInt(null));
+ }
+}
+
+class FieldAccessTestPut {
+
+ public int foo;
+
+ public static void main(String[] args) throws Exception {
+ FieldAccessTestPut obj = new FieldAccessTestPut();
+ FieldAccessTestPut.class.getField("foo").setInt(obj, 42);
+ System.out.print(42);
+ }
+}
+
+class FieldAccessTestPutStatic {
+
+ public static int foo;
+
+ public static void main(String[] args) throws Exception {
+ FieldAccessTestPutStatic.class.getField("foo").setInt(null, 42);
+ System.out.print(42);
+ }
+}
+
+class FieldAccessTestPutGet {
+
+ public int foo;
+
+ public static void main(String[] args) throws Exception {
+ FieldAccessTestPutGet obj = new FieldAccessTestPutGet();
+ FieldAccessTestPutGet.class.getField("foo").setInt(obj, 42);
+ System.out.print(FieldAccessTestPutGet.class.getField("foo").getInt(obj));
+ }
+}
+
+class FieldAccessTestPutGetStatic {
+
+ public static int foo;
+
+ public static void main(String[] args) throws Exception {
+ FieldAccessTestPutGetStatic.class.getField("foo").setInt(null, 42);
+ System.out.print(FieldAccessTestPutGetStatic.class.getField("foo").getInt(null));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
index ee2155e..375f0b5 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -23,6 +23,7 @@
import com.android.tools.r8.cf.code.CfLoad;
import com.android.tools.r8.cf.code.CfMonitor;
import com.android.tools.r8.cf.code.CfNew;
+import com.android.tools.r8.cf.code.CfNewArray;
import com.android.tools.r8.cf.code.CfNop;
import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.cf.code.CfReturn;
@@ -280,6 +281,11 @@
}
@Override
+ public boolean isNewArray() {
+ return instruction instanceof CfNewArray;
+ }
+
+ @Override
public int size() {
// TODO(b/122302789): CfInstruction#getSize()
throw new UnsupportedOperationException("CfInstruction doesn't have size yet.");
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
index 7051226..e2e801e 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -67,6 +67,7 @@
import com.android.tools.r8.code.MulIntLit8;
import com.android.tools.r8.code.MulLong;
import com.android.tools.r8.code.MulLong2Addr;
+import com.android.tools.r8.code.NewArray;
import com.android.tools.r8.code.NewInstance;
import com.android.tools.r8.code.Nop;
import com.android.tools.r8.code.PackedSwitch;
@@ -360,6 +361,11 @@
}
@Override
+ public boolean isNewArray() {
+ return instruction instanceof NewArray;
+ }
+
+ @Override
public boolean isMonitorEnter() {
return instruction instanceof MonitorEnter;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index 294cc8c..dd5af7c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -82,6 +82,8 @@
boolean isMultiplication();
+ boolean isNewArray();
+
boolean isMonitorEnter();
boolean isMonitorExit();
diff --git a/tools/as_utils.py b/tools/as_utils.py
index 93c910e..b4eb396 100644
--- a/tools/as_utils.py
+++ b/tools/as_utils.py
@@ -66,6 +66,44 @@
if (utils.R8_JAR not in line) and (utils.R8LIB_JAR not in line):
f.write(line)
+def GetMinAndCompileSdk(app, config, checkout_dir, apk_reference):
+ app_module = config.get('app_module', 'app')
+ build_gradle_file = os.path.join(checkout_dir, app_module, 'build.gradle')
+ assert os.path.isfile(build_gradle_file), (
+ 'Expected to find build.gradle file at {}'.format(build_gradle_file))
+
+ compile_sdk = None
+ min_sdks = []
+ target_sdk = None
+
+ with open(build_gradle_file) as f:
+ for line in f.readlines():
+ stripped = line.strip()
+ if stripped.startswith('compileSdkVersion '):
+ assert not compile_sdk
+ compile_sdk = int(stripped[len('compileSdkVersion '):])
+ if stripped.startswith('minSdkVersion '):
+ min_sdks.append(int(stripped[len('minSdkVersion '):]))
+ elif stripped.startswith('targetSdkVersion '):
+ assert not target_sdk
+ target_sdk = int(stripped[len('targetSdkVersion '):])
+
+ if len(min_sdks) == 1:
+ min_sdk = min_sdks[0]
+ else:
+ assert 'min_sdk' in config
+ min_sdk = config.get('min_sdk')
+
+ assert min_sdk, (
+ 'Expected to find `minSdkVersion` in {}'.format(build_gradle_file))
+ assert compile_sdk, (
+ 'Expected to find `compileSdkVersion` in {}'.format(build_gradle_file))
+
+ assert not target_sdk or target_sdk == compile_sdk, (
+ 'Expected `compileSdkVersion` and `targetSdkVersion` to be the same')
+
+ return (min_sdk, compile_sdk)
+
def IsGradleTaskName(x):
# Check that it is non-empty.
if not x:
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index a74f868..fcd0a76 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -78,6 +78,7 @@
'git_repo': 'https://github.com/sgjesse/tachiyomi.git',
'flavor': 'standard',
'releaseTarget': 'app:assembleRelease',
+ 'min_sdk': 16
},
'tivi': {
'app_id': 'app.tivi',
@@ -292,6 +293,10 @@
with open(proguard_config_file) as old:
assert(sum(1 for line in new) > sum(1 for line in old))
+ # Extract min-sdk and target-sdk
+ (min_sdk, compile_sdk) = as_utils.GetMinAndCompileSdk(app, config,
+ checkout_dir, apk_dest)
+
# Now rebuild generated apk.
previous_apk = apk_dest
for i in range(1, options.r8_compilation_steps):
@@ -300,7 +305,7 @@
checkout_dir, 'out', shrinker, 'app-release-{}.apk'.format(i))
RebuildAppWithShrinker(
previous_apk, recompiled_apk_dest, ext_proguard_config_file,
- shrinker)
+ shrinker, min_sdk, compile_sdk)
recompilation_result = {
'apk_dest': recompiled_apk_dest,
'build_status': 'success',
@@ -422,19 +427,28 @@
return (apk_dest, profile_dest_dir, proguard_config_dest)
-def RebuildAppWithShrinker(apk, apk_dest, proguard_config_file, shrinker):
+def RebuildAppWithShrinker(
+ apk, apk_dest, proguard_config_file, shrinker, min_sdk, compile_sdk):
assert 'r8' in shrinker
assert apk_dest.endswith('.apk')
# Compile given APK with shrinker to temporary zip file.
- api = 28 # TODO(christofferqa): Should be the one from build.gradle
- android_jar = os.path.join(utils.REPO_ROOT, utils.ANDROID_JAR.format(api=api))
+ android_jar = utils.get_android_jar(compile_sdk)
r8_jar = utils.R8LIB_JAR if IsMinifiedR8(shrinker) else utils.R8_JAR
zip_dest = apk_dest[:-4] + '.zip'
- cmd = ['java', '-ea:com.android.tools.r8...', '-cp', r8_jar,
- 'com.android.tools.r8.R8', '--release', '--pg-conf', proguard_config_file,
+ # TODO(christofferqa): Entry point should be CompatProguard if the shrinker
+ # is 'r8'.
+ entry_point = 'com.android.tools.r8.R8'
+
+ cmd = ['java', '-ea:com.android.tools.r8...', '-cp', r8_jar, entry_point,
+ '--release', '--min-api', str(min_sdk), '--pg-conf', proguard_config_file,
'--lib', android_jar, '--output', zip_dest, apk]
+
+ for android_optional_jar in utils.get_android_optional_jars(compile_sdk):
+ cmd.append('--lib')
+ cmd.append(android_optional_jar)
+
utils.PrintCmd(cmd)
subprocess.check_output(cmd)
diff --git a/tools/utils.py b/tools/utils.py
index f8b4469..72febdc 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -13,7 +13,8 @@
import tarfile
import tempfile
-ANDROID_JAR = 'third_party/android_jar/lib-v{api}/android.jar'
+ANDROID_JAR_DIR = 'third_party/android_jar/lib-v{api}'
+ANDROID_JAR = os.path.join(ANDROID_JAR_DIR, 'android.jar')
TOOLS_DIR = os.path.abspath(os.path.normpath(os.path.join(__file__, '..')))
REPO_ROOT = os.path.realpath(os.path.join(TOOLS_DIR, '..'))
THIRD_PARTY = os.path.join(REPO_ROOT, 'third_party')
@@ -308,8 +309,23 @@
if m is not None:
raise Exception("Do not use google JVM for benchmarking: " + version)
+def get_android_jar_dir(api):
+ return os.path.join(REPO_ROOT, ANDROID_JAR_DIR.format(api=api))
+
def get_android_jar(api):
return os.path.join(REPO_ROOT, ANDROID_JAR.format(api=api))
+def get_android_optional_jars(api):
+ android_optional_jars_dir = os.path.join(get_android_jar_dir(api), 'optional')
+ android_optional_jars = [
+ os.path.join(android_optional_jars_dir, 'android.test.base.jar'),
+ os.path.join(android_optional_jars_dir, 'android.test.mock.jar'),
+ os.path.join(android_optional_jars_dir, 'android.test.runner.jar'),
+ os.path.join(android_optional_jars_dir, 'org.apache.http.legacy.jar')
+ ]
+ return [
+ android_optional_jar for android_optional_jar in android_optional_jars
+ if os.path.isfile(android_optional_jar)]
+
def is_bot():
return 'BUILDBOT_BUILDERNAME' in os.environ
