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.
2019-09-05 16:45:05 +02:00
parent 530c1411c3
commit 6ef434a711
9 changed files with 80 additions and 50 deletions
@@ -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() {
@@ -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 = "<path>", 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
    }

@@ -23,4 +23,7 @@ object JvmAnalysisFlags {

    @JvmStatic
    val suppressMissingBuiltinsError by AnalysisFlag.Delegates.Boolean
+
+    @JvmStatic
+    val irCheckLocalNames by AnalysisFlag.Delegates.Boolean
 }
@@ -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 {
@@ -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
@@ -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) {
@@ -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<JvmBackendContext>(
+    { 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)
+    }
+}
@@ -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<JvmBackendContext>(
-    { 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)
-    }
-}
@@ -22,6 +22,7 @@ where advanced options include:
  -Xdisable-standard-script  Disable standard kotlin script support
  -Xfriend-paths=<path>      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=<path>       Paths where to find Java 9+ modules
  -Xjava-package-prefix      Package prefix for Java files
  -Xjava-source-roots=<path> Paths to directories with Java source files