Merge "Revert "Assert that definitionFor is only called on non-null class types.""
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
index e415b0f..a45064e 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -119,10 +119,8 @@
// For mapping invoke virtual instruction to target methods.
public Set<DexEncodedMethod> lookupVirtualTargets(DexMethod method) {
- if (method.holder.isArrayType()) {
- assert method.name == dexItemFactory.cloneMethodName;
- return null;
- }
+ Set<DexEncodedMethod> result = new HashSet<>();
+ // First add the target for receiver type method.type.
DexClass root = definitionFor(method.holder);
if (root == null) {
// type specified in method does not have a materialized class.
@@ -133,8 +131,6 @@
// This will fail at runtime.
return null;
}
- // First add the target for receiver type method.type.
- Set<DexEncodedMethod> result = new HashSet<>();
topTargets.forEachTarget(result::add);
// Add all matching targets from the subclass hierarchy.
for (DexType type : subtypes(method.holder)) {
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 1e125d9..09094aa 100644
--- a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
@@ -45,7 +45,6 @@
@Override
public DexClass definitionFor(DexType type) {
- assert type.isClassType() : "Cannot lookup definition for type: " + type;
DexClass result = programClasses.get(type);
if (result == null) {
result = libraryClasses.get(type);
diff --git a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
index b236428..6dea825 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
@@ -42,7 +42,9 @@
@Override
public DexClass definitionFor(DexType type) {
- assert type.isClassType() : "Cannot lookup definition for type: " + type;
+ if (type == null) {
+ return null;
+ }
DexClass clazz = programClasses.get(type);
if (clazz == null && classpathClasses != null) {
clazz = classpathClasses.get(type);
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java b/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java
index 75a9726..53a6245 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java
@@ -78,7 +78,7 @@
return true;
}
- if (fieldType.isClassType() && valueType.isReference()) {
+ if (valueType.isReference()) {
// Interface types are treated like Object according to the JVM spec.
// https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.10.1.2-100
DexClass clazz = appInfo.definitionFor(instruction.getField().type);
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstClass.java b/src/main/java/com/android/tools/r8/ir/code/ConstClass.java
index 7610f9c..bd57f29 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ConstClass.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ConstClass.java
@@ -91,9 +91,6 @@
// A const-class instruction can be dead code only if the resulting program is known to contain
// the class mentioned.
DexType baseType = clazz.toBaseType(appInfo.dexItemFactory);
- if (baseType.isPrimitiveType()) {
- return true;
- }
DexClass holder = appInfo.definitionFor(baseType);
return holder != null && holder.isProgramClass();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 1eae24e..7758d83 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -2156,9 +2156,6 @@
private boolean isTypeInaccessibleInCurrentContext(DexType type, DexEncodedMethod context) {
DexType baseType = type.toBaseType(appInfo.dexItemFactory);
- if (baseType.isPrimitiveType()) {
- return false;
- }
DexClass clazz = definitionFor(baseType);
if (clazz == null) {
// Conservatively say yes.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index cdf37c4..7375303 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -113,9 +113,6 @@
eligibleClass =
root.isNewInstance() ? root.asNewInstance().clazz : root.asStaticGet().getField().type;
- if (!eligibleClass.isClassType()) {
- return false;
- }
eligibleClassDefinition = appInfo.definitionFor(eligibleClass);
if (eligibleClassDefinition == null && lambdaRewriter != null) {
// Check if the class is synthesized for a desugared lambda
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 8560ba7..adc6979 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2414,10 +2414,6 @@
// https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-6.html#jvms-6.5.invokevirtual
assert method != null;
assert refinedReceiverType.isSubtypeOf(method.holder, this);
- if (method.holder.isArrayType()) {
- assert method.name == dexItemFactory.cloneMethodName;
- return null;
- }
DexClass holder = definitionFor(method.holder);
if (holder == null || holder.isLibraryClass() || holder.isInterface()) {
return null;
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java b/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java
index 71b3d53..543dcc4 100644
--- a/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java
+++ b/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.UseRegistry;
+import com.google.common.collect.ImmutableSet;
import java.util.Set;
import java.util.function.Consumer;
@@ -60,23 +61,35 @@
boolean value = false;
}
+ public static boolean hasReferencesOutside(
+ AppInfoWithSubtyping appInfo, DexProgramClass clazz, Set<DexType> types) {
+ BooleanBox result = new BooleanBox();
+
+ new MainDexDirectReferenceTracer(appInfo, type -> {
+ if (!types.contains(type)) {
+ DexClass cls = appInfo.definitionFor(type);
+ if (cls != null && !cls.isLibraryClass()) {
+ result.value = true;
+ }
+ }
+ }).run(ImmutableSet.of(clazz.type));
+
+ return result.value;
+ }
+
public static boolean hasReferencesOutsideFromCode(
AppInfoWithSubtyping appInfo, DexEncodedMethod method, Set<DexType> classes) {
BooleanBox result = new BooleanBox();
- new MainDexDirectReferenceTracer(
- appInfo,
- type -> {
- DexType baseType = type.toBaseType(appInfo.dexItemFactory);
- if (baseType.isClassType() && !classes.contains(baseType)) {
- DexClass cls = appInfo.definitionFor(baseType);
- if (cls != null && !cls.isLibraryClass()) {
- result.value = true;
- }
- }
- })
- .runOnCode(method);
+ new MainDexDirectReferenceTracer(appInfo, type -> {
+ if (!classes.contains(type)) {
+ DexClass cls = appInfo.definitionFor(type);
+ if (cls != null && !cls.isLibraryClass()) {
+ result.value = true;
+ }
+ }
+ }).runOnCode(method);
return result.value;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index ff196e3..898d06c 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -843,9 +843,6 @@
}
private void includeDescriptor(DexDefinition item, DexType type, ProguardKeepRule context) {
- if (type.isVoidType()) {
- return;
- }
if (type.isArrayType()) {
type = type.toBaseType(application.dexItemFactory);
}
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 15dcafa..d793e3b 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -336,16 +336,15 @@
}
private void markTypeAsPinned(DexType type, AbortReason reason) {
- DexType baseType = type.toBaseType(appInfo.dexItemFactory);
- if (!baseType.isClassType() || appInfo.isPinned(baseType)) {
+ if (appInfo.isPinned(type)) {
// We check for the case where the type is pinned according to appInfo.isPinned,
// so we only need to add it here if it is not the case.
return;
}
- DexClass clazz = appInfo.definitionFor(baseType);
+ DexClass clazz = appInfo.definitionFor(type);
if (clazz != null && clazz.isProgramClass()) {
- boolean changed = pinnedTypes.add(baseType);
+ boolean changed = pinnedTypes.add(type);
if (Log.ENABLED) {
if (changed && isMergeCandidate(clazz.asProgramClass(), ImmutableSet.of())) {
@@ -744,6 +743,10 @@
return false;
}
+ private boolean hasReferencesOutside(DexProgramClass clazz, Set<DexType> types) {
+ return MainDexDirectReferenceTracer.hasReferencesOutside(appInfo, clazz, types);
+ }
+
private void mergeClassIfPossible(DexProgramClass clazz) {
if (!mergeCandidates.contains(clazz)) {
return;
diff --git a/src/test/examples/classmerging/PinnedArrayParameterTypesTest.java b/src/test/examples/classmerging/PinnedArrayParameterTypesTest.java
deleted file mode 100644
index 4a8b5be..0000000
--- a/src/test/examples/classmerging/PinnedArrayParameterTypesTest.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// 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 classmerging;
-
-import java.lang.reflect.Method;
-
-public class PinnedArrayParameterTypesTest {
-
- public static void main(String[] args) throws Exception {
- for (Method method : TestClass.class.getMethods()) {
- if (method.getName().equals("method")) {
- Class<?> parameterType = method.getParameterTypes()[0];
-
- // Should print classmerging.PinnedArrayParameterTypesTest$Interface when
- // -keepparameternames is used.
- System.out.println(parameterType.getName());
-
- method.invoke(null, new Object[]{ new InterfaceImpl[]{ new InterfaceImpl() } });
- break;
- }
- }
- }
-
- public interface Interface {
-
- void foo();
- }
-
- public static class InterfaceImpl implements Interface {
-
- @Override
- public void foo() {
- System.out.println("In InterfaceImpl.foo()");
- }
- }
-
- public static class TestClass {
-
- // This method has been kept explicitly by a keep rule. Therefore, since -keepparameternames is
- // used, Interface must not be merged into InterfaceImpl.
- public static void method(Interface[] obj) {
- obj[0].foo();
- }
- }
-}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index c75058d..e3e0006 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -43,12 +43,6 @@
-keep public class classmerging.PinnedParameterTypesTest$TestClass {
public static void method(...);
}
--keep public class classmerging.PinnedArrayParameterTypesTest {
- public static void main(...);
-}
--keep public class classmerging.PinnedArrayParameterTypesTest$TestClass {
- public static void method(...);
-}
-keep public class classmerging.ProguardFieldMapTest {
public static void main(...);
}
diff --git a/src/test/java/com/android/tools/r8/D8TestBuilder.java b/src/test/java/com/android/tools/r8/D8TestBuilder.java
index 37e4dec..88d077b 100644
--- a/src/test/java/com/android/tools/r8/D8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/D8TestBuilder.java
@@ -3,18 +3,13 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
-import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer;
import com.android.tools.r8.D8Command.Builder;
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.utils.AndroidApp;
-import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
-import java.io.IOException;
import java.nio.file.Path;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.List;
import java.util.function.Consumer;
import java.util.function.Supplier;
@@ -22,9 +17,6 @@
extends TestCompilerBuilder<
D8Command, Builder, D8TestCompileResult, D8TestRunResult, D8TestBuilder> {
- // Consider an in-order collection of both class and files on the classpath.
- private List<Class<?>> classpathClasses = new ArrayList<>();
-
private D8TestBuilder(TestState state, Builder builder) {
super(state, builder, Backend.DEX);
}
@@ -42,29 +34,6 @@
D8TestCompileResult internalCompile(
Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException {
- if (!classpathClasses.isEmpty()) {
- Path cp;
- try {
- cp = getState().getNewTempFolder().resolve("cp.jar");
- } catch (IOException e) {
- throw builder.getReporter().fatalError("Failed to create temp file for classpath archive");
- }
- ArchiveConsumer archiveConsumer = new ArchiveConsumer(cp);
- for (Class<?> classpathClass : classpathClasses) {
- try {
- archiveConsumer.accept(
- ByteDataView.of(ToolHelper.getClassAsBytes(classpathClass)),
- DescriptorUtils.javaTypeToDescriptor(classpathClass.getTypeName()),
- builder.getReporter());
- } catch (IOException e) {
- builder
- .getReporter()
- .error("Failed to read bytes for classpath class: " + classpathClass.getTypeName());
- }
- }
- archiveConsumer.finished(builder.getReporter());
- builder.addClasspathFiles(cp);
- }
ToolHelper.runD8(builder, optionsConsumer);
return new D8TestCompileResult(getState(), app.get());
}
@@ -74,8 +43,7 @@
}
public D8TestBuilder addClasspathClasses(Collection<Class<?>> classes) {
- classpathClasses.addAll(classes);
- return self();
+ return addClasspathFiles(getFilesForClasses(classes));
}
public D8TestBuilder addClasspathFiles(Path... files) {
diff --git a/src/test/java/com/android/tools/r8/RegressionForPrimitiveDefinitionForLookup.java b/src/test/java/com/android/tools/r8/RegressionForPrimitiveDefinitionForLookup.java
deleted file mode 100644
index b43bb8b..0000000
--- a/src/test/java/com/android/tools/r8/RegressionForPrimitiveDefinitionForLookup.java
+++ /dev/null
@@ -1,43 +0,0 @@
-// 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;
-
-import com.android.tools.r8.utils.StringUtils;
-import org.junit.Test;
-
-class Tester {
- public int foo() {
- float[][][] fs = new float[1][2][3];
- return fs.length;
- }
-
- public static void main(String[] args) {
- System.out.println(new Tester().foo());
- }
-}
-
-// The DirectoryClasspathProvider asserts lookups are reference types which witnessed the issue.
-public class RegressionForPrimitiveDefinitionForLookup extends TestBase {
-
- public final Class<Tester> CLASS = Tester.class;
- public String EXPECTED = StringUtils.lines("1");
-
- @Test
- public void testWithArchiveClasspath() throws Exception {
- testForD8()
- .addClasspathClasses(CLASS)
- .addProgramClasses(CLASS)
- .run(CLASS)
- .assertSuccessWithOutput(EXPECTED);
- }
-
- @Test
- public void testWithDirectoryClasspath() throws Exception {
- testForD8()
- .addClasspathFiles(ToolHelper.getClassPathForTests())
- .addProgramClasses(CLASS)
- .run(CLASS)
- .assertSuccessWithOutput(EXPECTED);
- }
-}
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 1836a29..c4151d1 100644
--- a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
@@ -463,29 +463,6 @@
}
@Test
- public void testPinnedArrayParameterTypes() throws Throwable {
- String main = "classmerging.PinnedArrayParameterTypesTest";
- Path[] programFiles =
- new Path[] {
- CF_DIR.resolve("PinnedArrayParameterTypesTest.class"),
- CF_DIR.resolve("PinnedArrayParameterTypesTest$Interface.class"),
- CF_DIR.resolve("PinnedArrayParameterTypesTest$InterfaceImpl.class"),
- CF_DIR.resolve("PinnedArrayParameterTypesTest$TestClass.class")
- };
- Set<String> preservedClassNames =
- ImmutableSet.of(
- "classmerging.PinnedArrayParameterTypesTest",
- "classmerging.PinnedArrayParameterTypesTest$Interface",
- "classmerging.PinnedArrayParameterTypesTest$InterfaceImpl",
- "classmerging.PinnedArrayParameterTypesTest$TestClass");
- runTest(
- main,
- programFiles,
- preservedClassNames::contains,
- getProguardConfig(EXAMPLE_KEEP, "-keepparameternames"));
- }
-
- @Test
public void testProguardFieldMap() throws Throwable {
String main = "classmerging.ProguardFieldMapTest";
Path[] programFiles =
