Fix spurious warning for protected access with isolated splits
Bug: b/300247439
Change-Id: I76efaa337a63ad0c1399bead9fc6a84e9b7d8f75
diff --git a/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
index 4c5c33b..9ac1bd5 100644
--- a/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
+++ b/src/main/java/com/android/tools/r8/features/FeatureSplitBoundaryOptimizationUtils.java
@@ -42,10 +42,21 @@
if (!classToFeatureSplitMap.isInBase(definition, appView)) {
return false;
}
- // If isolated splits are enabled then the resolved method must be public.
- if (appView.options().getFeatureSplitConfiguration().isIsolatedSplitsEnabled()
- && !definition.getAccessFlags().isPublic()) {
- return false;
+ // If isolated splits are enabled then the resolved item must be either (1) public or (2)
+ // a protected member and the access is in a subclass of the resolved member's holder.
+ if (appView.options().getFeatureSplitConfiguration().isIsolatedSplitsEnabled()) {
+ if (definition.isClass()) {
+ if (!definition.getAccessFlags().isPublic()) {
+ return false;
+ }
+ } else if (definition.getAccessFlags().isPackagePrivateOrProtected()) {
+ if (definition.getAccessFlags().isPackagePrivate()
+ || !appView
+ .appInfo()
+ .isSubtype(context.getContextClass(), definition.getContextClass())) {
+ return false;
+ }
+ }
}
return true;
}
diff --git a/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java b/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
index 1043723..376acee 100644
--- a/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
+++ b/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
@@ -80,6 +80,10 @@
|| features.isInSameFeature(accessedItem, context, appView)) {
return;
}
+ if (accessedItem.getAccessFlags().isProtected()
+ && appView.appInfo().isSubtype(context.getContextClass(), accessedItem.getContextClass())) {
+ return;
+ }
DontWarnConfiguration dontWarnConfiguration = appView.getDontWarnConfiguration();
if (dontWarnConfiguration.matches(accessedItem) || dontWarnConfiguration.matches(context)) {
return;
diff --git a/src/main/java/com/android/tools/r8/graph/AccessControl.java b/src/main/java/com/android/tools/r8/graph/AccessControl.java
index 3ed6de7..2cceb1a 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessControl.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessControl.java
@@ -78,20 +78,19 @@
}
return classAccessibility;
}
+ if (!member.getHolderType().isSamePackage(context.getContextType())) {
+ if (memberFlags.isPackagePrivate()
+ || !appInfo.isSubtype(context.getContextType(), member.getHolderType())) {
+ return OptionalBool.FALSE;
+ }
+ }
if (appView.hasClassHierarchy()
&& context.isProgramDefinition()
&& !FeatureSplitBoundaryOptimizationUtils.isSafeForAccess(
member, context.asProgramDefinition(), appView.withClassHierarchy())) {
return OptionalBool.UNKNOWN;
}
- if (member.getHolderType().isSamePackage(context.getContextType())) {
- return classAccessibility;
- }
- if (memberFlags.isProtected()
- && appInfo.isSubtype(context.getContextType(), member.getHolderType())) {
- return classAccessibility;
- }
- return OptionalBool.FALSE;
+ return classAccessibility;
}
private static boolean isNestMate(DexClass clazz, DexClass context) {
diff --git a/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
index 1b24d61..10afe1b 100644
--- a/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
+++ b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
@@ -42,45 +42,43 @@
}
@Test
+ public void testPublicToProtected() throws Exception {
+ MethodReference accessedMethod =
+ Reference.methodFromMethod(NonPublicBase.class.getDeclaredMethod("protectedMethod"));
+ MethodReference main =
+ Reference.methodFromMethod(Feature.class.getDeclaredMethod("testPublicToProtected"));
+ runTestWithExpectedFailure(main, getExpectedDiagnosticMessage(accessedMethod, main));
+ }
+
+ @Test
+ public void testPublicToProtectedFromSubclass() throws Exception {
+ MethodReference main =
+ Reference.methodFromMethod(FeatureSub.class.getDeclaredMethod("testPublicToProtected"));
+ runTest(main, TestDiagnosticMessages::assertNoMessages);
+ }
+
+ @Test
public void testPublicToPublic() throws Exception {
- runTest("testPublicToPublic", TestDiagnosticMessages::assertNoMessages);
+ MethodReference main =
+ Reference.methodFromMethod(Feature.class.getDeclaredMethod("testPublicToPublic"));
+ runTest(main, TestDiagnosticMessages::assertNoMessages);
}
@Test
public void testPublicToNonPublic() throws Exception {
MethodReference accessedMethod =
Reference.methodFromMethod(NonPublicBase.class.getDeclaredMethod("nonPublicMethod"));
- MethodReference context =
+ MethodReference main =
Reference.methodFromMethod(Feature.class.getDeclaredMethod("testPublicToNonPublic"));
- assertFailsCompilationIf(
- enableIsolatedSplits,
- () ->
- runTest(
- "testPublicToNonPublic",
- diagnostics ->
- diagnostics.assertErrorsMatch(
- allOf(
- diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class),
- diagnosticMessage(
- equalTo(getExpectedDiagnosticMessage(accessedMethod, context)))))));
+ runTestWithExpectedFailure(main, getExpectedDiagnosticMessage(accessedMethod, main));
}
@Test
public void testNonPublicToPublic() throws Exception {
ClassReference accessedClass = Reference.classFromClass(NonPublicBaseSub.class);
- MethodReference context =
+ MethodReference main =
Reference.methodFromMethod(Feature.class.getDeclaredMethod("testNonPublicToPublic"));
- assertFailsCompilationIf(
- enableIsolatedSplits,
- () ->
- runTest(
- "testNonPublicToPublic",
- diagnostics ->
- diagnostics.assertErrorsMatch(
- allOf(
- diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class),
- diagnosticMessage(
- equalTo(getExpectedDiagnosticMessage(accessedClass, context)))))));
+ runTestWithExpectedFailure(main, getExpectedDiagnosticMessage(accessedClass, main));
}
private static String getExpectedDiagnosticMessage(
@@ -103,13 +101,32 @@
+ ").";
}
- private void runTest(String name, DiagnosticsConsumer inspector) throws Exception {
+ private void runTestWithExpectedFailure(MethodReference main, String expectedDiagnosticMessage)
+ throws Exception {
+ assertFailsCompilationIf(
+ enableIsolatedSplits,
+ () ->
+ runTest(
+ main,
+ diagnostics ->
+ diagnostics.assertErrorsMatch(
+ allOf(
+ diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class),
+ diagnosticMessage(equalTo(expectedDiagnosticMessage))))));
+ }
+
+ private void runTest(MethodReference main, DiagnosticsConsumer inspector) throws Exception {
testForR8(parameters.getBackend())
.addProgramClasses(NonPublicBase.class, PublicBaseSub.class, NonPublicBaseSub.class)
.addKeepClassAndMembersRules(
NonPublicBase.class, PublicBaseSub.class, NonPublicBaseSub.class)
- .addFeatureSplit(Feature.class)
- .addKeepRules("-keep class " + Feature.class.getTypeName() + " { void " + name + "(); }")
+ .addFeatureSplit(Feature.class, FeatureSub.class)
+ .addKeepRules(
+ "-keep class "
+ + main.getHolderClass().getTypeName()
+ + " { void "
+ + main.getMethodName()
+ + "(); }")
.enableIsolatedSplits(enableIsolatedSplits)
.setMinApi(parameters)
.compileWithExpectedDiagnostics(enableIsolatedSplits ? inspector : this::assertNoMessages);
@@ -121,6 +138,10 @@
static class NonPublicBase {
+ protected static void protectedMethod() {
+ // Intentionally empty.
+ }
+
public static void publicMethod() {
// Intentionally empty.
}
@@ -136,6 +157,10 @@
public static class Feature {
+ public static void testPublicToProtected() {
+ PublicBaseSub.protectedMethod();
+ }
+
public static void testPublicToPublic() {
PublicBaseSub.publicMethod();
}
@@ -148,4 +173,11 @@
NonPublicBaseSub.publicMethod();
}
}
+
+ public static class FeatureSub extends NonPublicBase {
+
+ public static void testPublicToProtected() {
+ PublicBaseSub.protectedMethod();
+ }
+ }
}
