From 47a1dd27dda5d158e4095b933b648b4832403a5e Mon Sep 17 00:00:00 2001 From: "Aleksei.Cherepanov" Date: Thu, 16 Sep 2021 00:30:16 +0300 Subject: [PATCH] Fix after review KT-CR-4441 of cc5382b3 Simplify API of InlineConstTracker In ConstLowering: Move transformer logic to another class to avoid mutable state. Avoid marking all files in module as module. Support of inner classes. #KT-46506 Fixed --- .../incremental/InlineConstTrackerImpl.kt | 10 ++-- .../CompilerCallbackServicesFacadeServer.kt | 5 +- .../common/CompilerCallbackServicesFacade.kt | 3 +- .../kotlin/daemon/RemoteInlineConstTracker.kt | 5 +- .../kotlin/backend/jvm/lower/ConstLowering.kt | 59 ++++++++----------- .../components/InlineConstTracker.kt | 26 +++++--- 6 files changed, 53 insertions(+), 55 deletions(-) diff --git a/build-common/src/org/jetbrains/kotlin/incremental/InlineConstTrackerImpl.kt b/build-common/src/org/jetbrains/kotlin/incremental/InlineConstTrackerImpl.kt index 708f4815ebe..39c25187769 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/InlineConstTrackerImpl.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/InlineConstTrackerImpl.kt @@ -6,15 +6,17 @@ package org.jetbrains.kotlin.incremental import org.jetbrains.kotlin.incremental.components.InlineConstTracker -import org.jetbrains.kotlin.incremental.components.ConstantRef +@Suppress("unused") class InlineConstTrackerImpl : InlineConstTracker { private val inlineConst = hashMapOf>() val inlineConstMap: Map> get() = inlineConst - override fun report(filePath: String, cRefs: Collection) { - inlineConst.getOrPut(filePath) { hashSetOf() }.addAll(cRefs) + override fun report(filePath: String, owner: String, name: String, constType: String) { + inlineConst.getOrPut(filePath) { hashSetOf() }.add(ConstantRef(owner, name, constType)) } -} \ No newline at end of file +} + +data class ConstantRef(var owner: String, var name: String, var constType: String) diff --git a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt index dddee83cb25..1cdc60d2c01 100644 --- a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt +++ b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt @@ -19,7 +19,6 @@ package org.jetbrains.kotlin.daemon.client import org.jetbrains.kotlin.daemon.common.* import org.jetbrains.kotlin.incremental.components.ExpectActualTracker import org.jetbrains.kotlin.incremental.components.InlineConstTracker -import org.jetbrains.kotlin.incremental.components.ConstantRef import org.jetbrains.kotlin.incremental.components.LookupInfo import org.jetbrains.kotlin.incremental.components.LookupTracker import org.jetbrains.kotlin.incremental.js.IncrementalDataProvider @@ -121,8 +120,8 @@ open class CompilerCallbackServicesFacadeServer( expectActualTracker!!.report(File(expectedFilePath), File(actualFilePath)) } - override fun inlineConstTracker_report(className: String, cRefs: Collection) { - inlineConstTracker?.report(className, cRefs) ?: throw NullPointerException("inlineConstTracker was not initialized") + override fun inlineConstTracker_report(filePath: String, owner: String, name: String, constType: String) { + inlineConstTracker?.report(filePath, owner, name, constType) ?: throw NullPointerException("inlineConstTracker was not initialized") } override fun incrementalResultsConsumer_processHeader(headerMetadata: ByteArray) { diff --git a/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/CompilerCallbackServicesFacade.kt b/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/CompilerCallbackServicesFacade.kt index ae97a65fbc1..647f008fa07 100644 --- a/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/CompilerCallbackServicesFacade.kt +++ b/compiler/daemon/daemon-common/src/org/jetbrains/kotlin/daemon/common/CompilerCallbackServicesFacade.kt @@ -17,7 +17,6 @@ package org.jetbrains.kotlin.daemon.common import org.jetbrains.kotlin.incremental.components.LookupInfo -import org.jetbrains.kotlin.incremental.components.ConstantRef import org.jetbrains.kotlin.incremental.js.JsInlineFunctionHash import org.jetbrains.kotlin.load.kotlin.incremental.components.JvmPackagePartProto import org.jetbrains.kotlin.modules.TargetId @@ -108,7 +107,7 @@ interface CompilerCallbackServicesFacade : Remote { // --------------------------------------------------- // InlineConstTracker @Throws(RemoteException::class) - fun inlineConstTracker_report(className: String, cRefs: Collection) + fun inlineConstTracker_report(filePath: String, owner: String, name: String, constType: String) // --------------------------------------------------- // IncrementalResultsConsumer (js) diff --git a/compiler/daemon/src/org/jetbrains/kotlin/daemon/RemoteInlineConstTracker.kt b/compiler/daemon/src/org/jetbrains/kotlin/daemon/RemoteInlineConstTracker.kt index 5f8cd40b98f..74d75f10add 100644 --- a/compiler/daemon/src/org/jetbrains/kotlin/daemon/RemoteInlineConstTracker.kt +++ b/compiler/daemon/src/org/jetbrains/kotlin/daemon/RemoteInlineConstTracker.kt @@ -9,15 +9,14 @@ import org.jetbrains.kotlin.daemon.common.DummyProfiler import org.jetbrains.kotlin.daemon.common.Profiler import org.jetbrains.kotlin.daemon.common.withMeasure import org.jetbrains.kotlin.incremental.components.InlineConstTracker -import org.jetbrains.kotlin.incremental.components.ConstantRef class RemoteInlineConstTracker( @Suppress("DEPRECATION") val facade: org.jetbrains.kotlin.daemon.common.CompilerCallbackServicesFacade, val profiler: Profiler = DummyProfiler() ): InlineConstTracker { - override fun report(filePath: String, cRefs: Collection) { + override fun report(filePath: String, owner: String, name: String, constType: String) { profiler.withMeasure(this) { - facade.inlineConstTracker_report(filePath, cRefs) + facade.inlineConstTracker_report(filePath, owner, name, constType) } } } \ No newline at end of file diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ConstLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ConstLowering.kt index 3a34752eaba..b2db3b006ae 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ConstLowering.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ConstLowering.kt @@ -9,21 +9,16 @@ 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.backend.jvm.ir.constantValue -import org.jetbrains.kotlin.ir.declarations.IrField -import org.jetbrains.kotlin.ir.declarations.IrFile -import org.jetbrains.kotlin.ir.declarations.IrSimpleFunction import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.IrCompositeImpl import org.jetbrains.kotlin.ir.expressions.impl.IrGetFieldImpl import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid -import org.jetbrains.kotlin.incremental.components.InlineConstTracker -import org.jetbrains.kotlin.incremental.components.ConstantRef import org.jetbrains.kotlin.config.CommonConfigurationKeys -import org.jetbrains.kotlin.descriptors.SourceFile -import org.jetbrains.kotlin.ir.descriptors.toIrBasedDescriptor -import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.ir.util.IdSignature -import org.jetbrains.kotlin.load.kotlin.toSourceElement +import org.jetbrains.kotlin.incremental.components.InlineConstTracker +import org.jetbrains.kotlin.ir.declarations.* +import org.jetbrains.kotlin.ir.util.classId +import org.jetbrains.kotlin.ir.util.parentAsClass +import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid internal val constPhase1 = makeIrFilePhase( ::ConstLowering, @@ -37,16 +32,22 @@ internal val constPhase2 = makeIrFilePhase( description = "Substitute calls to const properties with constant values" ) -class ConstLowering(val context: JvmBackendContext) : IrElementTransformerVoid(), FileLoweringPass { +class ConstLowering(val context: JvmBackendContext) : FileLoweringPass { val inlineConstTracker = - context.state.configuration[CommonConfigurationKeys.INLINE_CONST_TRACKER] ?: InlineConstTracker.DoNothing + context.state.configuration[CommonConfigurationKeys.INLINE_CONST_TRACKER] - override fun lower(irFile: IrFile) = irFile.transformChildrenVoid() + override fun lower(irFile: IrFile) = irFile.transformChildrenVoid(ConstTransformer(irFile, context, inlineConstTracker)) +} +private class ConstTransformer( + val irFile: IrFile, + val context: JvmBackendContext, + val inlineConstTracker: InlineConstTracker? +) : IrElementTransformerVoid() { private fun IrExpression.lowerConstRead(receiver: IrExpression?, field: IrField?): IrExpression? { val value = field?.constantValue() ?: return null transformChildrenVoid() - reportInlineConst(this@lowerConstRead, field) + reportInlineConst(field, value) val resultExpression = if (context.state.shouldInlineConstVals) value.copyWithOffsets(startOffset, endOffset) @@ -62,28 +63,16 @@ class ConstLowering(val context: JvmBackendContext) : IrElementTransformerVoid() ) } - private fun reportInlineConst(irExpression: IrExpression, field: IrField?) { - if (inlineConstTracker == InlineConstTracker.DoNothing) return - val value = field?.constantValue() ?: return + private fun reportInlineConst(field: IrField, value: IrConst<*>) { + if (inlineConstTracker == null) return + if (field.origin != IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB) return - if (!field.isFinal || !field.isStatic) return + val path = irFile.path + val owner = field.parentAsClass.classId?.asString()?.replace(".", "$")?.replace("/", ".") ?: return + val name = field.name.asString() + val constType = value.kind.asString - val sourceFile = field.toIrBasedDescriptor().containingDeclaration.toSourceElement.containingFile - if (sourceFile == SourceFile.NO_SOURCE_FILE || sourceFile.toString().lowercase().endsWith(".kt")) return - - for (file: KtFile in context.state.files) { - val fileName = file.virtualFilePath - val owner = - ((irExpression as? IrGetFieldImpl)?.symbol?.signature as? IdSignature.CompositeSignature)?.container?.asPublic()?.firstNameSegment - ?: continue - val name = field.name.asString() - val constType = value.kind.toString() - - inlineConstTracker.report( - fileName, - listOf(ConstantRef(owner, name, constType)) - ) - } + inlineConstTracker.report(path, owner, name, constType) } private fun IrExpression.shouldDropConstReceiver() = @@ -99,4 +88,4 @@ class ConstLowering(val context: JvmBackendContext) : IrElementTransformerVoid() override fun visitGetField(expression: IrGetField): IrExpression = expression.lowerConstRead(expression.receiver, expression.symbol.owner) ?: super.visitGetField(expression) -} +} \ No newline at end of file diff --git a/core/compiler.common/src/org/jetbrains/kotlin/incremental/components/InlineConstTracker.kt b/core/compiler.common/src/org/jetbrains/kotlin/incremental/components/InlineConstTracker.kt index 0766655fa98..9d8e2a87820 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/incremental/components/InlineConstTracker.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/incremental/components/InlineConstTracker.kt @@ -1,16 +1,26 @@ +/* + * Copyright 2010-2021 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.incremental.components -import org.jetbrains.kotlin.container.DefaultImplementation -import java.io.Serializable - -@DefaultImplementation(InlineConstTracker.DoNothing::class) +/** + * InlineConstTracker is used to track Java constants used in Kotlin for correct build scope expansion in IC during JPS build. + */ interface InlineConstTracker { - fun report(filePath: String, cRefs: Collection) + + /** + * Report Java constant, which is defined as [name] in [owner] java class. + * This constant is used in Kotlin file [filePath]. + * [constType] is one of Kotlin's [Byte, Short, Int, Long, Float, Double, Boolean, Char, String], + * that correspond to the eight primitive Java types or String + * Format of [owner] class is "package.Outer$Inner" + */ + fun report(filePath: String, owner: String, name: String, constType: String) object DoNothing : InlineConstTracker { - override fun report(filePath: String, cRefs: Collection) { + override fun report(filePath: String, owner: String, name: String, constType: String) { } } } - -data class ConstantRef(var owner: String, var name: String, var constType: String) : Serializable