Test warnings for package private access across isolated split
Bug: b/300247439
Change-Id: Ibe735397e38d45b9bbe44423cc50c56964707994
diff --git a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
index 0e79887..a20cfef 100644
--- a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
+++ b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.graph.PrunedItems;
@@ -128,17 +129,16 @@
public FeatureSplit getFeatureSplit(
ProgramDefinition definition,
SyntheticItems syntheticItems) {
- return getFeatureSplit(definition.getContextType(), syntheticItems);
+ return getFeatureSplit(definition.getReference(), syntheticItems);
}
public FeatureSplit getFeatureSplit(
- DexType type, AppView<? extends AppInfoWithClassHierarchy> appView) {
- return getFeatureSplit(type, appView.getSyntheticItems());
+ DexReference reference, AppView<? extends AppInfoWithClassHierarchy> appView) {
+ return getFeatureSplit(reference, appView.getSyntheticItems());
}
- public FeatureSplit getFeatureSplit(
- DexType type,
- SyntheticItems syntheticItems) {
+ public FeatureSplit getFeatureSplit(DexReference reference, SyntheticItems syntheticItems) {
+ DexType type = reference.getContextType();
if (syntheticItems == null) {
// Called from AndroidApp.dumpProgramResources().
return classToFeatureSplitMap.getOrDefault(type, FeatureSplit.BASE);
@@ -216,6 +216,16 @@
return !isInBase(clazz, syntheticItems);
}
+ public boolean isInSameFeature(
+ ProgramDefinition definition, ProgramDefinition other, AppView<?> appView) {
+ return isInSameFeature(definition, other, appView.getSyntheticItems());
+ }
+
+ public boolean isInSameFeature(
+ ProgramDefinition definition, ProgramDefinition other, SyntheticItems syntheticItems) {
+ return getFeatureSplit(definition, syntheticItems) == getFeatureSplit(other, syntheticItems);
+ }
+
public ClassToFeatureSplitMap rewrittenWithLens(GraphLens lens, Timing timing) {
return timing.time("Rewrite ClassToFeatureSplitMap", () -> rewrittenWithLens(lens));
}
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 5708818..dbef2b3 100644
--- a/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
+++ b/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
@@ -79,7 +79,7 @@
}
private void checkAccess(ProgramDefinition accessedItem, ProgramMethod context) {
- if (!features.isInBaseOrSameFeatureAs(accessedItem, context, appView)
+ if (!features.isInSameFeature(accessedItem, context, appView)
&& !accessedItem.getAccessFlags().isPublic()) {
appView
.reporter()
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index abafdc8..0e08d8f 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -75,6 +75,7 @@
private AllowedDiagnosticMessages allowedDiagnosticMessages = AllowedDiagnosticMessages.NONE;
private boolean allowUnusedProguardConfigurationRules = false;
+ private boolean enableIsolatedSplits = false;
private boolean enableMissingLibraryApiModeling = true;
private boolean enableStartupLayoutOptimization = true;
private CollectingGraphConsumer graphConsumer = null;
@@ -152,6 +153,7 @@
ToolHelper.addSyntheticProguardRulesConsumerForTesting(
builder, rules -> box.syntheticProguardRules = rules);
libraryDesugaringTestConfiguration.configure(builder);
+ builder.setEnableExperimentalIsolatedSplits(enableIsolatedSplits);
builder.setEnableExperimentalMissingLibraryApiModeling(enableMissingLibraryApiModeling);
builder.setEnableStartupLayoutOptimization(enableStartupLayoutOptimization);
StringBuilder pgConfOutput = wrapProguardConfigConsumer(builder);
@@ -856,6 +858,11 @@
return self();
}
+ public T enableIsolatedSplits(boolean enableIsolatedSplits) {
+ this.enableIsolatedSplits = enableIsolatedSplits;
+ return self();
+ }
+
public T addArtProfileForRewriting(ArtProfileProvider artProfileProvider) {
return addArtProfileForRewriting(
artProfileProvider,
diff --git a/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
new file mode 100644
index 0000000..1b24d61
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
@@ -0,0 +1,151 @@
+// Copyright (c) 2023, 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.features;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
+import static com.android.tools.r8.utils.codeinspector.AssertUtils.assertFailsCompilationIf;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.equalTo;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestCompilerBuilder.DiagnosticsConsumer;
+import com.android.tools.r8.TestDiagnosticMessages;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.features.diagnostic.IllegalAccessWithIsolatedFeatureSplitsDiagnostic;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.MethodReferenceUtils;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class PackagePrivateIsolatedSplitCrossBoundaryReferenceTest extends TestBase {
+
+ @Parameter(0)
+ public boolean enableIsolatedSplits;
+
+ @Parameter(1)
+ public TestParameters parameters;
+
+ @Parameters(name = "{1}, isolated splits: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ BooleanUtils.values(), getTestParameters().withDexRuntimesAndAllApiLevels().build());
+ }
+
+ @Test
+ public void testPublicToPublic() throws Exception {
+ runTest("testPublicToPublic", TestDiagnosticMessages::assertNoMessages);
+ }
+
+ @Test
+ public void testPublicToNonPublic() throws Exception {
+ MethodReference accessedMethod =
+ Reference.methodFromMethod(NonPublicBase.class.getDeclaredMethod("nonPublicMethod"));
+ MethodReference context =
+ Reference.methodFromMethod(Feature.class.getDeclaredMethod("testPublicToNonPublic"));
+ assertFailsCompilationIf(
+ enableIsolatedSplits,
+ () ->
+ runTest(
+ "testPublicToNonPublic",
+ diagnostics ->
+ diagnostics.assertErrorsMatch(
+ allOf(
+ diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class),
+ diagnosticMessage(
+ equalTo(getExpectedDiagnosticMessage(accessedMethod, context)))))));
+ }
+
+ @Test
+ public void testNonPublicToPublic() throws Exception {
+ ClassReference accessedClass = Reference.classFromClass(NonPublicBaseSub.class);
+ MethodReference context =
+ Reference.methodFromMethod(Feature.class.getDeclaredMethod("testNonPublicToPublic"));
+ assertFailsCompilationIf(
+ enableIsolatedSplits,
+ () ->
+ runTest(
+ "testNonPublicToPublic",
+ diagnostics ->
+ diagnostics.assertErrorsMatch(
+ allOf(
+ diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class),
+ diagnosticMessage(
+ equalTo(getExpectedDiagnosticMessage(accessedClass, context)))))));
+ }
+
+ private static String getExpectedDiagnosticMessage(
+ ClassReference accessedClass, MethodReference context) {
+ return "Unexpected illegal access to non-public class in another feature split "
+ + "(accessed: "
+ + accessedClass.getTypeName()
+ + ", context: "
+ + MethodReferenceUtils.toSourceString(context)
+ + ").";
+ }
+
+ private static String getExpectedDiagnosticMessage(
+ MethodReference accessedMethod, MethodReference context) {
+ return "Unexpected illegal access to non-public method in another feature split "
+ + "(accessed: "
+ + MethodReferenceUtils.toSourceString(accessedMethod)
+ + ", context: "
+ + MethodReferenceUtils.toSourceString(context)
+ + ").";
+ }
+
+ private void runTest(String name, 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 + "(); }")
+ .enableIsolatedSplits(enableIsolatedSplits)
+ .setMinApi(parameters)
+ .compileWithExpectedDiagnostics(enableIsolatedSplits ? inspector : this::assertNoMessages);
+ }
+
+ private void assertNoMessages(TestDiagnosticMessages diagnostics) {
+ diagnostics.assertNoMessages();
+ }
+
+ static class NonPublicBase {
+
+ public static void publicMethod() {
+ // Intentionally empty.
+ }
+
+ static void nonPublicMethod() {
+ // Intentionally empty.
+ }
+ }
+
+ public static class PublicBaseSub extends NonPublicBase {}
+
+ static class NonPublicBaseSub extends NonPublicBase {}
+
+ public static class Feature {
+
+ public static void testPublicToPublic() {
+ PublicBaseSub.publicMethod();
+ }
+
+ public static void testPublicToNonPublic() {
+ PublicBaseSub.nonPublicMethod();
+ }
+
+ public static void testNonPublicToPublic() {
+ NonPublicBaseSub.publicMethod();
+ }
+ }
+}
