Track invalid kotlin metadata
Bug: 155536535
Bug: 154346948
Change-Id: I83cba818fa2534f7b7751c35a52a5f95bf594372
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
index f52f65e..a1ad70f 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.kotlin;
+import static com.android.tools.r8.kotlin.KotlinMetadataUtils.INVALID_KOTLIN_INFO;
import static com.android.tools.r8.kotlin.KotlinMetadataUtils.NO_KOTLIN_INFO;
import com.android.tools.r8.graph.DexAnnotation;
@@ -36,6 +37,7 @@
+ clazz.type.toSourceString()
+ " has malformed kotlin.Metadata: "
+ e.getMessage()));
+ return INVALID_KOTLIN_INFO;
} catch (Throwable e) {
reporter.info(
new StringDiagnostic(
@@ -43,6 +45,7 @@
+ clazz.type.toSourceString()
+ "'s kotlin.Metadata: "
+ e.getMessage()));
+ return INVALID_KOTLIN_INFO;
}
}
return NO_KOTLIN_INFO;
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
index b3c167f..31818e8 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.kotlin;
+import static com.android.tools.r8.kotlin.KotlinMetadataUtils.INVALID_KOTLIN_INFO;
import static com.android.tools.r8.kotlin.KotlinMetadataUtils.NO_KOTLIN_INFO;
import com.android.tools.r8.graph.AppView;
@@ -73,9 +74,12 @@
KotlinClassLevelInfo kotlinInfo = clazz.getKotlinInfo();
DexAnnotation oldMeta =
clazz.annotations().getFirstMatching(kotlin.metadata.kotlinMetadataType);
+ if (kotlinInfo == INVALID_KOTLIN_INFO) {
+ // Maintain invalid kotlin info for classes.
+ return;
+ }
if (kotlinInfo == NO_KOTLIN_INFO) {
- // TODO(b/154346948): Track invalid meta-data objects such that we can enable this
- // assert oldMeta == null;
+ assert oldMeta == null;
return;
}
if (oldMeta != null) {
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
index fb173a2..e846c14 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
@@ -27,11 +27,23 @@
public class KotlinMetadataUtils {
- public static final NoKotlinInfo NO_KOTLIN_INFO = new NoKotlinInfo();
+ public static final NoKotlinInfo NO_KOTLIN_INFO = new NoKotlinInfo("NO_KOTLIN_INFO");
+ public static final NoKotlinInfo INVALID_KOTLIN_INFO = new NoKotlinInfo("INVALID_KOTLIN_INFO");
private static class NoKotlinInfo
implements KotlinClassLevelInfo, KotlinFieldLevelInfo, KotlinMethodLevelInfo {
+ private final String name;
+
+ private NoKotlinInfo(String name) {
+ this.name = name;
+ }
+
+ @Override
+ public String toString() {
+ return name;
+ }
+
@Override
public KotlinClassHeader rewrite(
DexClass clazz, AppView<AppInfoWithLiveness> appView, NamingLens namingLens) {
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClassInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClassInfo.java
index cc11647..af02060 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClassInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClassInfo.java
@@ -10,7 +10,6 @@
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Reporter;
-import kotlinx.metadata.InconsistentKotlinMetadataException;
import kotlinx.metadata.KmLambda;
import kotlinx.metadata.jvm.KotlinClassHeader;
import kotlinx.metadata.jvm.KotlinClassMetadata;
@@ -43,12 +42,8 @@
Reporter reporter) {
KmLambda lambda = null;
if (syntheticClass.isLambda()) {
- try {
- lambda = syntheticClass.toKmLambda();
- assert lambda != null;
- } catch (InconsistentKotlinMetadataException ex) {
- // TODO(b/155534905): Gracefully handle these errors by retaining the original object.
- }
+ lambda = syntheticClass.toKmLambda();
+ assert lambda != null;
}
return new KotlinSyntheticClassInfo(
lambda != null
diff --git a/src/test/java/com/android/tools/r8/KotlinTestBase.java b/src/test/java/com/android/tools/r8/KotlinTestBase.java
index 1a6a428..6cd9cfd 100644
--- a/src/test/java/com/android/tools/r8/KotlinTestBase.java
+++ b/src/test/java/com/android/tools/r8/KotlinTestBase.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import static org.hamcrest.CoreMatchers.containsString;
+
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.FileUtils;
@@ -12,6 +14,7 @@
import java.nio.file.Paths;
import java.util.List;
import java.util.stream.Collectors;
+import org.hamcrest.Matcher;
public abstract class KotlinTestBase extends TestBase {
@@ -66,4 +69,8 @@
protected Path getMappingfile(String folder, String mappingFileName) {
return Paths.get(ToolHelper.TESTS_DIR, RSRC, folder, mappingFileName);
}
+
+ protected static Matcher<String> expectedInfoMessagesFromKotlinStdLib() {
+ return containsString("No VersionRequirement");
+ }
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/ProcessKotlinStdlibTest.java b/src/test/java/com/android/tools/r8/kotlin/ProcessKotlinStdlibTest.java
index 4c45d3c..e3107bb 100644
--- a/src/test/java/com/android/tools/r8/kotlin/ProcessKotlinStdlibTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/ProcessKotlinStdlibTest.java
@@ -31,12 +31,15 @@
getTestParameters().withAllRuntimes().build(), KotlinTargetVersion.values());
}
- private void test(Collection<String> rules) throws Exception {
- test(rules, null);
+ private void test(Collection<String> rules, boolean expectInvalidFoo) throws Exception {
+ test(rules, expectInvalidFoo, null);
}
private void test(
- Collection<String> rules, ThrowableConsumer<R8FullTestBuilder> consumer) throws Exception {
+ Collection<String> rules,
+ boolean expectInvalidDebugInfo,
+ ThrowableConsumer<R8FullTestBuilder> consumer)
+ throws Exception {
testForR8(parameters.getBackend())
.addProgramFiles(ToolHelper.getKotlinStdlibJar())
.addKeepRules(rules)
@@ -44,58 +47,63 @@
.addKeepAttributes(ProguardKeepAttributes.INNER_CLASSES)
.addKeepAttributes(ProguardKeepAttributes.ENCLOSING_METHOD)
.apply(consumer)
- .compile();
+ .allowDiagnosticInfoMessages(expectInvalidDebugInfo)
+ .compile()
+ .assertAllInfoMessagesMatch(expectedInfoMessagesFromKotlinStdLib());
}
@Test
public void testAsIs() throws Exception {
- test(ImmutableList.of("-dontshrink", "-dontoptimize", "-dontobfuscate"));
+ test(ImmutableList.of("-dontshrink", "-dontoptimize", "-dontobfuscate"), true);
}
@Test
public void testDontShrinkAndDontOptimize() throws Exception {
- test(ImmutableList.of("-dontshrink", "-dontoptimize"));
+ test(ImmutableList.of("-dontshrink", "-dontoptimize"), true);
}
@Test
public void testDontShrinkAndDontOptimizeDifferently() throws Exception {
- test(
- ImmutableList.of("-keep,allowobfuscation class **.*Exception*"),
- tb -> {
- tb.noTreeShaking();
- tb.addOptionsModification(o -> {
- // Randomly choose a couple of optimizations.
- o.enableClassInlining = false;
- o.enableLambdaMerging = false;
- o.enableValuePropagation = false;
- });
- });
+ test(
+ ImmutableList.of("-keep,allowobfuscation class **.*Exception*"),
+ true,
+ tb -> {
+ tb.noTreeShaking();
+ tb.addOptionsModification(
+ o -> {
+ // Randomly choose a couple of optimizations.
+ o.enableClassInlining = false;
+ o.enableLambdaMerging = false;
+ o.enableValuePropagation = false;
+ });
+ });
}
@Test
public void testDontShrinkAndDontObfuscate() throws Exception {
- test(ImmutableList.of("-dontshrink", "-dontobfuscate"));
+ test(ImmutableList.of("-dontshrink", "-dontobfuscate"), true);
}
@Test
public void testDontShrink() throws Exception {
- test(ImmutableList.of("-dontshrink"));
+ test(ImmutableList.of("-dontshrink"), true);
}
@Test
public void testDontShrinkDifferently() throws Exception {
test(
ImmutableList.of("-keep,allowobfuscation class **.*Exception*"),
+ true,
tb -> tb.noTreeShaking());
}
@Test
public void testDontOptimize() throws Exception {
- test(ImmutableList.of("-dontoptimize"));
+ test(ImmutableList.of("-dontoptimize"), false);
}
@Test
public void testDontObfuscate() throws Exception {
- test(ImmutableList.of("-dontobfuscate"));
+ test(ImmutableList.of("-dontobfuscate"), false);
}
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java b/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
index f3feb33..87b5d10 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
@@ -3,13 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.kotlin.metadata;
-import static org.hamcrest.CoreMatchers.anyOf;
-import static org.hamcrest.CoreMatchers.containsString;
-
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.kotlin.AbstractR8KotlinTestBase;
import com.android.tools.r8.utils.DescriptorUtils;
-import org.hamcrest.Matcher;
abstract class KotlinMetadataTestBase extends AbstractR8KotlinTestBase {
@@ -31,8 +27,4 @@
static final String KT_FUNCTION1 = "Lkotlin/Function1;";
static final String KT_COMPARABLE = "Lkotlin/Comparable;";
-
- static Matcher<String> expectedInfoMessagesFromKotlinStdLib() {
- return anyOf(containsString("Invalid descriptor"), containsString("No VersionRequirement"));
- }
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
index 32a0e6b..e0ac745 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
@@ -50,12 +50,11 @@
.addProgramFiles(ToolHelper.getKotlinStdlibJar())
.setMinApi(parameters.getApiLevel())
.addKeepAllClassesRule()
- .addKeepRules("-keep class kotlin.Metadata")
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+ .allowDiagnosticInfoMessages()
.compile()
- // TODO(b/155536535): Enable this assert.
- // .assertAllInfoMessagesMatch(expectedInfoMessagesFromKotlinStdLib())
- // .assertInfosCount(5)
+ .assertAllInfoMessagesMatch(expectedInfoMessagesFromKotlinStdLib())
+ .assertInfosCount(3)
.inspect(this::inspect);
}
diff --git a/src/test/java/com/android/tools/r8/shaking/b134858535/EventPublisherTest.java b/src/test/java/com/android/tools/r8/shaking/b134858535/EventPublisherTest.java
index 29ab0f2..6d0d767 100644
--- a/src/test/java/com/android/tools/r8/shaking/b134858535/EventPublisherTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/b134858535/EventPublisherTest.java
@@ -20,7 +20,6 @@
@Test
public void testPrivateMethodsInLambdaClass() throws CompilationFailedException {
- // TODO(b/155534905): Update expectation.
testForR8(Backend.DEX)
.addProgramClasses(Main.class, Interface.class)
.addProgramClassFileData(EventPublisher$bDump.dump())
@@ -28,6 +27,5 @@
.addKeepMainRule(Main.class)
.setMinApi(AndroidApiLevel.L)
.compile();
- // .assertAllWarningMessagesMatch(containsString("Unrecognized Kotlin lambda"));
}
}
