Version 2.0.30
Cherry-pick: Add reproduction of b/148525512
CL: https://r8-review.googlesource.com/c/r8/+/48647
Cherry-pick: Don't add lambdas from features into lambda groups
CL: https://r8-review.googlesource.com/c/r8/+/48650
There where slight changes to the test after cherry picking due to
testing infrastructure for allowDiagnosticWarningMessages and
assertAllWarningMessagesMatch not being present on the 2.0 branch.
No changes to the actual fix.
Bug: 148525512
Bug: 149369974
Change-Id: I23b5e499c17fc36432e3bf2b4edbd636eef6420b
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 8e81cfe..7617447 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.29";
+ public static final String LABEL = "2.0.30";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
index 8fee6c5..a5e9d5c4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaMerger.java
@@ -234,7 +234,9 @@
cls.hasKotlinInfo()
&& cls.getKotlinInfo().isSyntheticClass()
&& cls.getKotlinInfo().asSyntheticClass().isLambda()
- && KotlinLambdaGroupIdFactory.hasValidAnnotations(kotlin, cls))
+ && KotlinLambdaGroupIdFactory.hasValidAnnotations(kotlin, cls)
+ && (appView.options().featureSplitConfiguration == null
+ || !appView.options().featureSplitConfiguration.isInFeature(cls)))
.sorted((a, b) -> a.type.slowCompareTo(b.type)) // Ensure stable ordering.
.forEachOrdered(
lambda -> {
diff --git a/src/test/java/com/android/tools/r8/KotlinTestBase.java b/src/test/java/com/android/tools/r8/KotlinTestBase.java
index a470bb3..45a2b11 100644
--- a/src/test/java/com/android/tools/r8/KotlinTestBase.java
+++ b/src/test/java/com/android/tools/r8/KotlinTestBase.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.FileUtils;
+import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -28,6 +29,12 @@
this.targetVersion = targetVersion;
}
+ protected static Path getKotlinFileInTestPackage(Package pkg, String fileName)
+ throws IOException {
+ String folder = DescriptorUtils.getBinaryNameFromJavaType(pkg.getName());
+ return getKotlinFileInTest(folder, fileName);
+ }
+
protected static Path getKotlinFileInTest(String folder, String fileName) {
return Paths.get(ToolHelper.TESTS_DIR, "java", folder, fileName + FileUtils.KT_EXTENSION);
}
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 00d991a..1290489 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -1169,8 +1169,11 @@
consumer.finished(null);
}
- protected static void writeClassFileDataToJar(Path output, Collection<byte[]> classes)
- throws IOException {
+ protected static void writeClassesToJar(Path output, Class<?>... classes) throws IOException {
+ writeClassesToJar(output, Arrays.asList(classes));
+ }
+
+ protected static void writeClassFileDataToJar(Path output, Collection<byte[]> classes) {
ClassFileConsumer consumer = new ArchiveConsumer(output);
for (byte[] clazz : classes) {
consumer.accept(ByteDataView.of(clazz), extractClassDescriptor(clazz), null);
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java
new file mode 100644
index 0000000..c95c372
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/B148525512.java
@@ -0,0 +1,151 @@
+// Copyright (c) 2020, 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.kotlin.lambda.b148525512;
+
+import static com.android.tools.r8.KotlinCompilerTool.KOTLINC;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.DexIndexedConsumer.ArchiveConsumer;
+import com.android.tools.r8.KotlinTestBase;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
+import com.android.tools.r8.utils.ArchiveResourceProvider;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class B148525512 extends KotlinTestBase {
+
+ private static final Package pkg = B148525512.class.getPackage();
+ private static final String kotlinTestClassesPackage = pkg.getName();
+ private static final String baseKtClassName = kotlinTestClassesPackage + ".BaseKt";
+ private static final String featureKtClassNamet = kotlinTestClassesPackage + ".FeatureKt";
+ private static final String baseClassName = kotlinTestClassesPackage + ".Base";
+
+ private static final Map<KotlinTargetVersion, Path> kotlinBaseClasses = new HashMap<>();
+ private static final Map<KotlinTargetVersion, Path> kotlinFeatureClasses = new HashMap<>();
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0},{1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDexRuntimes().withAllApiLevels().build(),
+ KotlinTargetVersion.values());
+ }
+
+ public B148525512(TestParameters parameters, KotlinTargetVersion targetVersion) {
+ super(targetVersion);
+ this.parameters = parameters;
+ }
+
+ @ClassRule public static TemporaryFolder classTemp = new TemporaryFolder();
+
+ @BeforeClass
+ public static void compileKotlin() throws Exception {
+ // Compile the base Kotlin with the FeatureAPI Java class on classpath.
+ Path featureApiJar = classTemp.newFile("feature_api.jar").toPath();
+ writeClassesToJar(featureApiJar, FeatureAPI.class);
+ for (KotlinTargetVersion targetVersion : KotlinTargetVersion.values()) {
+ Path ktBaseClasses =
+ kotlinc(KOTLINC, targetVersion)
+ .addClasspathFiles(featureApiJar)
+ .addSourceFiles(getKotlinFileInTestPackage(pkg, "base"))
+ .compile();
+ kotlinBaseClasses.put(targetVersion, ktBaseClasses);
+ // Compile the feature Kotlin code with the base classes on classpath.
+ Path ktFeatureClasses =
+ kotlinc(KOTLINC, targetVersion)
+ .addClasspathFiles(ktBaseClasses)
+ .addSourceFiles(getKotlinFileInTestPackage(pkg, "feature"))
+ .compile();
+ kotlinFeatureClasses.put(targetVersion, ktFeatureClasses);
+ }
+ }
+
+ private void checkLambdaGroups(CodeInspector inspector) {
+ List<FoundClassSubject> lambdaGroups =
+ inspector.allClasses().stream()
+ .filter(clazz -> clazz.getOriginalName().contains("LambdaGroup"))
+ .collect(Collectors.toList());
+ assertEquals(1, lambdaGroups.size());
+ MethodSubject invokeMethod = lambdaGroups.get(0).uniqueMethodWithName("invoke");
+ assertThat(invokeMethod, isPresent());
+ // The lambda group has 2 captures which capture "Base".
+ assertEquals(
+ 2,
+ invokeMethod
+ .streamInstructions()
+ .filter(InstructionSubject::isCheckCast)
+ .filter(
+ instruction ->
+ instruction.asCheckCast().getType().toSourceString().contains("Base"))
+ .count());
+ // The lambda group has no captures which capture "Feature" (lambdas in the feature are not
+ // in this lambda group).
+ assertTrue(
+ invokeMethod
+ .streamInstructions()
+ .filter(InstructionSubject::isCheckCast)
+ .noneMatch(
+ instruction ->
+ instruction.asCheckCast().getType().toSourceString().contains("Feature")));
+ }
+
+ @Test
+ public void test() throws Exception {
+ Path featureCode = temp.newFile("feature.zip").toPath();
+ R8TestCompileResult compileResult =
+ testForR8(parameters.getBackend())
+ .addProgramFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(kotlinBaseClasses.get(targetVersion))
+ .addProgramClasses(FeatureAPI.class)
+ .addKeepMainRule(baseKtClassName)
+ .addKeepClassAndMembersRules(baseClassName)
+ .addKeepClassAndMembersRules(featureKtClassNamet)
+ .addKeepClassAndMembersRules(FeatureAPI.class)
+ .setMinApi(parameters.getApiLevel())
+ .noMinification() // The check cast inspection above relies on original names.
+ .addFeatureSplit(
+ builder ->
+ builder
+ .addProgramResourceProvider(
+ ArchiveResourceProvider.fromArchive(
+ kotlinFeatureClasses.get(targetVersion), true))
+ .setProgramConsumer(new ArchiveConsumer(featureCode, false))
+ .build())
+ .compile()
+ .inspect(this::checkLambdaGroups);
+
+ // Run the code without the feature code present.
+ compileResult
+ .run(parameters.getRuntime(), baseKtClassName)
+ .assertSuccessWithOutputLines("1", "2");
+
+ // Run the code with the feature code present.
+ compileResult
+ .addRunClasspathFiles(featureCode)
+ .run(parameters.getRuntime(), baseKtClassName)
+ .assertSuccessWithOutputLines("1", "2", "3", "4");
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/FeatureAPI.java b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/FeatureAPI.java
new file mode 100644
index 0000000..95bc790
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/FeatureAPI.java
@@ -0,0 +1,27 @@
+// Copyright (c) 2020, 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.kotlin.lambda.b148525512;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class FeatureAPI {
+ public static boolean hasFeature() {
+ try {
+ Class.forName(FeatureAPI.class.getPackage().getName() + ".FeatureKt");
+ } catch (ClassNotFoundException e) {
+ return false;
+ }
+ return true;
+ }
+
+ public static void feature(int i)
+ throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException,
+ IllegalAccessException {
+ Class<?> featureKtClass = Class.forName(FeatureAPI.class.getPackage().getName() + ".FeatureKt");
+ Method featureMethod = featureKtClass.getMethod("feature", int.class);
+ featureMethod.invoke(null, i);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/base.kt b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/base.kt
new file mode 100644
index 0000000..aced216
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/base.kt
@@ -0,0 +1,18 @@
+// Copyright (c) 2020, 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.kotlin.lambda.b148525512
+
+fun printInt(l: () -> Int ) = println(l())
+
+open class Base(@JvmField var x: Int, @JvmField var y: Int)
+
+fun main(args: Array<String>) {
+ val base = Base(args.size + 1, args.size + 2)
+ printInt { base.x }
+ printInt { base.y }
+ if (FeatureAPI.hasFeature()) {
+ FeatureAPI.feature(args.size + 3)
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/feature.kt b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/feature.kt
new file mode 100644
index 0000000..3480225
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/b148525512/feature.kt
@@ -0,0 +1,13 @@
+// Copyright (c) 2020, 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.kotlin.lambda.b148525512
+
+class Feature(x: Int, y: Int) : Base(x, y)
+
+fun feature(i: Int) {
+ val f = Feature(i, i + 1)
+ printInt { f.x }
+ printInt { f.y }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastCfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastCfInstructionSubject.java
new file mode 100644
index 0000000..0f5c42c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastCfInstructionSubject.java
@@ -0,0 +1,27 @@
+// Copyright (c) 2020, 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.utils.codeinspector;
+
+import com.android.tools.r8.cf.code.CfCheckCast;
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.graph.DexType;
+
+public class CheckCastCfInstructionSubject extends CfInstructionSubject
+ implements CheckCastInstructionSubject {
+ public CheckCastCfInstructionSubject(CfInstruction instruction, MethodSubject method) {
+ super(instruction, method);
+ assert isCheckCast();
+ }
+
+ @Override
+ public DexType getType() {
+ return ((CfCheckCast) instruction).getType();
+ }
+
+ @Override
+ public CheckCastInstructionSubject asCheckCast() {
+ return this;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastDexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastDexInstructionSubject.java
new file mode 100644
index 0000000..ea10ad4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastDexInstructionSubject.java
@@ -0,0 +1,27 @@
+// Copyright (c) 2020, 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.utils.codeinspector;
+
+import com.android.tools.r8.code.CheckCast;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.graph.DexType;
+
+public class CheckCastDexInstructionSubject extends DexInstructionSubject
+ implements CheckCastInstructionSubject {
+ public CheckCastDexInstructionSubject(Instruction instruction, MethodSubject method) {
+ super(instruction, method);
+ assert isCheckCast();
+ }
+
+ @Override
+ public DexType getType() {
+ return ((CheckCast) instruction).getType();
+ }
+
+ @Override
+ public CheckCastInstructionSubject asCheckCast() {
+ return this;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastInstructionSubject.java
new file mode 100644
index 0000000..e7fa916
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CheckCastInstructionSubject.java
@@ -0,0 +1,11 @@
+// Copyright (c) 2020, 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.utils.codeinspector;
+
+import com.android.tools.r8.graph.DexType;
+
+public interface CheckCastInstructionSubject extends InstructionSubject {
+ DexType getType();
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
index d589f41..757216c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -354,6 +354,8 @@
return new NewInstanceDexInstructionSubject(instruction, method);
} else if (dexInst.isConstString(JumboStringMode.ALLOW)) {
return new ConstStringDexInstructionSubject(instruction, method);
+ } else if (dexInst.isCheckCast()) {
+ return new CheckCastDexInstructionSubject(instruction, method);
} else {
return dexInst;
}
@@ -369,6 +371,8 @@
return new NewInstanceCfInstructionSubject(instruction, method);
} else if (cfInst.isConstString(JumboStringMode.ALLOW)) {
return new ConstStringCfInstructionSubject(instruction, method);
+ } else if (cfInst.isCheckCast()) {
+ return new CheckCastCfInstructionSubject(instruction, method);
} else {
return cfInst;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index ebe2f02..114581c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -84,6 +84,10 @@
boolean isCheckCast(String type);
+ default CheckCastInstructionSubject asCheckCast() {
+ return null;
+ }
+
boolean isInstanceOf();
boolean isInstanceOf(String type);
