[JS IR BE] Fix validation errors (duplicate nodes and incorrect parent)

2020-03-12 19:55:40 +03:00
parent d36d62e226
commit cb15570d75
5 changed files with 56 additions and 20 deletions
@@ -111,17 +111,17 @@ open class DefaultArgumentStubGenerator(
                        val defaultFlag =
                            irCallOp(this@DefaultArgumentStubGenerator.context.ir.symbols.intAnd, context.irBuiltIns.intType, mask, bit)

-                        val expressionBody = valueParameter.defaultValue!!
-                        expressionBody.patchDeclarationParents(newIrFunction)
-                        expressionBody.transformChildrenVoid(object : IrElementTransformerVoid() {
-                            override fun visitGetValue(expression: IrGetValue): IrExpression {
-                                log { "GetValue: ${expression.symbol.owner}" }
-                                val valueSymbol = variables[expression.symbol.owner] ?: return expression
-                                return irGet(valueSymbol)
-                            }
-                        })
+                        val expression = valueParameter.defaultValue!!.expression
+                            .prepareToBeUsedIn(newIrFunction)
+                            .transform(object : IrElementTransformerVoid() {
+                                override fun visitGetValue(expression: IrGetValue): IrExpression {
+                                    log { "GetValue: ${expression.symbol.owner}" }
+                                    val valueSymbol = variables[expression.symbol.owner] ?: return expression
+                                    return irGet(valueSymbol)
+                                }
+                            }, null)

-                        selectArgumentOrDefault(defaultFlag, parameter, expressionBody.expression)
+                        selectArgumentOrDefault(defaultFlag, parameter, expression)
                    } else {
                        parameter
                    }
@@ -147,6 +147,28 @@ open class DefaultArgumentStubGenerator(
        return listOf(irFunction, newIrFunction)
    }

+    /**
+     * Prepares the default value to be used inside the `function` body by patching the parents.
+     * In K/JS it also copies the expression in order to avoid duplicate declarations after this lowering.
+     *
+     * In K/JVM copying doesn't preserve metadata, so the following case won't work:
+     *
+     * ```
+     *   import kotlin.reflect.jvm.reflect
+     *
+     *   fun foo(x: Function<*> = {}) {
+     *       // Will print "null" if lambda is copied
+     *       println(x.reflect())
+     *   }
+     * ```
+     *
+     * Thus the duplicate declarations during the lowering pipeline is considered to be a lesser evil.
+     */
+    protected open fun IrExpression.prepareToBeUsedIn(function: IrFunction): IrExpression {
+        return patchDeclarationParents(function)
+    }
+
+
    protected open fun IrBlockBodyBuilder.selectArgumentOrDefault(
        defaultFlag: IrExpression,
        parameter: IrValueParameter,
@@ -407,12 +407,9 @@ class EnumClassCreateInitializerLowering(val context: JsIrBackendContext) : Decl

                    irClass.enumEntries.forEach { entry ->
                        entry.correspondingField?.let { instanceField ->
-                            +irSetField(null, instanceField, entry.initializerExpression!!.expression)
+                            +irSetField(null, instanceField, entry.initializerExpression!!.expression.deepCopyWithSymbols(it))
                        }
                    }
-                }.also {
-                    // entry.initializerExpression can have local declarations
-                    it.acceptVoid(PatchDeclarationParentsVisitor(irClass))
                }.statements
            }
        }
@@ -6,6 +6,7 @@
 package org.jetbrains.kotlin.ir.backend.js.lower

 import org.jetbrains.kotlin.backend.common.BodyLoweringPass
+import org.jetbrains.kotlin.backend.common.deepCopyWithVariables
 import org.jetbrains.kotlin.backend.common.ir.isOverridableOrOverrides
 import org.jetbrains.kotlin.backend.common.lower.DefaultArgumentStubGenerator
 import org.jetbrains.kotlin.backend.common.lower.DEFAULT_DISPATCH_CALL
@@ -19,6 +20,7 @@ import org.jetbrains.kotlin.ir.expressions.*
 import org.jetbrains.kotlin.ir.expressions.impl.IrCallImpl
 import org.jetbrains.kotlin.ir.expressions.impl.IrFunctionReferenceImpl
 import org.jetbrains.kotlin.ir.util.deepCopyWithSymbols
+import org.jetbrains.kotlin.ir.util.patchDeclarationParents
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid
 import org.jetbrains.kotlin.name.Name
@@ -44,6 +46,12 @@ class JsDefaultArgumentStubGenerator(override val context: JsIrBackendContext) :
        }
    }

+    override fun IrExpression.prepareToBeUsedIn(function: IrFunction): IrExpression {
+        return deepCopyWithVariables().also {
+            it.patchDeclarationParents(function)
+        }
+    }
+
    private fun resolveInvoke(paramCount: Int): IrSimpleFunction {
        assert(paramCount > 0)
        val functionKlass = context.ir.symbols.functionN(paramCount).owner
@@ -13,6 +13,7 @@ import org.jetbrains.kotlin.ir.builders.*
 import org.jetbrains.kotlin.ir.builders.declarations.buildFun
 import org.jetbrains.kotlin.ir.builders.declarations.buildValueParameter
 import org.jetbrains.kotlin.ir.declarations.IrDeclaration
+import org.jetbrains.kotlin.ir.declarations.IrDeclarationParent
 import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
 import org.jetbrains.kotlin.ir.declarations.IrValueParameter
 import org.jetbrains.kotlin.ir.expressions.*
@@ -35,11 +36,9 @@ class PropertyReferenceLowering(private val context: JsIrBackendContext) : BodyL

    private val throwISE = context.throwISEsymbol

-    private val newDeclarations = mutableListOf<IrDeclaration>()
-
    override fun lower(irBody: IrBody, container: IrDeclaration) {
-        newDeclarations.clear()
-        irBody.transformChildrenVoid(PropertyReferenceTransformer())
+        val currentParent = container as? IrDeclarationParent ?: container.parent
+        val newDeclarations = PropertyReferenceTransformer(currentParent).process(irBody)
        if (!newDeclarations.isEmpty()) {
            val file = container.file
            newDeclarations.forEach { it.parent = file }
@@ -47,7 +46,14 @@ class PropertyReferenceLowering(private val context: JsIrBackendContext) : BodyL
        }
    }

-    private inner class PropertyReferenceTransformer : IrElementTransformerVoid() {
+    private inner class PropertyReferenceTransformer(var currentParent: IrDeclarationParent) : IrElementTransformerVoid() {
+
+        val newDeclarations = mutableListOf<IrDeclaration>()
+
+        fun process(irBody: IrBody): List<IrDeclaration> {
+            irBody.transformChildrenVoid(this)
+            return newDeclarations
+        }

        private fun buildFactoryFunction(reference: IrPropertyReference): IrSimpleFunction {
            val property = reference.symbol.owner
@@ -240,7 +246,7 @@ class PropertyReferenceLowering(private val context: JsIrBackendContext) : BodyL
                name = Name.identifier("${delegatedVar.name}\$stub")
            }

-            function.parent = delegatedVar.parent
+            function.parent = currentParent

            function.body = with(context.createIrBuilder(function.symbol)) {
                irBlockBody {
@@ -18,6 +18,7 @@ import org.jetbrains.kotlin.ir.expressions.impl.*
 import org.jetbrains.kotlin.ir.symbols.*
 import org.jetbrains.kotlin.ir.types.IrType
 import org.jetbrains.kotlin.ir.util.explicitParameters
+import org.jetbrains.kotlin.ir.util.patchDeclarationParents
 import org.jetbrains.kotlin.ir.visitors.*
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.utils.DFS
@@ -173,6 +174,8 @@ class JsSuspendFunctionsLowering(ctx: JsIrBackendContext) : AbstractSuspendFunct
        }

        stateMachineFunction.transform(LiveLocalsTransformer(localToPropertyMap, { JsIrBuilder.buildGetValue(thisReceiver) }, unit), null)
+        // TODO find out why some parents are incorrect
+        stateMachineFunction.body!!.patchDeclarationParents(stateMachineFunction)
    }

    private fun assignStateIds(entryState: SuspendState, subject: IrVariableSymbol, switch: IrWhen, rootLoop: IrLoop) {