From 6ef434a711f9d751b4f71174a1319f8dcd02c03b Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 5 Sep 2019 16:45:05 +0200 Subject: [PATCH] JVM IR: add -Xir-check-local-names to check names for consistency Since KotlinTypeMapper is no longer used in the JVM IR backend, we need not run CodegenBinding.initTrace and check that names of local entities are exactly equal to local names computed by that algorithm. However, it's still useful as an opt-in flag, to discover issues where unwanted elements take part in the naming (such as temporary IR variables, see for example cb2e68fece). So we introduce a new command line argument -Xir-check-local-names which, when the IR backend is used (via -Xuse-ir), launches the name computation algorithm from the old backend and then compares that the names are exactly equal to the names computed by the IR backend in InventNamesForLocalClasses. --- .../kotlin/codegen/state/GenerationState.kt | 4 +- .../arguments/K2JVMCompilerArguments.kt | 7 +++ .../kotlin/config/JvmAnalysisFlags.kt | 3 + .../jetbrains/kotlin/backend/common/Lower.kt | 6 ++ .../jetbrains/kotlin/backend/jvm/JvmLower.kt | 3 +- .../backend/jvm/codegen/ClassCodegen.kt | 5 +- .../lower/CheckLocalNamesWithOldBackend.kt | 57 +++++++++++++++++++ .../lower/RecordNamesForKotlinTypeMapper.kt | 44 -------------- compiler/testData/cli/jvm/extraHelp.out | 1 + 9 files changed, 80 insertions(+), 50 deletions(-) create mode 100644 compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/CheckLocalNamesWithOldBackend.kt delete mode 100644 compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RecordNamesForKotlinTypeMapper.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt index 4fba5b9755d..86a5c808a40 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt @@ -297,7 +297,9 @@ class GenerationState private constructor( fun beforeCompile() { markUsed() - CodegenBinding.initTrace(this) + if (!isIrBackend || languageVersionSettings.getFlag(JvmAnalysisFlags.irCheckLocalNames)) { + CodegenBinding.initTrace(this) + } } fun afterIndependentPart() { diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt index fc3fdd527a1..bdcd9e56dc4 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/K2JVMCompilerArguments.kt @@ -75,6 +75,12 @@ class K2JVMCompilerArguments : CommonCompilerArguments() { @Argument(value = "-Xuse-ir", description = "Use the IR backend") var useIR: Boolean by FreezableVar(false) + @Argument( + value = "-Xir-check-local-names", + description = "Check that names of local classes and anonymous objects are the same in the IR backend as in the old backend" + ) + var irCheckLocalNames: Boolean by FreezableVar(false) + @Argument(value = "-Xmodule-path", valueDescription = "", description = "Paths where to find Java 9+ modules") var javaModulePath: String? by NullableStringFreezableVar(null) @@ -298,6 +304,7 @@ class K2JVMCompilerArguments : CommonCompilerArguments() { result[JvmAnalysisFlags.inheritMultifileParts] = inheritMultifileParts result[JvmAnalysisFlags.sanitizeParentheses] = sanitizeParentheses result[JvmAnalysisFlags.suppressMissingBuiltinsError] = suppressMissingBuiltinsError + result[JvmAnalysisFlags.irCheckLocalNames] = irCheckLocalNames return result } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/config/JvmAnalysisFlags.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/config/JvmAnalysisFlags.kt index 90c4afa6636..9e4edd61074 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/config/JvmAnalysisFlags.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/config/JvmAnalysisFlags.kt @@ -23,4 +23,7 @@ object JvmAnalysisFlags { @JvmStatic val suppressMissingBuiltinsError by AnalysisFlag.Delegates.Boolean + + @JvmStatic + val irCheckLocalNames by AnalysisFlag.Delegates.Boolean } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/Lower.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/Lower.kt index 12a1a782e18..43a86009cb1 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/Lower.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/Lower.kt @@ -25,6 +25,12 @@ import org.jetbrains.kotlin.ir.visitors.acceptVoid interface FileLoweringPass { fun lower(irFile: IrFile) + + object Empty : FileLoweringPass { + override fun lower(irFile: IrFile) { + // Do nothing + } + } } interface ClassLoweringPass : FileLoweringPass { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index 6828a469771..75b24db2d37 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt @@ -146,6 +146,7 @@ private val returnableBlocksPhase = makeIrFilePhase( prerequisite = setOf(arrayConstructorPhase, assertionPhase) ) +@Suppress("Reformat") private val jvmFilePhases = stripTypeAliasDeclarationsPhase then provisionalFunctionExpressionPhase then @@ -220,7 +221,7 @@ private val jvmFilePhases = typeOperatorLowering then replaceKFunctionInvokeWithFunctionInvokePhase then - recordNamesForKotlinTypeMapperPhase then + checkLocalNamesWithOldBackendPhase then // should be last transformation removeDeclarationsThatWouldBeInlined then diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt index 14dd042bd1f..6f4895b13b0 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ClassCodegen.kt @@ -135,10 +135,7 @@ open class ClassCodegen protected constructor( private fun generateKotlinMetadataAnnotation() { val localDelegatedProperties = (irClass.attributeOwnerId as? IrClass)?.let(context.localDelegatedProperties::get) if (localDelegatedProperties != null && localDelegatedProperties.isNotEmpty()) { - // Remove this check once CodegenAnnotatingVisitor is no longer used in JVM IR - if (state.bindingContext.get(CodegenBinding.DELEGATED_PROPERTIES, type).isNullOrEmpty()) { - state.bindingTrace.record(CodegenBinding.DELEGATED_PROPERTIES, type, localDelegatedProperties.map { it.descriptor }) - } + state.bindingTrace.record(CodegenBinding.DELEGATED_PROPERTIES, type, localDelegatedProperties.map { it.descriptor }) } when (val metadata = irClass.metadata) { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/CheckLocalNamesWithOldBackend.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/CheckLocalNamesWithOldBackend.kt new file mode 100644 index 00000000000..a28d1d1ee34 --- /dev/null +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/CheckLocalNamesWithOldBackend.kt @@ -0,0 +1,57 @@ +/* + * Copyright 2010-2019 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.backend.jvm.lower + +import org.jetbrains.kotlin.backend.common.FileLoweringPass +import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase +import org.jetbrains.kotlin.backend.jvm.JvmBackendContext +import org.jetbrains.kotlin.codegen.binding.CodegenBinding +import org.jetbrains.kotlin.config.JvmAnalysisFlags +import org.jetbrains.kotlin.ir.IrElement +import org.jetbrains.kotlin.ir.declarations.IrClass +import org.jetbrains.kotlin.ir.declarations.IrFile +import org.jetbrains.kotlin.ir.util.render +import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid +import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid +import org.jetbrains.kotlin.ir.visitors.acceptVoid + +val checkLocalNamesWithOldBackendPhase = makeIrFilePhase( + { context -> + if (context.state.languageVersionSettings.getFlag(JvmAnalysisFlags.irCheckLocalNames)) + CheckLocalNamesWithOldBackend(context) + else + FileLoweringPass.Empty + }, + name = "CheckLocalNamesWithOldBackend", + description = "With -Xir-check-local-names, check that names for local classes and anonymous objects are the same in the IR backend as in the old backend" +) + +class CheckLocalNamesWithOldBackend(private val context: JvmBackendContext) : FileLoweringPass, IrElementVisitorVoid { + override fun lower(irFile: IrFile) { + irFile.acceptVoid(this) + } + + override fun visitClass(declaration: IrClass) { + val actualName = context.getLocalClassInfo(declaration)?.internalName + if (actualName != null) { + val expectedName = context.state.bindingTrace.get(CodegenBinding.ASM_TYPE, declaration.symbol.descriptor)?.internalName + if (expectedName != null && expectedName != actualName) { + throw AssertionError( + "Incorrect name for the class.\n" + + "IR: ${declaration.render()}\n" + + "Descriptor: ${declaration.descriptor}\n" + + "Expected name: $expectedName\n" + + "Actual name: $actualName" + ) + } + } + super.visitClass(declaration) + } + + override fun visitElement(element: IrElement) { + element.acceptChildrenVoid(this) + } +} diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RecordNamesForKotlinTypeMapper.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RecordNamesForKotlinTypeMapper.kt deleted file mode 100644 index 44e186ca8e8..00000000000 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RecordNamesForKotlinTypeMapper.kt +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2010-2019 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.backend.jvm.lower - -import org.jetbrains.kotlin.backend.common.FileLoweringPass -import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase -import org.jetbrains.kotlin.backend.jvm.JvmBackendContext -import org.jetbrains.kotlin.codegen.binding.CodegenBinding -import org.jetbrains.kotlin.ir.IrElement -import org.jetbrains.kotlin.ir.declarations.IrClass -import org.jetbrains.kotlin.ir.declarations.IrFile -import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid -import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid -import org.jetbrains.kotlin.ir.visitors.acceptVoid -import org.jetbrains.org.objectweb.asm.Type - -val recordNamesForKotlinTypeMapperPhase = makeIrFilePhase( - { context -> RecordNamesForKotlinTypeMapper(context) }, - name = "RecordNamesForKotlinTypeMapper", - description = "Record local class and anonymous object names for KotlinTypeMapper to work correctly" -) - -class RecordNamesForKotlinTypeMapper(private val context: JvmBackendContext) : FileLoweringPass, IrElementVisitorVoid { - override fun lower(irFile: IrFile) { - irFile.acceptVoid(this) - } - - override fun visitClass(declaration: IrClass) { - val internalName = context.getLocalClassInfo(declaration)?.internalName - if (internalName != null) { - // If this line fails, it means that the name invented by the JVM IR backend in InventNamesForLocalClasses is not equal - // to the name invented by the old backend in CodegenAnnotatingVisitor. The former should likely be fixed. - context.state.bindingTrace.record(CodegenBinding.ASM_TYPE, declaration.symbol.descriptor, Type.getObjectType(internalName)) - } - super.visitClass(declaration) - } - - override fun visitElement(element: IrElement) { - element.acceptChildrenVoid(this) - } -} diff --git a/compiler/testData/cli/jvm/extraHelp.out b/compiler/testData/cli/jvm/extraHelp.out index 442682b2ea5..6e2ebb111dd 100644 --- a/compiler/testData/cli/jvm/extraHelp.out +++ b/compiler/testData/cli/jvm/extraHelp.out @@ -22,6 +22,7 @@ where advanced options include: -Xdisable-standard-script Disable standard kotlin script support -Xfriend-paths= Paths to output directories for friend modules (whose internals should be visible) -Xmultifile-parts-inherit Compile multifile classes as a hierarchy of parts and facade + -Xir-check-local-names Check that names of local classes and anonymous objects are the same in the IR backend as in the old backend -Xmodule-path= Paths where to find Java 9+ modules -Xjava-package-prefix Package prefix for Java files -Xjava-source-roots= Paths to directories with Java source files