From 73aa36ca59c38f4cf73d09c09a95b0af763ee8fb Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Mon, 27 Jan 2020 14:09:44 +0100 Subject: [PATCH] JVM: Add D8 check that class files can be dexed with D8. This change adds D8 in addition to Dx which is being deprecated. At some point after the official Dx deprecation, we should remove the Dx checker. Moving to D8 has the additional benefit that D8 can dex code using java 8 features without using a separate desugaring tool. --- build.gradle.kts | 2 + .../codegen/box/classes/quotedClassName.kt | 5 ++ .../box/localClasses/nameWithWhitespace.kt | 8 +++ compiler/tests-common/build.gradle.kts | 1 + .../kotlin/codegen/CodegenTestCase.java | 9 ++- .../jetbrains/kotlin/codegen/D8Checker.java | 70 +++++++++++++++++++ .../kotlin/codegen/TestStdlibWithDxTest.kt | 6 +- 7 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 compiler/tests-common/tests/org/jetbrains/kotlin/codegen/D8Checker.java diff --git a/build.gradle.kts b/build.gradle.kts index b7c0a139cac..2b96f8618c7 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -173,6 +173,7 @@ extra["versions.jflex"] = "1.7.0" extra["versions.markdown"] = "0.1.25" extra["versions.trove4j"] = "1.0.20181211" extra["versions.completion-ranking-kotlin"] = "0.0.2" +extra["versions.r8"] = "1.5.70" // NOTE: please, also change KTOR_NAME in pathUtil.kt and all versions in corresponding jar names in daemon tests. extra["versions.ktor-network"] = "1.0.1" @@ -339,6 +340,7 @@ allprojects { maven("https://dl.bintray.com/kotlin/ktor") maven("https://kotlin.bintray.com/kotlin-dependencies") maven("https://jetbrains.bintray.com/intellij-third-party-dependencies") + maven("https://dl.google.com/dl/android/maven2") bootstrapKotlinRepo?.let(::maven) internalKotlinRepo?.let(::maven) } diff --git a/compiler/testData/codegen/box/classes/quotedClassName.kt b/compiler/testData/codegen/box/classes/quotedClassName.kt index 179ad998494..0c6a90ac5d3 100644 --- a/compiler/testData/codegen/box/classes/quotedClassName.kt +++ b/compiler/testData/codegen/box/classes/quotedClassName.kt @@ -1,6 +1,11 @@ // IGNORE_BACKEND: JS_IR // IGNORE_BACKEND: JS +// Exclamation marks are not valid in names in the dex file format. +// Therefore, do not attemp to dex this file as it will fail. +// See: https://source.android.com/devices/tech/dalvik/dex-format#simplename +// IGNORE_DEXING + class `A!u00A0`() { val ok = "OK" } diff --git a/compiler/testData/codegen/box/localClasses/nameWithWhitespace.kt b/compiler/testData/codegen/box/localClasses/nameWithWhitespace.kt index 2bdaf3a9a12..a51545f53d9 100644 --- a/compiler/testData/codegen/box/localClasses/nameWithWhitespace.kt +++ b/compiler/testData/codegen/box/localClasses/nameWithWhitespace.kt @@ -1,6 +1,14 @@ // IGNORE_BACKEND: JS_IR // IGNORE_BACKEND: JS +// Names with spaces are not valid according to the dex specification +// before DEX version 040. Therefore, do not attempt to dex the resulting +// class file. When D8 is updated to the most recent version, we can increase +// the min android api level during dexing to allow these spaces. +// +// See: https://source.android.com/devices/tech/dalvik/dex-format#simplename +// IGNORE_DEXING + fun `method with spaces`(): String { data class C(val s: String = "OK") return C().s diff --git a/compiler/tests-common/build.gradle.kts b/compiler/tests-common/build.gradle.kts index 669f5bde6cb..9e8f6001965 100644 --- a/compiler/tests-common/build.gradle.kts +++ b/compiler/tests-common/build.gradle.kts @@ -41,6 +41,7 @@ dependencies { testCompile(project(":kotlin-scripting-compiler-impl")) testCompile(commonDep("junit:junit")) testCompile(androidDxJar()) { isTransitive = false } + testCompile(commonDep("com.android.tools:r8")) testCompileOnly(intellijCoreDep()) { includeJars("intellij-core") } testCompile(intellijDep()) { includeJars( diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/CodegenTestCase.java b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/CodegenTestCase.java index 0d3aa614f12..5137fb6e6c9 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/CodegenTestCase.java +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/CodegenTestCase.java @@ -534,8 +534,15 @@ public abstract class CodegenTestCase extends KtUsefulTestCase { ); classFileFactory = generationState.getFactory(); - if (verifyWithDex() && DxChecker.RUN_DX_CHECKER) { + // Some names are not allowed in the dex file format and the VM will reject the program + // if they are used. Therefore, a few tests cannot be dexed as they use such names that + // are valid on the JVM but not on the Android Runtime. + boolean ignoreDexing = myFiles.getPsiFiles().stream().anyMatch( + it -> InTextDirectivesUtils.isDirectiveDefined(it.getText(), "IGNORE_DEXING") + ); + if (verifyWithDex() && DxChecker.RUN_DX_CHECKER && !ignoreDexing) { DxChecker.check(classFileFactory); + D8Checker.check(classFileFactory); } } catch (TestsCompiletimeError e) { diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/D8Checker.java b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/D8Checker.java new file mode 100644 index 00000000000..bdea96035c4 --- /dev/null +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/D8Checker.java @@ -0,0 +1,70 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.codegen; + +import com.android.tools.r8.*; +import com.android.tools.r8.origin.PathOrigin; +import com.android.tools.r8.utils.AndroidApiLevel; +import com.intellij.concurrency.IdeaForkJoinWorkerThreadFactory; +import kotlin.Pair; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.kotlin.backend.common.output.OutputFile; +import org.junit.Assert; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.nio.file.Paths; +import java.util.Collection; +import java.util.concurrent.Executors; +import java.util.function.Consumer; + +public class D8Checker { + + private D8Checker() { + } + + public static void check(ClassFileFactory outputFiles) { + runD8(builder -> { + for (OutputFile file : ClassFileUtilsKt.getClassFiles(outputFiles)) { + byte[] bytes = file.asByteArray(); + builder.addClassProgramData(bytes, new PathOrigin(Paths.get(file.getRelativePath()))); + } + }); + } + + public static void checkFilesWithD8(Collection> classFiles) { + runD8(builder -> { + classFiles.forEach(pair -> { + builder.addClassProgramData(pair.getFirst(), new PathOrigin(Paths.get(pair.getSecond()))); + }); + }); + } + + private static void runD8(Consumer addInput) { + ProgramConsumer ignoreOutputConsumer = new DexIndexedConsumer.ForwardingConsumer(null); + D8Command.Builder builder = D8Command.builder() + .setMinApiLevel(28) + .setMode(CompilationMode.DEBUG) + .setProgramConsumer(ignoreOutputConsumer); + addInput.accept(builder); + try { + D8.run(builder.build(), Executors.newSingleThreadExecutor()); + } + catch (CompilationFailedException e) { + Assert.fail(generateExceptionMessage(e)); + } + } + + private static String generateExceptionMessage(Throwable e) { + StringWriter writer = new StringWriter(); + try (PrintWriter printWriter = new PrintWriter(writer)) { + e.printStackTrace(printWriter); + String stackTrace = writer.toString(); + return stackTrace; + } + } +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/TestStdlibWithDxTest.kt b/compiler/tests/org/jetbrains/kotlin/codegen/TestStdlibWithDxTest.kt index 79157abe4a6..e0d80f87884 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/TestStdlibWithDxTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/codegen/TestStdlibWithDxTest.kt @@ -34,12 +34,16 @@ class TestStdlibWithDxTest { } private fun doTest(file: File) { + val files = mutableListOf>(); ZipInputStream(FileInputStream(file)).use { zip -> for (entry in generateSequence { zip.nextEntry }) { if (entry.name.endsWith(".class") && !entry.name.startsWith("META-INF/")) { - DxChecker.checkFileWithDx(zip.readBytes(), entry.name) + val bytes = zip.readBytes() + DxChecker.checkFileWithDx(bytes, entry.name) + files.add(Pair(bytes, entry.name)) } } } + D8Checker.checkFilesWithD8(files) } }