Fix missing class inlining of lambdas
Change-Id: I195a8eaf221f623d3002457f2cce65d8c0290028
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 9ccc4c2..5041e1d 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -1129,6 +1129,11 @@
} else {
assert appView.graphLense().isIdentityLense();
}
+
+ if (lambdaRewriter != null) {
+ lambdaRewriter.desugarLambdas(method, code);
+ assert code.isConsistentSSA();
+ }
}
if (lambdaMerger != null) {
@@ -1290,10 +1295,6 @@
stringConcatRewriter.desugarStringConcats(method.method, code);
- if (lambdaRewriter != null) {
- lambdaRewriter.desugarLambdas(method, code);
- assert code.isConsistentSSA();
- }
previous = printMethod(code, "IR after lambda desugaring (SSA)", previous);
assert code.verifyTypes(appView);
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
index 28a70b2..e7ba0b5 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
@@ -277,7 +277,7 @@
for (int i = 0; i < fieldCount; i++) {
FieldAccessFlags accessFlags =
FieldAccessFlags.fromSharedAccessFlags(
- Constants.ACC_FINAL | Constants.ACC_SYNTHETIC | Constants.ACC_PRIVATE);
+ Constants.ACC_FINAL | Constants.ACC_SYNTHETIC | Constants.ACC_PUBLIC);
fields[i] = new DexEncodedField(
getCaptureField(i), accessFlags, DexAnnotationSet.empty(), null);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 7f488a1..582de88 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -196,11 +196,13 @@
// TrivialInstanceInitializer. This will be checked in areInstanceUsersEligible(...).
// There must be no static initializer on the class itself.
- if (eligibleClassDefinition.hasClassInitializer()) {
+ if (eligibleClassDefinition.classInitializationMayHaveSideEffects(
+ appView,
+ // Types that are a super type of the current context are guaranteed to be initialized.
+ type -> appView.isSubtype(method.method.holder, type).isTrue())) {
return EligibilityStatus.HAS_CLINIT;
- } else {
- return EligibilityStatus.ELIGIBLE;
}
+ return EligibilityStatus.ELIGIBLE;
}
assert root.isStaticGet();
@@ -861,13 +863,7 @@
return null; // Don't inline itself.
}
- if (isDesugaredLambda && !singleTarget.accessFlags.isBridge()) {
- markSizeForInlining(invoke, singleTarget);
- return new InliningInfo(singleTarget, eligibleClass);
- }
-
MethodOptimizationInfo optimizationInfo = singleTarget.getOptimizationInfo();
-
ClassInlinerEligibilityInfo eligibility = optimizationInfo.getClassInlinerEligibility();
if (eligibility == null) {
return null;
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index 0084e78..8154dfc 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -102,7 +102,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = false)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 179, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 137, "lambdadesugaring"))
.run();
test("lambdadesugaring", "lambdadesugaring", "LambdaDesugaring")
@@ -110,10 +110,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- // TODO(b/120814598): Should be 24. Some lambdas are not class inlined because parameter
- // usages for lambda methods are not present for the class inliner.
- // TODO(b/141719453): Also, some are not inined due to instruction limits.
- .withDexCheck(inspector -> checkLambdaCount(inspector, 37, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 7, "lambdadesugaring"))
.run();
}
@@ -145,7 +142,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = false)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 179, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 137, "lambdadesugaring"))
.run();
test("lambdadesugaring", "lambdadesugaring", "LambdaDesugaring")
@@ -153,10 +150,7 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- // TODO(b/120814598): Should be 24. Some lambdas are not class inlined because parameter
- // usages for lambda methods are not present for the class inliner.
- // TODO(b/141719453): Also, some are not inined due to instruction limits.
- .withDexCheck(inspector -> checkLambdaCount(inspector, 37, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 7, "lambdadesugaring"))
.run();
}
@@ -170,7 +164,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 33, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 31, "lambdadesugaringnplus"))
.run();
test("lambdadesugaringnplus", "lambdadesugaringnplus", "LambdasWithStaticAndDefaultMethods")
@@ -180,10 +174,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- // TODO(b/120814598): Should be 5. Some lambdas are not class inlined because parameter
- // usages for lambda methods are not present for the class inliner.
- // TODO(b/141719453): Also, some are not inined due to instruction limits.
- .withDexCheck(inspector -> checkLambdaCount(inspector, 24, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaringnplus"))
.run();
}
@@ -197,7 +188,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 33, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 31, "lambdadesugaringnplus"))
.run();
test("lambdadesugaringnplus", "lambdadesugaringnplus", "LambdasWithStaticAndDefaultMethods")
@@ -207,10 +198,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- // TODO(b/120814598): Should be 5. Some lambdas are not class inlined because parameter
- // usages for lambda methods are not present for the class inliner.
- // TODO(b/141719453): Also, some are not inined due to instruction limits.
- .withDexCheck(inspector -> checkLambdaCount(inspector, 24, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaringnplus"))
.run();
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
index 82385a2..fc3f22f 100644
--- a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
@@ -338,11 +338,15 @@
"classmerging.LambdaRewritingTest",
"classmerging.LambdaRewritingTest$Function",
"classmerging.LambdaRewritingTest$InterfaceImpl");
- runTest(
+ runTestOnInput(
main,
- programFiles,
+ readProgramFiles(programFiles),
name -> preservedClassNames.contains(name) || name.contains("$Lambda$"),
- getProguardConfig(JAVA8_EXAMPLE_KEEP));
+ getProguardConfig(JAVA8_EXAMPLE_KEEP),
+ options -> {
+ this.configure(options);
+ options.enableClassInlining = false;
+ });
}
@Test
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index 3ad4a37..1058615 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -11,6 +11,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -317,12 +318,8 @@
.filter(name -> name.contains(LAMBDA_CLASS_NAME_PREFIX))
.collect(Collectors.toList()));
assertEquals(expectedTypes, collectTypes(clazz.uniqueMethodWithName("testStatefulLambda")));
-
- // TODO(b/120814598): Should be 0. Lambdas are not class inlined because parameter usage is not
- // available for each lambda constructor.
- assertEquals(
- 3,
- inspector.allClasses().stream().filter(ClassSubject::isSynthesizedJavaLambdaClass).count());
+ assertTrue(
+ inspector.allClasses().stream().noneMatch(ClassSubject::isSynthesizedJavaLambdaClass));
}
private String getProguardConfig(String main) {
