From 5bc8c1a6002fa663c1f091fe3b1f308d82333e24 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 28 Jan 2022 12:51:37 +0300 Subject: [PATCH] IR refactor patch / validate declaration parent --- .../kotlin/backend/common/IrValidator.kt | 74 ++++++++++++------- .../backend/common/phaser/DumperVerifier.kt | 9 +-- .../jetbrains/kotlin/backend/jvm/JvmLower.kt | 36 ++------- .../kotlin/ir/util/PatchDeclarationParents.kt | 62 ++-------------- 4 files changed, 65 insertions(+), 116 deletions(-) diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/IrValidator.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/IrValidator.kt index 2753f343f8b..4714a51c3f5 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/IrValidator.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/IrValidator.kt @@ -18,17 +18,20 @@ package org.jetbrains.kotlin.backend.common import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.* +import org.jetbrains.kotlin.ir.util.DeclarationParentsVisitor +import org.jetbrains.kotlin.ir.util.dump import org.jetbrains.kotlin.ir.util.render -import org.jetbrains.kotlin.ir.visitors.IrElementVisitor import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid +@Suppress("unused") fun validateIrFile(context: CommonBackendContext, irFile: IrFile) { val visitor = IrValidator(context, IrValidatorConfig(abortOnError = false, ensureAllNodesAreDifferent = false)) irFile.acceptVoid(visitor) } +@Suppress("unused") fun validateIrModule(context: CommonBackendContext, irModule: IrModuleFragment) { val visitor = IrValidator( context, @@ -92,31 +95,50 @@ class IrValidator(val context: CommonBackendContext, val config: IrValidatorConf } } -fun IrModuleFragment.checkDeclarationParents() { - this.accept(CheckDeclarationParentsVisitor, null) -} - -object CheckDeclarationParentsVisitor : IrElementVisitor { - - override fun visitElement(element: IrElement, data: IrDeclarationParent?) { - element.acceptChildren(this, element as? IrDeclarationParent ?: data) - } - - override fun visitDeclaration(declaration: IrDeclarationBase, data: IrDeclarationParent?) { - checkParent(declaration, data) - super.visitDeclaration(declaration, data) - } - - private fun checkParent(declaration: IrDeclaration, expectedParent: IrDeclarationParent?) { - val parent = try { - declaration.parent - } catch (e: Throwable) { - error("$declaration for ${declaration.render()} has no parent") - } - - if (parent != expectedParent) { - error("$declaration for ${declaration.render()} has unexpected parent $parent") - } +fun IrElement.checkDeclarationParents() { + val checker = CheckDeclarationParentsVisitor() + acceptVoid(checker) + if (checker.errors.isNotEmpty()) { + val expectedParents = LinkedHashSet() + throw AssertionError( + buildString { + append("Declarations with wrong parent: ") + append(checker.errors.size) + append("\n") + checker.errors.forEach { + append("declaration: ") + append(it.declaration.render()) + append("\n\t") + append(it.declaration) + append("\nexpectedParent: ") + append(it.expectedParent.render()) + append("\nactualParent: ") + append(it.actualParent?.render()) + append("\n") + expectedParents.add(it.expectedParent) + } + append("\nExpected parents:\n") + expectedParents.forEach { + append(it.dump()) + } + } + ) } } +class CheckDeclarationParentsVisitor : DeclarationParentsVisitor() { + class Error(val declaration: IrDeclaration, val expectedParent: IrDeclarationParent, val actualParent: IrDeclarationParent?) + + val errors = ArrayList() + + override fun handleParent(declaration: IrDeclaration, parent: IrDeclarationParent) { + try { + val actualParent = declaration.parent + if (actualParent != parent) { + errors.add(Error(declaration, parent, actualParent)) + } + } catch (e: Exception) { + errors.add(Error(declaration, parent, null)) + } + } +} diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/phaser/DumperVerifier.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/phaser/DumperVerifier.kt index 99be6b1044a..5537f565197 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/phaser/DumperVerifier.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/phaser/DumperVerifier.kt @@ -5,10 +5,7 @@ package org.jetbrains.kotlin.backend.common.phaser -import org.jetbrains.kotlin.backend.common.CheckDeclarationParentsVisitor -import org.jetbrains.kotlin.backend.common.CommonBackendContext -import org.jetbrains.kotlin.backend.common.IrValidator -import org.jetbrains.kotlin.backend.common.IrValidatorConfig +import org.jetbrains.kotlin.backend.common.* import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.util.dump @@ -60,7 +57,7 @@ fun makeVerifyAction(verifier: (Context, Data) -> Unit): Action< fun dumpIrElement(actionState: ActionState, data: IrElement, @Suppress("UNUSED_PARAMETER") context: Any?): String { val beforeOrAfterStr = actionState.beforeOrAfter.name.toLowerCaseAsciiOnly() - var dumpText: String = "" + var dumpText = "" val elementName: String val dumpStrategy = System.getProperty("org.jetbrains.kotlin.compiler.ir.dump.strategy") @@ -143,7 +140,7 @@ fun validationCallback(context: CommonBackendContext, fra checkProperties = checkProperties, ) fragment.accept(IrValidator(context, validatorConfig), null) - fragment.accept(CheckDeclarationParentsVisitor, null) + fragment.checkDeclarationParents() } val validationAction = makeVerifyAction(::validationCallback) diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index 096f349e5f3..3b2add6df8c 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.backend.jvm import org.jetbrains.kotlin.backend.common.CommonBackendContext import org.jetbrains.kotlin.backend.common.FileLoweringPass +import org.jetbrains.kotlin.backend.common.checkDeclarationParents import org.jetbrains.kotlin.backend.common.lower.* import org.jetbrains.kotlin.backend.common.lower.loops.forLoopsPhase import org.jetbrains.kotlin.backend.common.lower.optimizations.foldConstantLoweringPhase @@ -19,7 +20,9 @@ import org.jetbrains.kotlin.descriptors.DescriptorVisibility import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.symbols.IrValueSymbol -import org.jetbrains.kotlin.ir.util.* +import org.jetbrains.kotlin.ir.util.PatchDeclarationParentsVisitor +import org.jetbrains.kotlin.ir.util.isAnonymousObject +import org.jetbrains.kotlin.ir.util.parentAsClass import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid @@ -28,6 +31,7 @@ import org.jetbrains.kotlin.name.NameUtils private var patchParentPhases = 0 +@Suppress("unused") private fun makePatchParentsPhase(): NamedCompilerPhase { val number = patchParentPhases++ return makeIrFilePhase( @@ -39,6 +43,7 @@ private fun makePatchParentsPhase(): NamedCompilerPhase { val number = checkParentPhases++ return makeIrFilePhase( @@ -56,34 +61,7 @@ private class PatchDeclarationParents : FileLoweringPass { private class CheckDeclarationParents : FileLoweringPass { override fun lower(irFile: IrFile) { - val checker = CheckDeclarationParentsVisitor() - irFile.acceptVoid(checker) - if (checker.errors.isNotEmpty()) { - val expectedParents = LinkedHashSet() - throw AssertionError( - buildString { - append("Declarations with wrong parent: ") - append(checker.errors.size) - append("\n") - checker.errors.forEach { - append("declaration: ") - append(it.declaration.render()) - append("\n\t") - append(it.declaration) - append("\nexpectedParent: ") - append(it.expectedParent.render()) - append("\nactualParent: ") - append(it.actualParent?.render()) - append("\n") - expectedParents.add(it.expectedParent) - } - append("\nExpected parents:\n") - expectedParents.forEach { - append(it.dump()) - } - } - ) - } + irFile.checkDeclarationParents() } } diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/PatchDeclarationParents.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/PatchDeclarationParents.kt index 520d87d136a..89f891cfce4 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/PatchDeclarationParents.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/util/PatchDeclarationParents.kt @@ -11,7 +11,6 @@ import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid import org.jetbrains.kotlin.ir.visitors.acceptVoid import java.util.* -import kotlin.collections.ArrayList fun T.patchDeclarationParents(initialParent: IrDeclarationParent? = null) = apply { @@ -19,13 +18,8 @@ fun T.patchDeclarationParents(initialParent: IrDeclarationParent acceptVoid(visitor) } -class PatchDeclarationParentsVisitor() : IrElementVisitorVoid { - - constructor(containingDeclaration: IrDeclarationParent) : this() { - declarationParentsStack.push(containingDeclaration) - } - - private val declarationParentsStack = ArrayDeque() +abstract class DeclarationParentsVisitor : IrElementVisitorVoid { + protected val declarationParentsStack = ArrayDeque() override fun visitElement(element: IrElement) { element.acceptChildrenVoid(this) @@ -38,7 +32,7 @@ class PatchDeclarationParentsVisitor() : IrElementVisitorVoid { } override fun visitDeclaration(declaration: IrDeclarationBase) { - patchParent(declaration) + handleParent(declaration, declarationParentsStack.peekFirst()) if (declaration is IrDeclarationParent) { declarationParentsStack.push(declaration) @@ -51,58 +45,16 @@ class PatchDeclarationParentsVisitor() : IrElementVisitorVoid { } } - private fun patchParent(declaration: IrDeclaration) { - declaration.parent = declarationParentsStack.peekFirst() - } + protected abstract fun handleParent(declaration: IrDeclaration, parent: IrDeclarationParent) } - -class CheckDeclarationParentsVisitor() : IrElementVisitorVoid { +class PatchDeclarationParentsVisitor() : DeclarationParentsVisitor() { constructor(containingDeclaration: IrDeclarationParent) : this() { declarationParentsStack.push(containingDeclaration) } - private val declarationParentsStack = ArrayDeque() - - class Data(val declaration: IrDeclaration, val expectedParent: IrDeclarationParent, val actualParent: IrDeclarationParent?) - - val errors = ArrayList() - - override fun visitElement(element: IrElement) { - element.acceptChildrenVoid(this) - } - - override fun visitPackageFragment(declaration: IrPackageFragment) { - declarationParentsStack.push(declaration) - super.visitPackageFragment(declaration) - declarationParentsStack.pop() - } - - override fun visitDeclaration(declaration: IrDeclarationBase) { - checkParent(declaration) - - if (declaration is IrDeclarationParent) { - declarationParentsStack.push(declaration) - } - - super.visitDeclaration(declaration) - - if (declaration is IrDeclarationParent) { - declarationParentsStack.pop() - } - } - - private fun checkParent(declaration: IrDeclaration) { - val expectedParent = declarationParentsStack.peekFirst() - try { - val actualParent = declaration.parent - if (actualParent != expectedParent) { - errors.add(Data(declaration, expectedParent, actualParent)) - } - } catch (e: Exception) { - errors.add(Data(declaration, expectedParent, null)) - } - + override fun handleParent(declaration: IrDeclaration, parent: IrDeclarationParent) { + declaration.parent = parent } }