React to review comments

2018-11-26 16:05:47 +03:00
parent c9343c89bc
commit c018109bca
14 changed files with 72 additions and 102 deletions
@@ -22,7 +22,7 @@ class RenderIrElementWithDescriptorsVisitor : IrElementVisitor<String, Nothing?>
            "MODULE_FRAGMENT ${declaration.descriptor}"

    override fun visitFile(declaration: IrFile, data: Nothing?): String =
-            "FILE ${declaration.name}"
+            "FILE ${declaration.path}"

    override fun visitFunction(declaration: IrFunction, data: Nothing?): String =
            "FUN ${declaration.descriptor}"
@@ -155,8 +155,7 @@ val IrFunctionReference.isSuspend get() = (symbol.owner as? IrSimpleFunction)?.i

 fun IrValueParameter.copyTo(
    irFunction: IrFunction,
-    shift: Int = 0,
-    index: Int? = null,
+    index: Int = this.index,
    startOffset: Int = this.startOffset,
    endOffset: Int = this.endOffset,
    origin: IrDeclarationOrigin = this.origin,
@@ -164,16 +163,13 @@ fun IrValueParameter.copyTo(
    type: IrType = this.type.remapTypeParameters(this.parent as IrTypeParametersContainer, irFunction),
    varargElementType: IrType? = this.varargElementType
 ): IrValueParameter {
-    // You cannot specify both index and nontrivial shift.
-    assert(index == null || shift == 0)
-    val newIndex = index ?: (shift + this.index)
    val descriptor = WrappedValueParameterDescriptor(symbol.descriptor.annotations, symbol.descriptor.source)
    val symbol = IrValueParameterSymbolImpl(descriptor)
    val defaultValueCopy = defaultValue?.deepCopyWithVariables()
    defaultValueCopy?.patchDeclarationParents(irFunction)
    return IrValueParameterImpl(
        startOffset, endOffset, origin, symbol,
-        name, newIndex, type, varargElementType, isCrossinline, isNoinline
+        name, index, type, varargElementType, isCrossinline, isNoinline
    ).also {
        descriptor.bind(it)
        it.parent = irFunction
@@ -208,7 +204,7 @@ fun IrFunction.copyParameterDeclarationsFrom(from: IrFunction) {
    extensionReceiverParameter = from.extensionReceiverParameter?.copyTo(this)

    val shift = valueParameters.size
-    valueParameters += from.valueParameters.map { it.copyTo(this, shift) }
+    valueParameters += from.valueParameters.map { it.copyTo(this, index = it.index + shift) }
 }

 fun IrTypeParametersContainer.copyTypeParametersFrom(
@@ -246,22 +242,22 @@ fun IrFunction.copyValueParametersToStatic(
    assert(target.valueParameters.isEmpty())

    var shift = 0
-    source.dispatchReceiverParameter?.apply {
+    source.dispatchReceiverParameter?.let { p ->
        target.valueParameters.add(
-            copyTo(
+            p.copyTo(
                target,
-                origin = origin,
-                shift = shift++,
+                origin = p.origin,
+                index = p.index + shift++,
                name = Name.identifier("\$this")
            )
        )
    }
-    source.extensionReceiverParameter?.apply {
+    source.extensionReceiverParameter?.let { p ->
        target.valueParameters.add(
-            copyTo(
+            p.copyTo(
                target,
-                origin = origin,
-                shift = shift++,
+                origin = p.origin,
+                index = p.index + shift++,
                name = Name.identifier("\$receiver")
            )
        )
@@ -271,7 +267,7 @@ fun IrFunction.copyValueParametersToStatic(
            oldValueParameter.copyTo(
                target,
                origin = origin,
-                shift = shift
+                index = oldValueParameter.index + shift
            )
        )
    }
@@ -132,8 +132,8 @@ open class DefaultArgumentStubGenerator constructor(val context: CommonBackendCo
                    symbol = irFunction.symbol, descriptor = irFunction.symbol.descriptor,
                    typeArgumentsCount = irFunction.typeParameters.size
                ).apply {
-                    (0 until typeArgumentsCount).forEach { i ->
-                        putTypeArgument(i, newIrFunction.typeParameters[i].defaultType)
+                    newIrFunction.typeParameters.forEachIndexed { i, param ->
+                        putTypeArgument(i, param.defaultType)
                    }
                    dispatchReceiver = newIrFunction.dispatchReceiverParameter?.let { irGet(it) }

@@ -141,8 +141,8 @@ open class DefaultArgumentStubGenerator constructor(val context: CommonBackendCo
                }
            } else {
                +irReturn(irCall(irFunction).apply {
-                    (0 until typeArgumentsCount).forEach { i ->
-                        putTypeArgument(i, newIrFunction.typeParameters[i].defaultType)
+                    newIrFunction.typeParameters.forEachIndexed { i, param ->
+                        putTypeArgument(i, param.defaultType)
                    }
                    dispatchReceiver = newIrFunction.dispatchReceiverParameter?.let { irGet(it) }
                    extensionReceiver = newIrFunction.extensionReceiverParameter?.let { irGet(it) }
@@ -255,10 +255,10 @@ class InlineClassLowering(val context: BackendContext) {
            dispatchReceiverParameter = null
            extensionReceiverParameter = function.extensionReceiverParameter?.copyTo(this)
            if (function is IrSimpleFunction) {
-                valueParameters.add(function.dispatchReceiverParameter!!.copyTo(this, shift = 1))
-                valueParameters += function.valueParameters.map { p -> p.copyTo(this, shift = 1) }
+                valueParameters.add(function.dispatchReceiverParameter!!.let { p -> p.copyTo(this, index = p.index + 1) })
+                valueParameters += function.valueParameters.map { p -> p.copyTo(this, index = p.index + 1) }
            } else {
-                valueParameters += function.valueParameters.map { p -> p.copyTo(this, shift = 0) }
+                valueParameters += function.valueParameters.map { p -> p.copyTo(this) }
            }
            parent = function.parent
            assert(isStaticMethodOfClass)
@@ -554,7 +554,7 @@ class LocalDeclarationsLowering(
            }

            oldDeclaration.valueParameters.mapTo(this) { v ->
-                v.copyTo(newDeclaration, capturedValues.size).also {
+                v.copyTo(newDeclaration, index = v.index + capturedValues.size).also {
                    newParameterToOld.putAbsentOrSame(it, v)
                }
            }
@@ -112,7 +112,7 @@ class JsDeclarationFactory : DeclarationFactory {
        val newValueParameters = mutableListOf(outerThisValueParameter)

        for (p in oldConstructor.valueParameters) {
-            newValueParameters += p.copyTo(newConstructor, 1)
+            newValueParameters += p.copyTo(newConstructor, index = p.index + 1)
        }

        newConstructor.valueParameters += newValueParameters
@@ -66,7 +66,7 @@ class ModuleIndex(val module: IrModuleFragment) {

            override fun visitDeclaration(declaration: IrDeclaration) {
                super.visitDeclaration(declaration)
-                declarationToFile[declaration.descriptor] = currentFile!!.name
+                declarationToFile[declaration.descriptor] = currentFile!!.path
            }
        })
    }
@@ -7,13 +7,13 @@ package org.jetbrains.kotlin.ir.backend.js.transformers.irToJs

 import org.jetbrains.kotlin.ir.backend.js.utils.JsGenerationContext
 import org.jetbrains.kotlin.ir.declarations.IrFile
-import org.jetbrains.kotlin.ir.declarations.name
+import org.jetbrains.kotlin.ir.declarations.path
 import org.jetbrains.kotlin.js.backend.ast.JsDeclarationScope
 import org.jetbrains.kotlin.js.backend.ast.JsStatement

 class IrFileToJsTransformer : BaseIrElementToJsNodeTransformer<JsStatement, JsGenerationContext> {
    override fun visitFile(declaration: IrFile, context: JsGenerationContext): JsStatement {
-        val fileContext = context.newDeclaration(JsDeclarationScope(context.currentScope, "scope for file ${declaration.name}"))
+        val fileContext = context.newDeclaration(JsDeclarationScope(context.currentScope, "scope for file ${declaration.path}"))
        val block = fileContext.currentBlock

        declaration.declarations.forEach {
@@ -53,12 +53,11 @@ fun JvmBackendContext.getSourceMapper(declaration: IrClass): DefaultSourceMapper
    //       whole file the class is declared in rather than the class only.
    // TODO: revise
    val endLineNumber = fileEntry.getSourceRangeInfo(0, fileEntry.maxOffset).endLineNumber
-    val shortName = File(declaration.fileParent.name).name
    return DefaultSourceMapper(
        SourceInfo.createInfoForIr(
            endLineNumber + 1,
            this.state.typeMapper.mapType(declaration.descriptor).internalName,
-            shortName
+            declaration.fileParent.name
        )
    )
 }
@@ -9,24 +9,24 @@ import org.jetbrains.kotlin.backend.common.ClassLoweringPass
 import org.jetbrains.kotlin.backend.common.makePhase
 import org.jetbrains.kotlin.backend.common.descriptors.WrappedSimpleFunctionDescriptor
 import org.jetbrains.kotlin.backend.common.ir.copyParameterDeclarationsFrom
+import org.jetbrains.kotlin.backend.common.lower.createIrBuilder
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin
-import org.jetbrains.kotlin.backend.jvm.JvmLoweredStatementOrigin
 import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
 import org.jetbrains.kotlin.codegen.OwnerKind
 import org.jetbrains.kotlin.codegen.state.GenerationState
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.descriptors.deserialization.PLATFORM_DEPENDENT_ANNOTATION_FQ_NAME
 import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
+import org.jetbrains.kotlin.ir.builders.irBlockBody
+import org.jetbrains.kotlin.ir.builders.irCall
+import org.jetbrains.kotlin.ir.builders.irGet
+import org.jetbrains.kotlin.ir.builders.irReturn
 import org.jetbrains.kotlin.ir.declarations.IrClass
 import org.jetbrains.kotlin.ir.declarations.IrDeclarationOrigin
 import org.jetbrains.kotlin.ir.declarations.IrFunction
 import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction
 import org.jetbrains.kotlin.ir.declarations.impl.IrFunctionImpl
-import org.jetbrains.kotlin.ir.expressions.impl.IrBlockBodyImpl
-import org.jetbrains.kotlin.ir.expressions.impl.IrCallImpl
-import org.jetbrains.kotlin.ir.expressions.impl.IrGetValueImpl
-import org.jetbrains.kotlin.ir.expressions.impl.IrReturnImpl
 import org.jetbrains.kotlin.ir.symbols.impl.IrSimpleFunctionSymbolImpl
 import org.jetbrains.kotlin.ir.util.hasAnnotation
 import org.jetbrains.kotlin.ir.util.isInterface
@@ -53,31 +53,32 @@ class InterfaceDelegationLowering(val context: JvmBackendContext) : IrElementTra
        } else {
            Pair(irClass, false)
        }
-        for ((interfaceFun, value) in actualClass.getNonPrivateInterfaceMethods()) {
-            //skip java 8 default methods
-            if (!interfaceFun.isDefinitelyNotDefaultImplsMethod() && !interfaceFun.isMethodOfAny()) {
-                generateDelegationToDefaultImpl(irClass, interfaceFun, value, isDefaultImplsGeneration)
+
+        val newDeclarations = mutableListOf<IrFunction>()
+        for (function in actualClass.declarations) {
+            if (function !is IrSimpleFunction) continue
+            if (function.origin !== IrDeclarationOrigin.FAKE_OVERRIDE) continue
+
+            val implementation = function.resolveFakeOverride() ?: continue
+            if (!implementation.hasInterfaceParent() ||
+                Visibilities.isPrivate(implementation.visibility) ||
+                implementation.visibility === Visibilities.INVISIBLE_FAKE ||
+                implementation.isDefinitelyNotDefaultImplsMethod() || implementation.isMethodOfAny()
+            ) {
+                continue
            }
+
+            newDeclarations.add(generateDelegationToDefaultImpl(implementation, function, isDefaultImplsGeneration))
        }

-//        CodegenUtil.getNonPrivateTraitMethods(actualClass.descriptor, !isDefaultImplsGeneration)) {
-//            //skip java 8 default methods
-//            if (!interfaceFun.isDefinitelyNotDefaultImplsMethod() && !FunctionCodegen.isMethodOfAny(interfaceFun)) {
-//                generateDelegationToDefaultImpl(
-//                    irClass, context.ir.symbols.externalSymbolTable.referenceSimpleFunction(
-//                        interfaceFun.original
-//                    ).owner, value, isDefaultImplsGeneration
-//                )
-//            }
-//        }
+        irClass.declarations.addAll(newDeclarations)
    }

    private fun generateDelegationToDefaultImpl(
-        irClass: IrClass,
-        interfaceFun: IrFunction,
+        interfaceFun: IrSimpleFunction,
        inheritedFun: IrSimpleFunction,
        isDefaultImplsGeneration: Boolean
-    ) {
+    ): IrFunction {
        val defaultImplFun = context.declarationFactory.getDefaultImplsFunction(interfaceFun)

        val irFunction =
@@ -109,55 +110,30 @@ class InterfaceDelegationLowering(val context: JvmBackendContext) : IrElementTra
                    copyParameterDeclarationsFrom(inheritedFun)
                }
            } else context.declarationFactory.getDefaultImplsFunction(inheritedFun)
-        val irBody = IrBlockBodyImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET)
-        irFunction.body = irBody
-        irClass.declarations.add(irFunction)

-        val irCallImpl =
-            IrCallImpl(
-                UNDEFINED_OFFSET,
-                UNDEFINED_OFFSET,
-                defaultImplFun.returnType,
-                defaultImplFun.symbol,
-                defaultImplFun.descriptor,
-                origin = JvmLoweredStatementOrigin.DEFAULT_IMPLS_DELEGATION
-            )
-        irBody.statements.add(
-            IrReturnImpl(
-                UNDEFINED_OFFSET,
-                UNDEFINED_OFFSET,
-                irFunction.returnType,
-                irFunction.symbol,
-                irCallImpl
-            )
-        )
-
-        var offset = 0
-        irFunction.dispatchReceiverParameter?.let {
-            irCallImpl.putValueArgument(
-                offset,
-                IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, it.symbol)
-            )
-            offset++
+        context.createIrBuilder(irFunction.symbol, UNDEFINED_OFFSET, UNDEFINED_OFFSET).apply {
+            irFunction.body = irBlockBody {
+                +irReturn(
+                    irCall(defaultImplFun.symbol, irFunction.returnType).apply {
+                        var offset = 0
+                        irFunction.dispatchReceiverParameter?.let { putValueArgument(offset++, irGet(it)) }
+                        irFunction.extensionReceiverParameter?.let { putValueArgument(offset++, irGet(it)) }
+                        irFunction.valueParameters.mapIndexed { i, parameter -> putValueArgument(i + offset, irGet(parameter)) }
+                    }
+                )
+            }
        }

-        irFunction.extensionReceiverParameter?.let {
-            irCallImpl.putValueArgument(
-                offset,
-                IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, it.symbol)
-            )
-            offset++
-        }
-
-        irFunction.valueParameters.mapIndexed { i, parameter ->
-            irCallImpl.putValueArgument(i + offset, IrGetValueImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, parameter.symbol, null))
-        }
+        return irFunction
    }

    private fun IrSimpleFunction.isMethodOfAny() =
        ((valueParameters.size == 0 && name.asString() in setOf("hashCode", "toString")) ||
                (valueParameters.size == 1 && name.asString() == "equals" && valueParameters[0].type == context.irBuiltIns.anyType))

+    private fun IrSimpleFunction.hasInterfaceParent() =
+        (parent as? IrClass)?.isInterface == true
+
    private fun IrSimpleFunction.isDefinitelyNotDefaultImplsMethod() =
        resolveFakeOverride()?.let { origin == IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB } == true ||
                hasAnnotation(PLATFORM_DEPENDENT_ANNOTATION_FQ_NAME)
@@ -100,7 +100,7 @@ internal fun createStaticFunctionWithReceivers(
            index = offset++
        )
        valueParameters.addAll(listOfNotNull(dispatchReceiver, extensionReceiver) +
-                                       oldFunction.valueParameters.map { it.copyTo(this, shift = offset) }
+                                       oldFunction.valueParameters.map { it.copyTo(this, index = it.index + offset) }
        )

        val mapping: Map<IrValueParameter, IrValueParameter> =
@@ -25,6 +25,7 @@ import org.jetbrains.kotlin.ir.symbols.IrFileSymbol
 import org.jetbrains.kotlin.ir.symbols.IrPackageFragmentSymbol
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformer
 import org.jetbrains.kotlin.name.FqName
+import java.io.File

 interface IrPackageFragment : IrElement, IrDeclarationContainer, IrSymbolOwner {
    val packageFragmentDescriptor: PackageFragmentDescriptor
@@ -47,4 +48,5 @@ interface IrFile : IrPackageFragment, IrAnnotationContainer {
        accept(transformer, data) as IrFile
 }

-val IrFile.name: String get() = fileEntry.name
+val IrFile.path: String get() = fileEntry.name
+val IrFile.name: String get() = File(path).name
@@ -49,7 +49,7 @@ class RenderIrElementVisitor : IrElementVisitor<String, Nothing?> {
        "EXTERNAL_PACKAGE_FRAGMENT fqName:${declaration.fqName}"

    override fun visitFile(declaration: IrFile, data: Nothing?): String =
-        "FILE fqName:${declaration.fqName} fileName:${declaration.name}"
+        "FILE fqName:${declaration.fqName} fileName:${declaration.path}"

    override fun visitFunction(declaration: IrFunction, data: Nothing?): String =
        "FUN ${declaration.renderOriginIfNonTrivial()}${declaration.renderDeclared()}"
@@ -16,10 +16,7 @@

 package org.jetbrains.kotlin.ir

-import org.jetbrains.kotlin.ir.declarations.IrFile
-import org.jetbrains.kotlin.ir.declarations.IrFunction
-import org.jetbrains.kotlin.ir.declarations.IrModuleFragment
-import org.jetbrains.kotlin.ir.declarations.name
+import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir2cfg.generators.FunctionGenerator
 import org.jetbrains.kotlin.ir2cfg.util.dump
 import org.jetbrains.kotlin.test.KotlinTestUtils
@@ -45,9 +42,9 @@ abstract class AbstractIrCfgTestCase : AbstractIrGeneratorTestCase() {
    private fun IrModuleFragment.cfgDump(): String {
        val builder = StringBuilder()
        for (file in this.files) {
-            builder.appendln("// FILE: ${file.name}")
+            builder.appendln("// FILE: ${file.path}")
            builder.appendln(file.cfgDump())
-            builder.appendln("// END FILE: ${file.name}")
+            builder.appendln("// END FILE: ${file.path}")
            builder.appendln()
        }
        return builder.toString()