Reland "Propagate the value of kotlin._Assertions.ENABLED in D8"
Based on the assertions configuration the value of
kotlin._Assertions.ENABLED is propagated in all compilers based on the
Kotlin stdlib code for kotlin._Assertions.
Bug: 139898386
This reverts commit 53c1edda78a23eddbc8fc8b0f77a7ab2c62db041 with a
fix.
Change-Id: I0e12ebc54b2c230ceb8318ca02147498d875d1ec
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
index 061cbcf..b41e8fa 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
@@ -127,7 +127,7 @@
}
private boolean hasKotlincClinitAssertionCode(DexEncodedMethod method) {
- if (method.holder() == dexItemFactory.kotlin.kotlinAssertions) {
+ if (method.holder() == dexItemFactory.kotlin.assertions.type) {
CfCode code = method.getCode().asCfCode();
for (int i = 1; i < code.instructions.size(); i++) {
CfInstruction instruction = code.instructions.get(i - 1);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/AssertionsRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/AssertionsRewriter.java
index a356e8b..81b43ef 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/AssertionsRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/AssertionsRewriter.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -77,6 +78,7 @@
private final DexItemFactory dexItemFactory;
private final AssertionTransformation defaultTransformation;
private final List<ConfigurationEntryWithDexString> configuration;
+ private final AssertionsConfiguration.AssertionTransformation kotlinTransformation;
private final boolean enabled;
public AssertionsRewriter(AppView<?> appView) {
@@ -86,6 +88,7 @@
if (!enabled) {
defaultTransformation = null;
configuration = null;
+ kotlinTransformation = null;
return;
}
// Convert the assertion transformation to the representation used for this rewriter.
@@ -94,6 +97,8 @@
appView.options().assertionsConfiguration.assertionsConfigurations.stream()
.map(entry -> new ConfigurationEntryWithDexString(entry, appView.dexItemFactory()))
.collect(Collectors.toList());
+ kotlinTransformation =
+ getTransformationForType(appView.dexItemFactory().kotlin.assertions.type);
}
// Static method used by other analyses to see if additional analysis is required to support
@@ -104,6 +109,10 @@
}
private AssertionTransformation getTransformationForMethod(DexEncodedMethod method) {
+ return getTransformationForType(method.holder());
+ }
+
+ private AssertionTransformation getTransformationForType(DexType type) {
AssertionTransformation transformation = defaultTransformation;
for (ConfigurationEntryWithDexString entry : configuration) {
switch (entry.entry.getScope()) {
@@ -112,18 +121,18 @@
break;
case PACKAGE:
if (entry.value.size == 0) {
- if (!method.holder().descriptor.contains(dexItemFactory.descriptorSeparator)) {
+ if (!type.descriptor.contains(dexItemFactory.descriptorSeparator)) {
transformation = entry.entry.getTransformation();
}
- } else if (method.holder().descriptor.startsWith(entry.value)) {
+ } else if (type.descriptor.startsWith(entry.value)) {
transformation = entry.entry.getTransformation();
}
break;
case CLASS:
- if (method.holder().descriptor.equals(entry.value)) {
+ if (type.descriptor.equals(entry.value)) {
transformation = entry.entry.getTransformation();
}
- if (isDescriptorForClassOrInnerClass(entry.value, method.holder().descriptor)) {
+ if (isDescriptorForClassOrInnerClass(entry.value, type.descriptor)) {
transformation = entry.entry.getTransformation();
}
break;
@@ -317,10 +326,10 @@
}
clinit = clazz.getClassInitializer();
}
- if (clinit == null || !clinit.getOptimizationInfo().isInitializerEnablingJavaVmAssertions()) {
- return;
- }
-
+ // For javac generated code it is assumed that the code in <clinit> will tell if the code
+ // in other methods of the class can have assertion checks.
+ boolean isInitializerEnablingJavaVmAssertions =
+ clinit != null && clinit.getOptimizationInfo().isInitializerEnablingJavaVmAssertions();
// This code will process the assertion code in all methods including <clinit>.
InstructionListIterator iterator = code.instructionListIterator();
while (iterator.hasNext()) {
@@ -328,7 +337,7 @@
if (current.isInvokeMethod()) {
InvokeMethod invoke = current.asInvokeMethod();
if (invoke.getInvokedMethod() == dexItemFactory.classMethods.desiredAssertionStatus) {
- if (method.holder() == dexItemFactory.kotlin.kotlinAssertions) {
+ if (method.holder() == dexItemFactory.kotlin.assertions.type) {
rewriteKotlinAssertionEnable(code, transformation, iterator, invoke);
} else {
iterator.replaceCurrentInstruction(code.createIntConstant(0));
@@ -336,15 +345,24 @@
}
} else if (current.isStaticPut()) {
StaticPut staticPut = current.asStaticPut();
- if (staticPut.getField().name == dexItemFactory.assertionsDisabled) {
+ if (isInitializerEnablingJavaVmAssertions
+ && staticPut.getField().name == dexItemFactory.assertionsDisabled) {
iterator.remove();
}
} else if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
- if (staticGet.getField().name == dexItemFactory.assertionsDisabled) {
+ // Rewrite $assertionsDisabled getter (only if the initializer enabled assertions).
+ if (isInitializerEnablingJavaVmAssertions
+ && staticGet.getField().name == dexItemFactory.assertionsDisabled) {
iterator.replaceCurrentInstruction(
code.createIntConstant(transformation == AssertionTransformation.DISABLE ? 1 : 0));
}
+ // Rewrite kotlin._Assertions.ENABLED getter.
+ if (staticGet.getField() == dexItemFactory.kotlin.assertions.enabledField) {
+ iterator.replaceCurrentInstruction(
+ code.createIntConstant(
+ kotlinTransformation == AssertionTransformation.DISABLE ? 0 : 1));
+ }
}
}
}
@@ -362,7 +380,7 @@
Instruction nextInstruction = iterator.next();
if (nextInstruction.isStaticPut()
&& nextInstruction.asStaticPut().getField().holder
- == dexItemFactory.kotlin.kotlinAssertions
+ == dexItemFactory.kotlin.assertions.type
&& nextInstruction.asStaticPut().getField().name == dexItemFactory.enabledFieldName
&& invoke.outValue().numberOfUsers() == 1
&& invoke.outValue().numberOfPhiUsers() == 0
diff --git a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
index b4308b9..6fe2c18 100644
--- a/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
+++ b/src/main/java/com/android/tools/r8/kotlin/Kotlin.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.DiagnosticsHandler;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
@@ -35,10 +36,10 @@
public final DexItemFactory factory;
- public final DexType kotlinAssertions;
public final Functional functional;
public final Intrinsics intrinsics;
public final Metadata metadata;
+ public final _Assertions assertions;
public static final class ClassClassifiers {
@@ -51,10 +52,10 @@
public Kotlin(DexItemFactory factory) {
this.factory = factory;
- this.kotlinAssertions = factory.createType(addKotlinPrefix("_Assertions;"));
this.functional = new Functional();
this.intrinsics = new Intrinsics();
this.metadata = new Metadata();
+ this.assertions = new _Assertions();
// See {@link org.jetbrains.kotlin.metadata.jvm.deserialization.ClassMapperLite}
this.knownTypeConversion =
@@ -172,6 +173,13 @@
public final DexString extraInt = factory.createString("xi");
}
+ public final class _Assertions {
+ public final DexType type = factory.createType(addKotlinPrefix("_Assertions;"));
+ public final DexString enabledFieldName = factory.createString("ENABLED");
+ public final DexField enabledField =
+ factory.createField(type, factory.booleanType, enabledFieldName);
+ }
+
// kotlin.jvm.internal.Intrinsics class
public final class Intrinsics {
public final DexType type = factory.createType(addKotlinPrefix("jvm/internal/Intrinsics;"));
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionConfigurationKotlinTest.java b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionConfigurationKotlinTest.java
index b201ce6..c2fdaa1 100644
--- a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionConfigurationKotlinTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionConfigurationKotlinTest.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.ThrowableConsumer;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.ThrowingConsumer;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -52,17 +53,23 @@
private static final Map<KotlinTargetVersion, Path> kotlinClasses = new HashMap<>();
private final TestParameters parameters;
+ private final boolean kotlinStdlibAsLibrary;
- @Parameterized.Parameters(name = "{0},{1}")
+ @Parameterized.Parameters(name = "{0}, {1}, kotlin-stdlib as library: {2}")
public static Collection<Object[]> data() {
return buildParameters(
- getTestParameters().withAllRuntimesAndApiLevels().build(), KotlinTargetVersion.values());
+ getTestParameters().withAllRuntimesAndApiLevels().build(),
+ KotlinTargetVersion.values(),
+ BooleanUtils.values());
}
public AssertionConfigurationKotlinTest(
- TestParameters parameters, KotlinTargetVersion targetVersion) {
+ TestParameters parameters,
+ KotlinTargetVersion targetVersion,
+ boolean kotlinStdlibAsClasspath) {
super(targetVersion);
this.parameters = parameters;
+ this.kotlinStdlibAsLibrary = kotlinStdlibAsClasspath;
}
@BeforeClass
@@ -76,21 +83,50 @@
}
}
+ private Path kotlinStdlibLibraryForRuntime() throws Exception {
+ Path kotlinStdlibCf = ToolHelper.getKotlinStdlibJar();
+ if (parameters.getRuntime().isCf()) {
+ return kotlinStdlibCf;
+ }
+
+ Path kotlinStdlibDex = temp.newFolder().toPath().resolve("kotlin-stdlib-dex.jar");
+ testForD8()
+ .addProgramFiles(kotlinStdlibCf)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .writeToZip(kotlinStdlibDex);
+ return kotlinStdlibDex;
+ }
+
private void runD8Test(
ThrowableConsumer<D8TestBuilder> builderConsumer,
ThrowingConsumer<CodeInspector, RuntimeException> inspector,
List<String> outputLines)
throws Exception {
- testForD8()
- .addProgramFiles(ToolHelper.getKotlinStdlibJar())
- .addProgramFiles(kotlinClasses.get(targetVersion))
- .setMinApi(parameters.getApiLevel())
- .apply(builderConsumer)
- .run(
- parameters.getRuntime(),
- getClass().getPackage().getName() + ".kotlintestclasses.TestClassKt")
- .inspect(inspector)
- .assertSuccessWithOutputLines(outputLines);
+ if (kotlinStdlibAsLibrary) {
+ testForD8()
+ .addClasspathFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(kotlinClasses.get(targetVersion))
+ .setMinApi(parameters.getApiLevel())
+ .apply(builderConsumer)
+ .addRunClasspathFiles(kotlinStdlibLibraryForRuntime())
+ .run(
+ parameters.getRuntime(),
+ getClass().getPackage().getName() + ".kotlintestclasses.TestClassKt")
+ .inspect(inspector)
+ .assertSuccessWithOutputLines(outputLines);
+ } else {
+ testForD8()
+ .addProgramFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(kotlinClasses.get(targetVersion))
+ .setMinApi(parameters.getApiLevel())
+ .apply(builderConsumer)
+ .run(
+ parameters.getRuntime(),
+ getClass().getPackage().getName() + ".kotlintestclasses.TestClassKt")
+ .inspect(inspector)
+ .assertSuccessWithOutputLines(outputLines);
+ }
}
public void runR8Test(
@@ -108,21 +144,38 @@
boolean enableJvmAssertions)
throws Exception {
- testForR8(parameters.getBackend())
- .addProgramFiles(ToolHelper.getKotlinStdlibJar())
- .addProgramFiles(kotlinClasses.get(targetVersion))
- .addKeepMainRule(testClassKt)
- .addKeepClassAndMembersRules(class1, class2)
- .setMinApi(parameters.getApiLevel())
- .apply(builderConsumer)
- .noMinification()
- .allowDiagnosticWarningMessages()
- .compile()
- .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
- .enableRuntimeAssertions(enableJvmAssertions)
- .run(parameters.getRuntime(), testClassKt)
- .inspect(inspector)
- .assertSuccessWithOutputLines(outputLines);
+ if (kotlinStdlibAsLibrary) {
+ testForR8(parameters.getBackend())
+ .addClasspathFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(kotlinClasses.get(targetVersion))
+ .addKeepMainRule(testClassKt)
+ .addKeepClassAndMembersRules(class1, class2)
+ .setMinApi(parameters.getApiLevel())
+ .apply(builderConsumer)
+ .noMinification()
+ .addRunClasspathFiles(kotlinStdlibLibraryForRuntime())
+ .compile()
+ .enableRuntimeAssertions(enableJvmAssertions)
+ .run(parameters.getRuntime(), testClassKt)
+ .inspect(inspector)
+ .assertSuccessWithOutputLines(outputLines);
+ } else {
+ testForR8(parameters.getBackend())
+ .addProgramFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(kotlinClasses.get(targetVersion))
+ .addKeepMainRule(testClassKt)
+ .addKeepClassAndMembersRules(class1, class2)
+ .setMinApi(parameters.getApiLevel())
+ .apply(builderConsumer)
+ .noMinification()
+ .allowDiagnosticWarningMessages()
+ .compile()
+ .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
+ .enableRuntimeAssertions(enableJvmAssertions)
+ .run(parameters.getRuntime(), testClassKt)
+ .inspect(inspector)
+ .assertSuccessWithOutputLines(outputLines);
+ }
}
private List<String> allAssertionsExpectedLines() {
@@ -136,6 +189,7 @@
private void checkAssertionCodeRemoved(ClassSubject subject, boolean isR8) {
assertThat(subject, isPresent());
if (subject.getOriginalName().equals("kotlin._Assertions")) {
+ assert !kotlinStdlibAsLibrary;
// With R8 the static-put of the kotlin._Assertions.INSTANCE field is removed as well,
// as is not used.
assertEquals(
@@ -151,9 +205,9 @@
.streamInstructions()
.anyMatch(InstructionSubject::isConstNumber));
} else {
- // In R8 the false (default) value of kotlin._Assertions.ENABLED is propagated.
- assertEquals(
- !isR8,
+ // The value of kotlin._Assertions.ENABLED is propagated from the assertions configuration
+ // for the class kotlin._Assertions.
+ assertFalse(
subject
.uniqueMethodWithName("m")
.streamInstructions()
@@ -168,6 +222,7 @@
private void checkAssertionCodeEnabled(ClassSubject subject, boolean isR8) {
assertThat(subject, isPresent());
if (subject.getOriginalName().equals("kotlin._Assertions")) {
+ assert !kotlinStdlibAsLibrary;
// With R8 the static-put of the kotlin._Assertions.INSTANCE field is removed as is not used.
assertEquals(
(isR8 ? 1 : 2),
@@ -199,6 +254,7 @@
ClassSubject subject = inspector.clazz(clazz);
assertThat(subject, isPresent());
if (subject.getOriginalName().equals("kotlin._Assertions")) {
+ assert !kotlinStdlibAsLibrary;
// With R8 the static-put of the kotlin._Assertions.INSTANCE field is removed as is not used.
assertEquals(
(isR8 ? 1 : 2),
@@ -224,7 +280,7 @@
private void checkAssertionCodeRemoved(CodeInspector inspector, boolean isR8) {
if (isR8) {
assertThat(inspector.clazz("kotlin._Assertions"), not(isPresent()));
- } else {
+ } else if (!kotlinStdlibAsLibrary) {
checkAssertionCodeRemoved(inspector, "kotlin._Assertions", isR8);
}
checkAssertionCodeRemoved(inspector, class1, isR8);
@@ -234,7 +290,7 @@
private void checkAssertionCodeEnabled(CodeInspector inspector, boolean isR8) {
if (isR8) {
assertThat(inspector.clazz("kotlin._Assertions"), not(isPresent()));
- } else {
+ } else if (!kotlinStdlibAsLibrary) {
checkAssertionCodeEnabled(inspector, "kotlin._Assertions", isR8);
}
checkAssertionCodeEnabled(inspector, class1, isR8);
@@ -242,7 +298,9 @@
}
private void checkAssertionCodeLeft(CodeInspector inspector, boolean isR8) {
- checkAssertionCodeLeft(inspector, "kotlin._Assertions", isR8);
+ if (!kotlinStdlibAsLibrary) {
+ checkAssertionCodeLeft(inspector, "kotlin._Assertions", isR8);
+ }
checkAssertionCodeLeft(inspector, class1, isR8);
checkAssertionCodeLeft(inspector, class2, isR8);
}
