Tests and fixes for missing classes from instance field instructions
Change-Id: I9e11248b9b7e6eee9276358e279b9e0a3a3dfde6
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
index 41ae033..24b17fb 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
@@ -381,7 +381,7 @@
// Map/required fields cannot be removed. Therefore, we mark such fields as both read and
// written such that we cannot optimize any field reads or writes.
enqueuer.registerReflectiveFieldAccess(valueStorage.getReference(), dynamicMethod);
- worklist.enqueueMarkReachableFieldAction(
+ worklist.enqueueMarkInstanceFieldAsReachableAction(
valueStorage, dynamicMethod, KeepReason.reflectiveUseIn(dynamicMethod));
valueStorageIsLive = true;
} else {
@@ -446,7 +446,7 @@
// Unconditionally register the hazzer and one-of proto fields as written from
// dynamicMethod().
if (enqueuer.registerReflectiveFieldWrite(newlyLiveField.getReference(), dynamicMethod)) {
- worklist.enqueueMarkReachableFieldAction(
+ worklist.enqueueMarkInstanceFieldAsReachableAction(
newlyLiveField, dynamicMethod, KeepReason.reflectiveUseIn(dynamicMethod));
}
}
@@ -566,7 +566,7 @@
}
if (enqueuer.registerReflectiveFieldWrite(oneOfField.getReference(), dynamicMethod)) {
- worklist.enqueueMarkReachableFieldAction(
+ worklist.enqueueMarkInstanceFieldAsReachableAction(
oneOfField, dynamicMethod, KeepReason.reflectiveUseIn(dynamicMethod));
}
}
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 b8a6cd5..73d5b00 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1451,11 +1451,10 @@
return;
}
- // Must mark the field as targeted even if it does not exist.
- markFieldAsTargeted(fieldReference, currentMethod);
-
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
if (resolutionResult.isFailedOrUnknownResolution()) {
+ // Must mark the field as targeted even if it does not exist.
+ markFieldAsTargeted(fieldReference, currentMethod);
noClassMerging.add(fieldReference.getHolderType());
return;
}
@@ -1481,15 +1480,12 @@
Log.verbose(getClass(), "Register Iget `%s`.", fieldReference);
}
- // If unused interface removal is enabled, then we won't necessarily mark the actual holder of
- // the field as live, if the holder is an interface.
- if (appView.options().enableUnusedInterfaceRemoval) {
- if (field.getReference() != fieldReference) {
- markTypeAsLive(field.getHolder(), currentMethod);
- }
+ if (field.getReference() != fieldReference) {
+ // Mark the initial resolution holder as live.
+ markTypeAsLive(resolutionResult.getInitialResolutionHolder(), currentMethod);
}
- workList.enqueueMarkReachableFieldAction(
+ workList.enqueueMarkInstanceFieldAsReachableAction(
field, currentMethod, KeepReason.fieldReferencedIn(currentMethod));
}
@@ -1507,11 +1503,10 @@
return;
}
- // Must mark the field as targeted even if it does not exist.
- markFieldAsTargeted(fieldReference, currentMethod);
-
FieldResolutionResult resolutionResult = resolveField(fieldReference, currentMethod);
if (resolutionResult.isFailedOrUnknownResolution()) {
+ // Must mark the field as targeted even if it does not exist.
+ markFieldAsTargeted(fieldReference, currentMethod);
noClassMerging.add(fieldReference.getHolderType());
return;
}
@@ -1537,16 +1532,13 @@
Log.verbose(getClass(), "Register Iput `%s`.", fieldReference);
}
- // If unused interface removal is enabled, then we won't necessarily mark the actual holder of
- // the field as live, if the holder is an interface.
- if (appView.options().enableUnusedInterfaceRemoval) {
- if (field.getReference() != fieldReference) {
- markTypeAsLive(field.getHolder(), currentMethod);
- }
+ if (field.getReference() != fieldReference) {
+ // Mark the initial resolution holder as live.
+ markTypeAsLive(resolutionResult.getInitialResolutionHolder(), currentMethod);
}
KeepReason reason = KeepReason.fieldReferencedIn(currentMethod);
- workList.enqueueMarkReachableFieldAction(field, currentMethod, reason);
+ workList.enqueueMarkInstanceFieldAsReachableAction(field, currentMethod, reason);
}
void traceStaticFieldRead(DexField field, ProgramMethod currentMethod) {
@@ -1608,9 +1600,9 @@
}
if (field.getReference() != fieldReference) {
- // Mark the non-rebound field access as targeted. Note that this should only be done if the
- // field is not a dead proto field (in which case we bail-out above).
- markFieldAsTargeted(fieldReference, currentMethod);
+ // Mark the initial resolution holder as live. Note that this should only be done if the field
+ // is not a dead proto field (in which case we bail-out above).
+ markTypeAsLive(resolutionResult.getInitialResolutionHolder(), currentMethod);
}
markStaticFieldAsLive(field, currentMethod);
@@ -1673,9 +1665,9 @@
}
if (field.getReference() != fieldReference) {
- // Mark the non-rebound field access as targeted. Note that this should only be done if the
- // field is not a dead proto field (in which case we bail-out above).
- markFieldAsTargeted(fieldReference, currentMethod);
+ // Mark the initial resolution holder as live. Note that this should only be done if the field
+ // is not a dead proto field (in which case we bail-out above).
+ markTypeAsLive(resolutionResult.getInitialResolutionHolder(), currentMethod);
}
markStaticFieldAsLive(field, currentMethod);
@@ -1743,6 +1735,13 @@
markTypeAsLive(clazz, reason);
}
+ private void markTypeAsLive(DexClass clazz, ProgramDefinition context) {
+ if (clazz.isProgramClass()) {
+ DexProgramClass programClass = clazz.asProgramClass();
+ markTypeAsLive(programClass, graphReporter.reportClassReferencedFrom(programClass, context));
+ }
+ }
+
private void markTypeAsLive(DexProgramClass clazz, ProgramDefinition context) {
markTypeAsLive(clazz, graphReporter.reportClassReferencedFrom(clazz, context));
}
@@ -2658,9 +2657,14 @@
}
}
+ private void markFieldAsTargeted(ProgramField field) {
+ markTypeAsLive(field.getHolder(), field);
+ markTypeAsLive(field.getType(), field);
+ }
+
private void markFieldAsTargeted(DexField field, ProgramMethod context) {
- markTypeAsLive(field.type, context);
- markTypeAsLive(field.holder, context);
+ markTypeAsLive(field.getHolderType(), context);
+ markTypeAsLive(field.getType(), context);
}
private void markStaticFieldAsLive(ProgramField field, ProgramMethod context) {
@@ -2669,9 +2673,8 @@
private void markStaticFieldAsLive(
ProgramField field, ProgramDefinition context, KeepReason reason) {
- // Mark the type live here, so that the class exists at runtime.
- markTypeAsLive(field.getHolder(), field);
- markTypeAsLive(field.getReference().type, field);
+ // Mark the field type and holder live here, so that they exist at runtime.
+ markFieldAsTargeted(field);
markDirectAndIndirectClassInitializersAsLive(field.getHolder());
@@ -2701,8 +2704,8 @@
private void markInstanceFieldAsLive(
ProgramField field, ProgramDefinition context, KeepReason reason) {
- markTypeAsLive(field.getHolder(), field);
- markTypeAsLive(field.getType(), field);
+ markFieldAsTargeted(field);
+
if (Log.ENABLED) {
Log.verbose(getClass(), "Adding instance field `%s` to live set.", field);
}
@@ -2830,8 +2833,7 @@
field.getHolder())) {
markInstanceFieldAsLive(field, context, reason);
} else {
- markTypeAsLive(field.getHolder(), field);
- markTypeAsLive(field.getReference().type, field);
+ markFieldAsTargeted(field);
// Add the field to the reachable set if the type later becomes instantiated.
reachableInstanceFields
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerWorklist.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerWorklist.java
index 07432c7..9ce5737 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerWorklist.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerWorklist.java
@@ -59,13 +59,13 @@
}
}
- static class MarkReachableFieldAction extends EnqueuerAction {
+ static class MarkInstanceFieldAsReachableAction extends EnqueuerAction {
private final ProgramField field;
// TODO(b/175854431): Avoid pushing context on worklist.
private final ProgramDefinition context;
private final KeepReason reason;
- public MarkReachableFieldAction(
+ public MarkInstanceFieldAsReachableAction(
ProgramField field, ProgramDefinition context, KeepReason reason) {
this.field = field;
this.context = context;
@@ -287,9 +287,9 @@
addToQueue(new MarkReachableSuperAction(method, from));
}
- public void enqueueMarkReachableFieldAction(
+ public void enqueueMarkInstanceFieldAsReachableAction(
ProgramField field, ProgramDefinition context, KeepReason reason) {
- addToQueue(new MarkReachableFieldAction(field, context, reason));
+ addToQueue(new MarkInstanceFieldAsReachableAction(field, context, reason));
}
// TODO(b/142378367): Context is the containing method that is cause of the instantiation.
diff --git a/src/main/java/com/android/tools/r8/shaking/MissingClassesDiagnostic.java b/src/main/java/com/android/tools/r8/shaking/MissingClassesDiagnostic.java
index 2e59a42..e6bd2e2 100644
--- a/src/main/java/com/android/tools/r8/shaking/MissingClassesDiagnostic.java
+++ b/src/main/java/com/android/tools/r8/shaking/MissingClassesDiagnostic.java
@@ -58,16 +58,16 @@
}
String getReferencedFromMessageSuffix(ClassReference missingClass) {
- if (!methodContexts.isEmpty()) {
- return " (referenced from: "
- + MethodReferenceUtils.toSourceString(methodContexts.iterator().next())
- + ")";
- }
if (!fieldContexts.isEmpty()) {
return " (referenced from: "
+ FieldReferenceUtils.toSourceString(fieldContexts.iterator().next())
+ ")";
}
+ if (!methodContexts.isEmpty()) {
+ return " (referenced from: "
+ + MethodReferenceUtils.toSourceString(methodContexts.iterator().next())
+ + ")";
+ }
// TODO(b/175543745): The legacy reporting is context insensitive, and therefore uses the
// missing classes as their own context. Once legacy reporting is removed, this should be
// simplified to taking the first context.
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToExistingFieldTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToExistingFieldTest.java
new file mode 100644
index 0000000..6c7f620
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToExistingFieldTest.java
@@ -0,0 +1,65 @@
+// Copyright (c) 2020, 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.missingclasses;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.FieldReference;
+import com.android.tools.r8.references.Reference;
+import org.junit.Test;
+
+/**
+ * If a field reference that refers to a missing class resolves to a definition, then the field
+ * definition is to be blamed.
+ */
+public class MissingClassReferencedFromInstanceGetToExistingFieldTest
+ extends MissingClassesTestBase {
+
+ private static final FieldReference referencedFrom =
+ Reference.field(
+ Reference.classFromClass(Main.class),
+ "field",
+ Reference.classFromClass(MissingClass.class));
+
+ public MissingClassReferencedFromInstanceGetToExistingFieldTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testNoRules() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom));
+ }
+
+ @Test
+ public void testDontWarnMainClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class));
+ }
+
+ @Test
+ public void testDontWarnMissingClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(MissingClass.class));
+ }
+
+ @Test
+ public void testIgnoreWarnings() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class,
+ diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom),
+ addIgnoreWarnings());
+ }
+
+ static class Main {
+
+ public MissingClass field;
+
+ public static void main(String[] args) {
+ MissingClass ignore = new Main().field;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToMissingFieldTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToMissingFieldTest.java
new file mode 100644
index 0000000..851d65d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstanceGetToMissingFieldTest.java
@@ -0,0 +1,61 @@
+// Copyright (c) 2020, 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.missingclasses;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.MethodReferenceUtils;
+import org.junit.Test;
+
+/**
+ * If a field reference that refers to a missing class does not resolve, then the enclosing method
+ * is to be blamed.
+ */
+public class MissingClassReferencedFromInstanceGetToMissingFieldTest
+ extends MissingClassesTestBase {
+
+ private static final MethodReference referencedFrom =
+ MethodReferenceUtils.mainMethod(Reference.classFromClass(Main.class));
+
+ public MissingClassReferencedFromInstanceGetToMissingFieldTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testNoRules() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom));
+ }
+
+ @Test
+ public void testDontWarnMainClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class));
+ }
+
+ @Test
+ public void testDontWarnMissingClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(MissingClass.class));
+ }
+
+ @Test
+ public void testIgnoreWarnings() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class,
+ diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom),
+ addIgnoreWarnings());
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ int ignore = new MissingClass().field;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToExistingFieldTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToExistingFieldTest.java
new file mode 100644
index 0000000..b69625e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToExistingFieldTest.java
@@ -0,0 +1,65 @@
+// Copyright (c) 2020, 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.missingclasses;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.FieldReference;
+import com.android.tools.r8.references.Reference;
+import org.junit.Test;
+
+/**
+ * If a field reference that refers to a missing class resolves to a definition, then the field
+ * definition is to be blamed.
+ */
+public class MissingClassReferencedFromInstancePutToExistingFieldTest
+ extends MissingClassesTestBase {
+
+ private static final FieldReference referencedFrom =
+ Reference.field(
+ Reference.classFromClass(Main.class),
+ "field",
+ Reference.classFromClass(MissingClass.class));
+
+ public MissingClassReferencedFromInstancePutToExistingFieldTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testNoRules() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom));
+ }
+
+ @Test
+ public void testDontWarnMainClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class));
+ }
+
+ @Test
+ public void testDontWarnMissingClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(MissingClass.class));
+ }
+
+ @Test
+ public void testIgnoreWarnings() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class,
+ diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom),
+ addIgnoreWarnings());
+ }
+
+ static class Main {
+
+ MissingClass field;
+
+ public static void main(String[] args) {
+ new Main().field = null;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToMissingFieldTest.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToMissingFieldTest.java
new file mode 100644
index 0000000..ec615f1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassReferencedFromInstancePutToMissingFieldTest.java
@@ -0,0 +1,61 @@
+// Copyright (c) 2020, 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.missingclasses;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.MethodReferenceUtils;
+import org.junit.Test;
+
+/**
+ * If a field reference that refers to a missing class does not resolve, then the enclosing method
+ * is to be blamed.
+ */
+public class MissingClassReferencedFromInstancePutToMissingFieldTest
+ extends MissingClassesTestBase {
+
+ private static final MethodReference referencedFrom =
+ MethodReferenceUtils.mainMethod(Reference.classFromClass(Main.class));
+
+ public MissingClassReferencedFromInstancePutToMissingFieldTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void testNoRules() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, diagnostics -> inspectDiagnosticsWithNoRules(diagnostics, referencedFrom));
+ }
+
+ @Test
+ public void testDontWarnMainClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(Main.class));
+ }
+
+ @Test
+ public void testDontWarnMissingClass() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class, TestDiagnosticMessages::assertNoMessages, addDontWarn(MissingClass.class));
+ }
+
+ @Test
+ public void testIgnoreWarnings() throws Exception {
+ compileWithExpectedDiagnostics(
+ Main.class,
+ diagnostics -> inspectDiagnosticsWithIgnoreWarnings(diagnostics, referencedFrom),
+ addIgnoreWarnings());
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ new MissingClass().field = 42;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java b/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java
index 08cb3de..191c28d 100644
--- a/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java
+++ b/src/test/java/com/android/tools/r8/missingclasses/MissingClassesTestBase.java
@@ -34,6 +34,8 @@
static class MissingClass {
static int FIELD;
+
+ int field;
}
private final TestParameters parameters;
