Add warning and diagnostic for final fields in R classes
This will only run if we are in optimized resource shrinking
Bug: 335772436
Change-Id: Id7fe5a830625a996b97e3d490cfd439a1044e1de
diff --git a/src/main/java/com/android/tools/r8/errors/FinalRClassEntriesWithOptimizedShrinkingDiagnostic.java b/src/main/java/com/android/tools/r8/errors/FinalRClassEntriesWithOptimizedShrinkingDiagnostic.java
new file mode 100644
index 0000000..c2f18a5
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/FinalRClassEntriesWithOptimizedShrinkingDiagnostic.java
@@ -0,0 +1,41 @@
+// Copyright (c) 2024, 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.errors;
+
+import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.keepanno.annotations.KeepForApi;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+
+@KeepForApi
+public class FinalRClassEntriesWithOptimizedShrinkingDiagnostic implements Diagnostic {
+
+ private final Origin origin;
+ private final DexField dexField;
+
+ public FinalRClassEntriesWithOptimizedShrinkingDiagnostic(Origin origin, DexField dexField) {
+ this.origin = origin;
+ this.dexField = dexField;
+ }
+
+ @Override
+ public Origin getOrigin() {
+ return origin;
+ }
+
+ @Override
+ public Position getPosition() {
+ return Position.UNKNOWN;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ return "Running optimized resource shrinking with final R class ids is not supported and "
+ + "can lead to missing resources and code necessary for program execution. "
+ + "Field "
+ + dexField
+ + " is final";
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
index 01e5f1e..f70748d 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.graph.analysis;
import com.android.build.shrinker.r8integration.R8ResourceShrinkerState;
+import com.android.tools.r8.errors.FinalRClassEntriesWithOptimizedShrinkingDiagnostic;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
@@ -90,7 +91,6 @@
}
}
- @SuppressWarnings("ReferenceEquality")
private void populateRClassValues(ProgramField field) {
// TODO(287398085): Pending discussions with the AAPT2 team, we might need to harden this
// to not fail if we wrongly classify an unrelated class as R class in our heuristic..
@@ -100,10 +100,28 @@
if (programClassInitializer != null) {
analyzeClassInitializer(rClassValueBuilder, programClassInitializer);
}
-
+ warnOnFinalIdFields(field.getHolder());
fieldToValueMapping.put(field.getHolderType(), rClassValueBuilder.build());
}
+ private void warnOnFinalIdFields(DexProgramClass holder) {
+ if (!appView.options().isOptimizedResourceShrinking()) {
+ return;
+ }
+ for (DexEncodedField field : holder.fields()) {
+ if (field.isStatic()
+ && field.isFinal()
+ && field.hasExplicitStaticValue()
+ && field.getType().isIntType()) {
+ appView
+ .reporter()
+ .warning(
+ new FinalRClassEntriesWithOptimizedShrinkingDiagnostic(
+ holder.origin, field.getReference()));
+ }
+ }
+ }
+
private void analyzeClassInitializer(
RClassFieldToValueStore.Builder rClassValueBuilder, ProgramMethod programClassInitializer) {
IRCode code = programClassInitializer.buildIR(appView, MethodConversionOptions.nonConverting());
diff --git a/src/test/java/com/android/tools/r8/androidresources/DiagnosticOnFinalIntFieldsTest.java b/src/test/java/com/android/tools/r8/androidresources/DiagnosticOnFinalIntFieldsTest.java
new file mode 100644
index 0000000..a096a8f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/DiagnosticOnFinalIntFieldsTest.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2024, 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.androidresources;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.errors.FinalRClassEntriesWithOptimizedShrinkingDiagnostic;
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+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 DiagnosticOnFinalIntFieldsTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withDefaultDexRuntime().withAllApiLevels().build();
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.string.class)
+ .build(temp);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ // We test the final id field type by simply passing the standard test resources but ignoring
+ // the aapt generated R class, instead we pass directly the R class from this file, which
+ // have no real resource references, but does have a non integer field.
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(FooBar.class)
+ .addAndroidResources(
+ getTestResources(temp),
+ temp.newFile("resout.zip").toPath(),
+ ImmutableList.of(ToolHelper.getClassAsBytes(R.string.class)))
+ .addKeepMainRule(FooBar.class)
+ .enableOptimizedShrinking()
+ .allowDiagnosticWarningMessages()
+ .compileWithExpectedDiagnostics(
+ diagnostics ->
+ diagnostics
+ .assertOnlyWarnings()
+ .assertWarningsMatch(
+ diagnosticType(FinalRClassEntriesWithOptimizedShrinkingDiagnostic.class)));
+ }
+
+ public static class FooBar {
+ public static void main(String[] args) {
+ if (System.currentTimeMillis() == 0) {
+ System.out.println(R.string.foo);
+ System.out.println(R.string.bar);
+ }
+ if (R.string.nonResource != null) {
+ System.out.println("bar");
+ }
+ }
+ }
+
+ public static class R {
+ public static class string {
+ private static Object nonResource = new Object();
+ public static int foo = 0x7f110004;
+ public static final int bar = 0x7f110005;
+ }
+ }
+}
