From e92017f64ec26f031824254fa82e983f292c9b81 Mon Sep 17 00:00:00 2001 From: Dmitriy Dolovov Date: Wed, 29 Nov 2023 22:07:14 +0100 Subject: [PATCH] [KLIB Resolver] Deprecate Logger.fatal() Invocation of Logger.fatal() may cause severe side effects such as throwing an exception or even terminating the current JVM process (check various implementations of this function for details). The code that uses Logger.fatal() sometimes expects a particular kind of side effect. This is totally a design flaw. And it's definitely not a responsibility of Logger to influence the execution flow of the program. --- ...rStandaloneLibrarySymbolProviderFactory.kt | 4 +++- .../common/messages/CompilerLoggerAdapter.kt | 1 + .../backend/common/IrMessageLoggerAdapter.kt | 1 + .../org/jetbrains/kotlin/util/WithLogger.kt | 21 +++++++++++++++---- .../kotlin/library/SearchPathResolver.kt | 13 +++++++++++- .../kotlin/library/impl/KotlinLibraryImpl.kt | 2 ++ .../org/jetbrains/kotlin/cli/klib/main.kt | 5 ++++- .../kotlin/gradle/utils/cacheKlibUtils.kt | 9 +++++--- .../kotlin/commonizer/NativeLibraryLoader.kt | 5 +++-- .../kotlin/commonizer/cli/CliLoggerAdapter.kt | 10 +++++++-- .../commonizer/konan/LibraryCommonizer.kt | 5 +++-- 11 files changed, 60 insertions(+), 16 deletions(-) diff --git a/analysis/analysis-api-standalone/analysis-api-fir-standalone-base/src/org/jetbrains/kotlin/analysis/api/standalone/base/project/structure/LLFirStandaloneLibrarySymbolProviderFactory.kt b/analysis/analysis-api-standalone/analysis-api-fir-standalone-base/src/org/jetbrains/kotlin/analysis/api/standalone/base/project/structure/LLFirStandaloneLibrarySymbolProviderFactory.kt index 69caa99e22d..e8d41723b01 100644 --- a/analysis/analysis-api-standalone/analysis-api-fir-standalone-base/src/org/jetbrains/kotlin/analysis/api/standalone/base/project/structure/LLFirStandaloneLibrarySymbolProviderFactory.kt +++ b/analysis/analysis-api-standalone/analysis-api-fir-standalone-base/src/org/jetbrains/kotlin/analysis/api/standalone/base/project/structure/LLFirStandaloneLibrarySymbolProviderFactory.kt @@ -34,6 +34,7 @@ import java.nio.file.Path import kotlin.io.path.absolutePathString import kotlin.io.path.extension import kotlin.io.path.isDirectory +import org.jetbrains.kotlin.util.Logger as KLogger class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project) : LLFirLibrarySymbolProviderFactory() { override fun createJvmLibrarySymbolProvider( @@ -154,7 +155,7 @@ class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project) private val LOG = Logger.getInstance(LLFirStandaloneLibrarySymbolProviderFactory::class.java) } - private object IntellijLogBasedLogger : org.jetbrains.kotlin.util.Logger { + private object IntellijLogBasedLogger : KLogger { override fun log(message: String) { LOG.info(message) } @@ -167,6 +168,7 @@ class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project) LOG.warn(message) } + @Deprecated(KLogger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(KLogger.FATAL_REPLACEMENT)) override fun fatal(message: String): Nothing { throw IllegalStateException(message) } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/CompilerLoggerAdapter.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/CompilerLoggerAdapter.kt index 7a6738fb012..a3eef0c9005 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/CompilerLoggerAdapter.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/CompilerLoggerAdapter.kt @@ -25,6 +25,7 @@ private class CompilerLoggerAdapter( override fun warning(message: String) = messageCollector.report(if (treatWarningsAsErrors) ERROR else STRONG_WARNING, message, null) override fun error(message: String) = messageCollector.report(ERROR, message, null) + @Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT)) override fun fatal(message: String): Nothing { error(message) (messageCollector as? GroupingMessageCollector)?.flush() diff --git a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/IrMessageLoggerAdapter.kt b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/IrMessageLoggerAdapter.kt index 4df53f033a5..eacf38e637b 100644 --- a/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/IrMessageLoggerAdapter.kt +++ b/compiler/ir/serialization.common/src/org/jetbrains/kotlin/backend/common/IrMessageLoggerAdapter.kt @@ -19,6 +19,7 @@ private class IrMessageLoggerAdapter(private val irMessageLogger: IrMessageLogge override fun warning(message: String) = irMessageLogger.report(WARNING, message, null) override fun error(message: String) = irMessageLogger.report(ERROR, message, null) + @Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT)) override fun fatal(message: String): Nothing { error(message) throw CompilationErrorException() diff --git a/compiler/util-io/src/org/jetbrains/kotlin/util/WithLogger.kt b/compiler/util-io/src/org/jetbrains/kotlin/util/WithLogger.kt index 683c33dbe09..6d43167cd74 100644 --- a/compiler/util-io/src/org/jetbrains/kotlin/util/WithLogger.kt +++ b/compiler/util-io/src/org/jetbrains/kotlin/util/WithLogger.kt @@ -4,9 +4,20 @@ import kotlin.system.exitProcess interface Logger { fun log(message: String) - fun error(message: String) fun warning(message: String) + fun error(message: String) + + @Deprecated(FATAL_DEPRECATION_MESSAGE, ReplaceWith(FATAL_REPLACEMENT)) fun fatal(message: String): Nothing + + companion object { + const val FATAL_DEPRECATION_MESSAGE = "Invocation of fatal() may cause severe side effects such as throwing an exception or " + + "even terminating the current JVM process (check various implementations of this function for details). " + + "The code that uses Logger.fatal() sometimes expects a particular kind of side effect. " + + "This is an undesirable design. And it's definitely not a responsibility of Logger to influence " + + "the execution flow of the program." + const val FATAL_REPLACEMENT = "error(message)" + } } interface WithLogger { @@ -27,10 +38,12 @@ interface WithLogger { */ object DummyLogger : Logger { override fun log(message: String) = println(message) - override fun error(message: String) = println("e: $message") override fun warning(message: String) = println("w: $message") + override fun error(message: String) = println("e: $message") + + @Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT)) override fun fatal(message: String): Nothing { - println("e: $message") - exitProcess(1) + error(message) + exitProcess(1) // WARNING: This would stop the JVM process! } } diff --git a/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt b/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt index 85620d0ca5c..0911b75e37c 100644 --- a/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt +++ b/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt @@ -268,7 +268,18 @@ abstract class KotlinLibrarySearchPathResolver( override fun resolve(unresolved: RequiredUnresolvedLibrary, isDefaultLink: Boolean): L { return resolveOrNull(unresolved, isDefaultLink) - ?: logger.fatal("KLIB resolver: Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}") + ?: run { + // It does not make sense to replace logger.fatal() by logger.error() here, because: + // 1. We don't know which exactly side effect (throwing an exception, exiting JVM process, etc) is performed + // by logger.fatal() as we don't know the concrete Logger class. So, we can't use logger.error() + // and emulate the proper side effect here. + // 2. The contract of resolve(RequiredUnresolvedLibrary, Boolean) has a design flaw: It assumes that either + // the library is resolved or the side effect takes place. The latter (side effect) affects the execution + // flow of the program and should not be a responsibility of SearchPathResolver. + // 3. Finally, we are going to drop SearchPathResolver which is a part of KLIB resolver. + @Suppress("DEPRECATION") + logger.fatal("KLIB resolver: Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}") + } } override fun libraryMatch(candidate: L, unresolved: UnresolvedLibrary): Boolean = true diff --git a/compiler/util-klib/src/org/jetbrains/kotlin/library/impl/KotlinLibraryImpl.kt b/compiler/util-klib/src/org/jetbrains/kotlin/library/impl/KotlinLibraryImpl.kt index c7aec4f5b2f..d5756c070e2 100644 --- a/compiler/util-klib/src/org/jetbrains/kotlin/library/impl/KotlinLibraryImpl.kt +++ b/compiler/util-klib/src/org/jetbrains/kotlin/library/impl/KotlinLibraryImpl.kt @@ -371,6 +371,8 @@ fun isKotlinLibrary(libraryFile: File): Boolean = try { override fun log(message: String) = Unit // don't log override fun error(message: String) = Unit // don't log override fun warning(message: String) = Unit // don't log + + @Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT)) override fun fatal(message: String): Nothing = kotlin.error("This function should not be called") }, knownIrProviders = emptyList() diff --git a/kotlin-native/klib/src/main/kotlin/org/jetbrains/kotlin/cli/klib/main.kt b/kotlin-native/klib/src/main/kotlin/org/jetbrains/kotlin/cli/klib/main.kt index 891cea0079f..f053b093776 100644 --- a/kotlin-native/klib/src/main/kotlin/org/jetbrains/kotlin/cli/klib/main.kt +++ b/kotlin-native/klib/src/main/kotlin/org/jetbrains/kotlin/cli/klib/main.kt @@ -155,10 +155,13 @@ internal fun logError(text: String, withStacktrace: Boolean = false): Nothing { } object KlibToolLogger : Logger, IrMessageLogger { + override fun log(message: String) = println(message) override fun warning(message: String) = logWarning(message) override fun error(message: String) = logWarning(message) + + @Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT)) override fun fatal(message: String) = logError(message, withStacktrace = true) - override fun log(message: String) = println(message) + override fun report(severity: IrMessageLogger.Severity, message: String, location: IrMessageLogger.Location?) { when (severity) { IrMessageLogger.Severity.INFO -> log(message) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/cacheKlibUtils.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/cacheKlibUtils.kt index 8cc0462ca15..cd23d3e9416 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/cacheKlibUtils.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/cacheKlibUtils.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.library.uniqueName import java.io.File import java.nio.charset.StandardCharsets import java.security.MessageDigest +import org.jetbrains.kotlin.util.Logger as KLogger internal fun getCacheDirectory( rootCacheDirectory: File, @@ -112,11 +113,13 @@ internal fun getAllDependencies(dependency: ResolvedDependencyResult): Set when (outputTarget.allLeaves().size) { - 0 -> logger.fatal("No targets specified") - 1 -> logger.fatal("Too few targets specified: $outputTarget") + 0 -> logger.errorAndExitJvmProcess("No targets specified") + 1 -> logger.errorAndExitJvmProcess("Too few targets specified: $outputTarget") } } }