Check for missing reservation state for classes not visited
Bug: 173184123
Bug: 172895918
Change-Id: Ia7f205c8d89aaaea817a205401b9129926dba549
diff --git a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
index e6fcce6..b1805c2 100644
--- a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
@@ -104,7 +104,7 @@
// Used for iterating the sub trees that has this node as root.
final Set<DexType> children = new HashSet<>();
// Collection of the frontier reservation types and the interface type itself.
- final Set<DexType> reservationTypes = new HashSet<>();
+ private final Set<DexType> reservationTypes = new HashSet<>();
InterfaceReservationState(DexClass iface) {
this.iface = iface;
@@ -139,6 +139,10 @@
return isReserved == null ? null : method.getName();
}
+ void addReservationType(DexType type) {
+ this.reservationTypes.add(type);
+ }
+
void reserveName(DexString reservedName, DexEncodedMethod method) {
forAll(
s -> {
@@ -162,7 +166,7 @@
}
return null;
});
- return result == null ? true : result;
+ return result == null || result;
}
void addRenaming(DexString newName, DexEncodedMethod method) {
@@ -304,7 +308,7 @@
}
return null;
});
- return result == null ? true : result;
+ return result == null || result;
}
void addRenaming(DexString newName, MethodNameMinifier.State minifierState) {
@@ -398,7 +402,7 @@
assert iface.isInterface();
minifierState.allocateReservationStateAndReserve(iface.type, iface.type);
InterfaceReservationState iFaceState = new InterfaceReservationState(iface);
- iFaceState.reservationTypes.add(iface.type);
+ iFaceState.addReservationType(iface.type);
interfaceStateMap.put(iface.type, iFaceState);
}
}
@@ -644,8 +648,12 @@
InterfaceReservationState iState = interfaceStateMap.get(directlyImplemented);
if (iState != null) {
DexType frontierType = minifierState.getFrontier(clazz.type);
- assert minifierState.getReservationState(frontierType) != null;
- iState.reservationTypes.add(frontierType);
+ iState.addReservationType(frontierType);
+ // The reservation state should already be added, but if a class is extending
+ // an interface, we will not visit the class during the sub-type traversel
+ if (minifierState.getReservationState(clazz.type) == null) {
+ minifierState.allocateReservationStateAndReserve(clazz.type, frontierType);
+ }
}
}
}
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 589f1bd..35c201a 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -607,6 +607,33 @@
}
}
+ private void warnIfClassExtendsInterfaceOrImplementsClass(DexProgramClass clazz) {
+ if (clazz.superType != null) {
+ DexClass superClass = definitionFor(clazz.superType);
+ if (superClass != null && superClass.isInterface()) {
+ options.reporter.warning(
+ new StringDiagnostic(
+ "Class "
+ + clazz.toSourceString()
+ + " extends "
+ + superClass.toSourceString()
+ + " which is an interface"));
+ }
+ }
+ for (DexType iface : clazz.interfaces.values) {
+ DexClass ifaceClass = definitionFor(iface);
+ if (ifaceClass != null && !ifaceClass.isInterface()) {
+ options.reporter.warning(
+ new StringDiagnostic(
+ "Class "
+ + clazz.toSourceString()
+ + " implements "
+ + ifaceClass.toSourceString()
+ + " which is not an interface"));
+ }
+ }
+ }
+
private void enqueueRootItems(Map<DexReference, Set<ProguardKeepRuleBase>> items) {
items.entrySet().forEach(this::enqueueRootItem);
}
@@ -1556,6 +1583,9 @@
markTypeAsLive(holder.superType, reason);
}
+ // Warn if the class extends an interface or implements a class
+ warnIfLibraryExtendsInterfaceOrImplementsClass(holder);
+
// If this is an interface that has just become live, then report previously seen but unreported
// implemented-by edges.
transitionUnusedInterfaceToLive(holder);
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java b/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java
index ea6b2f7..a7c0baf 100644
--- a/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java
+++ b/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java
@@ -5,7 +5,6 @@
package com.android.tools.r8;
import com.android.tools.r8.ToolHelper.ProcessResult;
-import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.io.IOException;
@@ -41,14 +40,6 @@
return proguardMap;
}
- public String stdout() {
- return processResult.stdout;
- }
-
- public String stderr() {
- return processResult.stdout;
- }
-
@Override
public ExternalR8TestCompileResult self() {
return this;
@@ -61,12 +52,12 @@
@Override
public String getStdout() {
- throw new Unimplemented("Unexpected attempt to access stdout from external R8");
+ return processResult.stdout;
}
@Override
public String getStderr() {
- throw new Unimplemented("Unexpected attempt to access stderr from external R8");
+ return processResult.stderr;
}
@Override
diff --git a/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapCurrentEqualityTest.java b/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapCurrentEqualityTest.java
index 4baa16c..d67f0ce 100644
--- a/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapCurrentEqualityTest.java
+++ b/src/test/java/com/android/tools/r8/cf/bootstrap/BootstrapCurrentEqualityTest.java
@@ -256,8 +256,8 @@
.setMode(mode)
.compile();
// Check that the process outputs (exit code, stdout, stderr) are the same.
- assertEquals(result.stdout(), runR8R8.stdout());
- assertEquals(result.stderr(), runR8R8.stderr());
+ assertEquals(result.getStdout(), runR8R8.getStdout());
+ assertEquals(result.getStderr(), runR8R8.getStderr());
// Check that the output jars are the same.
assertProgramsEqual(result.outputJar(), runR8R8.outputJar());
}
