From a7fef487c1a8bbfe777135141447a431d322ce78 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 16 Nov 2021 01:28:12 +0100 Subject: [PATCH] JVM IR: fix support of cyclic module dependencies Previous episode was at 987a3460 (KT-45915). Apparently, it's not enough to run psi2ir for all modules, and then backend for all modules. If there are several modules in the chunk, IR in any one of them can reference IR of any other one after psi2ir. So we would end up in a situation where we're running codegen for the first module, but the second module is completely unlowered. This would break some assumptions in the codegen, for example in KT-49575, codegen would see a reference to a top-level function from another module, and would fail because the function has no containing class (since file facades have not been generated yet!), and thus must be an intrinsic, yet no such intrinsic is known to the codegen. The solution is to run lowerings first on all modules, and then run codegen on all modules. The kind of error explained above shouldn't be possible anymore, because lowerings have to deal both with lowered and unlowered code from other files all the time, so lowerings can't assume that reference from other module is lowered either (or unlowered). The code is not very nice, but hopefully it can be improved as soon as we get rid of the old JVM backend (and maybe later with the migration to FIR too). #KT-49575 Fixed --- .../kotlin/codegen/CodegenFactory.kt | 27 ++++++++- .../FirKotlinToJvmBytecodeCompiler.kt | 12 ++-- .../compiler/KotlinToJVMBytecodeCompiler.kt | 59 +++++++++++++------ .../cli/jvm/compiler/cliCompilerUtils.kt | 6 +- .../kotlin/backend/jvm/JvmIrCodegenFactory.kt | 17 +++++- .../cli/jvm/modulesWithDependencyCycleA.kt | 8 ++- .../cli/jvm/modulesWithDependencyCycleB.kt | 7 ++- 7 files changed, 99 insertions(+), 37 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/CodegenFactory.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/CodegenFactory.kt index 5ee16b245a5..da4e7ca164f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/CodegenFactory.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/CodegenFactory.kt @@ -36,7 +36,14 @@ interface CodegenFactory { // modules combined, and then backend is run on each individual module. fun getModuleChunkBackendInput(wholeBackendInput: BackendInput, sourceFiles: Collection): BackendInput - fun generateModule(state: GenerationState, input: BackendInput) + fun invokeLowerings(state: GenerationState, input: BackendInput): CodegenInput + + fun invokeCodegen(input: CodegenInput) + + fun generateModule(state: GenerationState, input: BackendInput) { + val result = invokeLowerings(state, input) + invokeCodegen(result) + } class IrConversionInput( val project: Project, @@ -57,8 +64,14 @@ interface CodegenFactory { } } + // These opaque interfaces are needed to transfer the result of psi2ir to lowerings to codegen. + // Hopefully this can be refactored/simplified once the old JVM backend code is removed. interface BackendInput + interface CodegenInput { + val state: GenerationState + } + companion object { fun doCheckCancelled(state: GenerationState) { if (state.classBuilderMode.generateBodies) { @@ -69,7 +82,9 @@ interface CodegenFactory { } object DefaultCodegenFactory : CodegenFactory { - object DummyOldBackendInput : CodegenFactory.BackendInput + private object DummyOldBackendInput : CodegenFactory.BackendInput + + private class DummyOldCodegenInput(override val state: GenerationState) : CodegenFactory.CodegenInput override fun convertToIr(input: CodegenFactory.IrConversionInput): CodegenFactory.BackendInput = DummyOldBackendInput @@ -78,7 +93,7 @@ object DefaultCodegenFactory : CodegenFactory { sourceFiles: Collection, ): CodegenFactory.BackendInput = DummyOldBackendInput - override fun generateModule(state: GenerationState, input: CodegenFactory.BackendInput) { + override fun invokeLowerings(state: GenerationState, input: CodegenFactory.BackendInput): CodegenFactory.CodegenInput { val filesInPackages = MultiMap() val filesInMultifileClasses = MultiMap() @@ -103,6 +118,12 @@ object DefaultCodegenFactory : CodegenFactory { CodegenFactory.doCheckCancelled(state) generatePackage(state, packageFqName, filesInPackages.get(packageFqName)) } + + return DummyOldCodegenInput(state) + } + + override fun invokeCodegen(input: CodegenFactory.CodegenInput) { + // Do nothing } private fun generateMultifileClass(state: GenerationState, multifileClassFqName: FqName, files: Collection) { diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt index 0fac7a16cb7..49c15d4aa1c 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt @@ -62,9 +62,7 @@ import org.jetbrains.kotlin.resolve.PlatformDependentAnalyzerServices import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatformAnalyzerServices import org.jetbrains.kotlin.resolve.multiplatform.isCommonSource import org.jetbrains.kotlin.utils.addToStdlib.runIf -import org.jetbrains.kotlin.utils.newLinkedHashMapWithExpectedSize import java.io.File -import kotlin.collections.set object FirKotlinToJvmBytecodeCompiler { fun compileModulesUsingFrontendIR( @@ -83,11 +81,12 @@ object FirKotlinToJvmBytecodeCompiler { "ATTENTION!\n This build uses in-dev FIR: \n -Xuse-fir" ) - val outputs = newLinkedHashMapWithExpectedSize>(chunk.size) + val outputs = ArrayList>(chunk.size) val targetIds = projectConfiguration.get(JVMConfigurationKeys.MODULES)?.map(::TargetId) val incrementalComponents = projectConfiguration.get(JVMConfigurationKeys.INCREMENTAL_COMPILATION_COMPONENTS) val isMultiModuleChunk = chunk.size > 1 + // TODO: run lowerings for all modules in the chunk, then run codegen for all modules. for (module in chunk) { val moduleConfiguration = projectConfiguration.applyModuleProperties(module, buildFile) val context = CompilationContext( @@ -105,19 +104,18 @@ object FirKotlinToJvmBytecodeCompiler { (projectEnvironment as? PsiBasedProjectEnvironment)?.project?.let { IrGenerationExtension.getInstances(it) } ?: emptyList() ) val generationState = context.compileModule() ?: return false - outputs[module] = generationState + outputs += generationState } val mainClassFqName: FqName? = runIf(chunk.size == 1 && projectConfiguration.get(JVMConfigurationKeys.OUTPUT_JAR) != null) { - val firResult = outputs.values.single().first - findMainClass(firResult) + findMainClass(outputs.single().first) } return writeOutputs( (projectEnvironment as? PsiBasedProjectEnvironment)?.project, projectConfiguration, chunk, - outputs.mapValues { (_, value) -> value.second }, + outputs.map(Pair::second), mainClassFqName ) } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt index 87a94497059..73eb18cf2e3 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinToJVMBytecodeCompiler.kt @@ -42,15 +42,15 @@ import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.config.JVMConfigurationKeys import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.diagnostics.DiagnosticReporterFactory +import org.jetbrains.kotlin.diagnostics.impl.BaseDiagnosticsCollector import org.jetbrains.kotlin.ir.backend.jvm.jvmResolveLibraries import org.jetbrains.kotlin.load.kotlin.ModuleVisibilityManager import org.jetbrains.kotlin.modules.Module import org.jetbrains.kotlin.progress.ProgressIndicatorAndCompilationCanceledStatus import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.jvm.KotlinJavaPsiFacade -import org.jetbrains.kotlin.utils.newLinkedHashMapWithExpectedSize import java.io.File -import kotlin.collections.set object KotlinToJVMBytecodeCompiler { internal fun compileModules( @@ -111,11 +111,12 @@ object KotlinToJVMBytecodeCompiler { findMainClass(result.bindingContext, projectConfiguration.languageVersionSettings, environment.getSourceFiles()) else null - val outputs = newLinkedHashMapWithExpectedSize(chunk.size) - val localFileSystem = VirtualFileManager.getInstance().getFileSystem(StandardFileSystems.FILE_PROTOCOL) val (codegenFactory, wholeBackendInput) = convertToIr(environment, result) + val diagnosticsReporter = DiagnosticReporterFactory.createReporter() + + val codegenInputs = ArrayList(chunk.size) for (module in chunk) { ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() @@ -125,13 +126,20 @@ object KotlinToJVMBytecodeCompiler { val moduleConfiguration = projectConfiguration.applyModuleProperties(module, buildFile) val backendInput = codegenFactory.getModuleChunkBackendInput(wholeBackendInput, ktFiles) - outputs[module] = generate(environment, moduleConfiguration, result, ktFiles, module, codegenFactory, backendInput) + codegenInputs += runLowerings( + environment, moduleConfiguration, result, ktFiles, module, codegenFactory, backendInput, diagnosticsReporter + ) + } + + val outputs = ArrayList(chunk.size) + + for (input in codegenInputs) { + outputs += runCodegen(input, input.state, result.bindingContext, diagnosticsReporter, environment.configuration) } return writeOutputs(environment.project, projectConfiguration, chunk, outputs, mainClassFqName) } - internal fun configureSourceRoots(configuration: CompilerConfiguration, chunk: List, buildFile: File? = null) { for (module in chunk) { val commonSources = getBuildFilePaths(buildFile, module.getCommonSourceFiles()).toSet() @@ -240,7 +248,12 @@ object KotlinToJVMBytecodeCompiler { result.throwIfError() val (codegenFactory, backendInput) = convertToIr(environment, result) - return generate(environment, environment.configuration, result, environment.getSourceFiles(), null, codegenFactory, backendInput) + val diagnosticsReporter = DiagnosticReporterFactory.createReporter() + val input = runLowerings( + environment, environment.configuration, result, environment.getSourceFiles(), null, codegenFactory, backendInput, + diagnosticsReporter + ) + return runCodegen(input, input.state, result.bindingContext, diagnosticsReporter, environment.configuration) } private fun convertToIr(environment: KotlinCoreEnvironment, result: AnalysisResult): Pair { @@ -325,7 +338,7 @@ object KotlinToJVMBytecodeCompiler { override fun toString() = "All files under: $directories" } - private fun generate( + private fun runLowerings( environment: KotlinCoreEnvironment, configuration: CompilerConfiguration, result: AnalysisResult, @@ -333,9 +346,8 @@ object KotlinToJVMBytecodeCompiler { module: Module?, codegenFactory: CodegenFactory, backendInput: CodegenFactory.BackendInput, - ): GenerationState { - val diagnosticsReporter = DiagnosticReporterFactory.createReporter() - + diagnosticsReporter: BaseDiagnosticsCollector, + ): CodegenFactory.CodegenInput { val state = GenerationState.Builder( environment.project, ClassBuilderFactories.BINARIES, @@ -352,30 +364,41 @@ object KotlinToJVMBytecodeCompiler { ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() - val performanceManager = environment.configuration.get(CLIConfigurationKeys.PERF_MANAGER) - performanceManager?.notifyGenerationStarted() + environment.configuration.get(CLIConfigurationKeys.PERF_MANAGER)?.notifyGenerationStarted() state.beforeCompile() ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() - codegenFactory.generateModule(state, backendInput) + return codegenFactory.invokeLowerings(state, backendInput) + } + + private fun runCodegen( + codegenInput: CodegenFactory.CodegenInput, + state: GenerationState, + bindingContext: BindingContext, + diagnosticsReporter: BaseDiagnosticsCollector, + configuration: CompilerConfiguration, + ): GenerationState { + ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() + state.codegenFactory.invokeCodegen(codegenInput) CodegenFactory.doCheckCancelled(state) state.factory.done() - performanceManager?.notifyGenerationFinished() + configuration.get(CLIConfigurationKeys.PERF_MANAGER)?.notifyGenerationFinished() ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() + val messageCollector = configuration.getNotNull(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY) AnalyzerWithCompilerReport.reportDiagnostics( FilteredJvmDiagnostics( state.collectedExtraJvmDiagnostics, - result.bindingContext.diagnostics + bindingContext.diagnostics ), - environment.messageCollector + messageCollector ) - FirDiagnosticsCompilerResultsReporter.reportToMessageCollector(diagnosticsReporter, environment.messageCollector) + FirDiagnosticsCompilerResultsReporter.reportToMessageCollector(diagnosticsReporter, messageCollector) ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() return state diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/cliCompilerUtils.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/cliCompilerUtils.kt index 187247f5b6c..1940abd7d91 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/cliCompilerUtils.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/cliCompilerUtils.kt @@ -119,16 +119,16 @@ fun writeOutputs( project: Project?, projectConfiguration: CompilerConfiguration, chunk: List, - outputs: Map, + outputs: List, mainClassFqName: FqName? ): Boolean { try { - for ((_, state) in outputs) { + for (state in outputs) { ProgressIndicatorAndCompilationCanceledStatus.checkCanceled() writeOutput(state.configuration, state.factory, mainClassFqName) } } finally { - outputs.values.forEach(GenerationState::destroy) + outputs.forEach(GenerationState::destroy) } if (projectConfiguration.getBoolean(JVMConfigurationKeys.COMPILE_JAVA)) { diff --git a/compiler/ir/backend.jvm/entrypoint/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt b/compiler/ir/backend.jvm/entrypoint/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt index 005e680fb5b..c8e771453ba 100644 --- a/compiler/ir/backend.jvm/entrypoint/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt +++ b/compiler/ir/backend.jvm/entrypoint/src/org/jetbrains/kotlin/backend/jvm/JvmIrCodegenFactory.kt @@ -63,6 +63,13 @@ open class JvmIrCodegenFactory( val notifyCodegenStart: () -> Unit ) : CodegenFactory.BackendInput + private data class JvmIrCodegenInput( + override val state: GenerationState, + val context: JvmBackendContext, + val module: IrModuleFragment, + val notifyCodegenStart: () -> Unit, + ) : CodegenFactory.CodegenInput + override fun convertToIr(input: CodegenFactory.IrConversionInput): JvmIrBackendInput { val (mangler, symbolTable) = if (externalSymbolTable != null) externalMangler!! to externalSymbolTable @@ -213,7 +220,7 @@ open class JvmIrCodegenFactory( ) } - override fun generateModule(state: GenerationState, input: CodegenFactory.BackendInput) { + override fun invokeLowerings(state: GenerationState, input: CodegenFactory.BackendInput): CodegenFactory.CodegenInput { val (irModuleFragment, symbolTable, customPhaseConfig, irProviders, extensions, backendExtension, notifyCodegenStart) = input as JvmIrBackendInput val irSerializer = if ( @@ -233,9 +240,13 @@ open class JvmIrCodegenFactory( phases.invokeToplevel(phaseConfig, context, irModuleFragment) - notifyCodegenStart() + return JvmIrCodegenInput(state, context, irModuleFragment, notifyCodegenStart) + } - jvmCodegenPhases.invokeToplevel(phaseConfig, context, irModuleFragment) + override fun invokeCodegen(input: CodegenFactory.CodegenInput) { + val (state, context, module, notifyCodegenStart) = input as JvmIrCodegenInput + notifyCodegenStart() + jvmCodegenPhases.invokeToplevel(PhaseConfig(jvmCodegenPhases), context, module) // TODO: split classes into groups connected by inline calls; call this after every group // and clear `JvmBackendContext.classCodegens` diff --git a/compiler/testData/cli/jvm/modulesWithDependencyCycleA.kt b/compiler/testData/cli/jvm/modulesWithDependencyCycleA.kt index 0d934645509..07327a027d3 100644 --- a/compiler/testData/cli/jvm/modulesWithDependencyCycleA.kt +++ b/compiler/testData/cli/jvm/modulesWithDependencyCycleA.kt @@ -1,8 +1,14 @@ package a -import b.Y +import b.* class A class X(val y: Y? = null) class Z : Y() + +fun topLevelA1() { + topLevelB() +} + +fun topLevelA2() {} diff --git a/compiler/testData/cli/jvm/modulesWithDependencyCycleB.kt b/compiler/testData/cli/jvm/modulesWithDependencyCycleB.kt index ad5dc0729ed..e3ba4f3f4b9 100644 --- a/compiler/testData/cli/jvm/modulesWithDependencyCycleB.kt +++ b/compiler/testData/cli/jvm/modulesWithDependencyCycleB.kt @@ -1,8 +1,11 @@ package b -import a.A -import a.X +import a.* class B(val a: A? = null) open class Y(val x: X? = null) + +fun topLevelB() { + topLevelA2() +}