Extend visibility checks to include package names
Change-Id: I18b7fca5296b4085d3992465a018c929e94e4e65
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 10620fc6..ee86680 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -109,25 +109,14 @@
}
public boolean isMoreVisibleThan(
- AccessFlags other, String packageNameThis, String packageNameOther) {
+ AccessFlags<?> other, String packageNameThis, String packageNameOther) {
int visibilityOrdinal = getVisibilityOrdinal();
if (visibilityOrdinal > other.getVisibilityOrdinal()) {
return true;
}
- if (visibilityOrdinal == other.getVisibilityOrdinal()
+ return visibilityOrdinal == other.getVisibilityOrdinal()
&& isVisibilityDependingOnPackage()
- && !packageNameThis.equals(packageNameOther)) {
- return true;
- }
- return false;
- }
-
- public boolean isAtLeastAsVisibleAs(AccessFlags other) {
- return getVisibilityOrdinal() >= other.getVisibilityOrdinal();
- }
-
- public boolean isSameVisibility(AccessFlags other) {
- return getVisibilityOrdinal() == other.getVisibilityOrdinal();
+ && !packageNameThis.equals(packageNameOther);
}
public int getVisibilityOrdinal() {
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
index 42f5d96..369e60c 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -536,7 +536,7 @@
assert potentialHolder.isInterface();
for (DexEncodedMethod virtualMethod : potentialHolder.virtualMethods()) {
if (virtualMethod.getReference().match(method.getReference())
- && virtualMethod.accessFlags.isSameVisibility(method.accessFlags)) {
+ && virtualMethod.isSameVisibility(method)) {
return true;
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index d68d906..985e215 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -569,6 +569,40 @@
return isStatic();
}
+ public boolean isAtLeastAsVisibleAsOtherInSameHierarchy(
+ DexEncodedMethod other, AppView<? extends AppInfoWithClassHierarchy> appView) {
+ assert getReference().getProto() == other.getReference().getProto();
+ assert appView.isSubtype(getHolderType(), other.getHolderType()).isTrue()
+ || appView.isSubtype(other.getHolderType(), getHolderType()).isTrue();
+ AccessFlags<MethodAccessFlags> accessFlags = getAccessFlags();
+ AccessFlags<?> otherAccessFlags = other.getAccessFlags();
+ if (accessFlags.getVisibilityOrdinal() < otherAccessFlags.getVisibilityOrdinal()) {
+ return false;
+ } else if (accessFlags.isPrivate()) {
+ return getHolderType() == other.getHolderType();
+ } else if (accessFlags.isPublic() || accessFlags.isProtected()) {
+ return true;
+ } else {
+ assert accessFlags.isPackagePrivate();
+ return getHolderType().getPackageName().equals(other.getHolderType().getPackageName());
+ }
+ }
+
+ public boolean isSameVisibility(DexEncodedMethod other) {
+ AccessFlags<MethodAccessFlags> accessFlags = getAccessFlags();
+ if (accessFlags.getVisibilityOrdinal() != other.getAccessFlags().getVisibilityOrdinal()) {
+ return false;
+ }
+ if (accessFlags.isPublic()) {
+ return true;
+ }
+ if (accessFlags.isPrivate()) {
+ return getHolderType() == other.getHolderType();
+ }
+ assert accessFlags.isVisibilityDependingOnPackage();
+ return getHolderType().getPackageName().equals(other.getHolderType().getPackageName());
+ }
+
/**
* Returns true if this method is synthetic.
*/
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
index 8a023db..08f3912 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
@@ -395,8 +395,7 @@
.isPossiblyFalse()
|| !newResolutionResult
.getResolvedMethod()
- .getAccessFlags()
- .isAtLeastAsVisibleAs(resolutionResult.getResolvedMethod().getAccessFlags())
+ .isAtLeastAsVisibleAsOtherInSameHierarchy(resolutionResult.getResolvedMethod(), appView)
// isOverriding expects both arguments to be not private.
|| (!newResolutionResult.getResolvedMethod().isPrivateMethod()
&& !isOverriding(
diff --git a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
index d0066e0..8847f5f 100644
--- a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
@@ -59,18 +59,10 @@
if (targetMethod == null) {
return null;
}
- if (method.getAccessFlags().isPublic()) {
- if (!targetMethod.getAccessFlags().isPublic()) {
- return null;
- }
- } else {
- if (targetMethod.getAccessFlags().isProtected()
- && !targetMethod.getHolderType().isSamePackage(method.getHolderType())) {
- return null;
- }
- if (targetMethod.getAccessFlags().isPrivate()) {
- return null;
- }
+ if (!targetMethod
+ .getDefinition()
+ .isAtLeastAsVisibleAsOtherInSameHierarchy(method.getDefinition(), appView)) {
+ return null;
}
if (definition.isStatic()
&& method.getHolder().hasClassInitializer()
diff --git a/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java b/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java
index f1fc012..58faf90 100644
--- a/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.shaking.b136195382;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -13,8 +12,6 @@
import com.android.tools.r8.shaking.b136195382.package2.Main;
import com.android.tools.r8.shaking.b136195382.package2.SubFactory;
import com.android.tools.r8.shaking.b136195382.package2.SubService;
-import java.io.IOException;
-import java.util.concurrent.ExecutionException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -35,8 +32,7 @@
}
@Test
- public void testRemovingBridge()
- throws ExecutionException, CompilationFailedException, IOException {
+ public void testRemovingBridge() throws Exception {
testForR8(parameters.getBackend())
.addProgramClasses(
Service.class, Factory.class, SubService.class, SubFactory.class, Main.class)
@@ -44,7 +40,6 @@
.enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("Hello World!")
- .inspector();
+ .assertSuccessWithOutputLines("Hello World!");
}
}
