IC: Clean up closing caches logic in IncrementalCompilerRunner.kt
Make the code more readable and address a small issue where we
did not close caches when returning `ICResult.RequiresRebuild`
(it was not exactly a correctness issue because we would recompile
non-incrementally in that case).
^KT-55709 In progress (This is a follow-up on a previous change for this
bug)
diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
index d1fa057..eafa2cb 100644
--- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
+++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
@@ -156,27 +156,13 @@
}
changedFiles as ChangedFiles.Known?
- // Because `caches` is a Closeable resource, it is important to close them in the event of an exception or before any return
- // statements.
- // Note: Historically, closing caches used to throw an exception sometimes, so currently we want to collect those exceptions. In the
- // future, if closing caches is safe, and we are no longer interested in those exceptions, we can simplify this code by using
- // Kotlin's `Closable.use` function (see `compileNonIncrementally`).
- fun closeCaches(caches: CacheManager, activeException: Throwable) {
- try {
- caches.close()
- } catch (e: Throwable) {
- activeException.addSuppressed(e)
- }
- }
-
val caches = createCacheManager(args, projectDir)
- val exitCode: ExitCode
- try {
+
+ fun compile(): ICResult {
// Step 1: Get changed files
val knownChangedFiles: ChangedFiles.Known = try {
getChangedFiles(changedFiles, allSourceFiles, caches)
} catch (e: Throwable) {
- closeCaches(caches, e)
return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e)
}
@@ -188,7 +174,6 @@
calculateSourcesToCompile(caches, knownChangedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
}
} catch (e: Throwable) {
- closeCaches(caches, e)
return ICResult.Failed(IC_FAILED_TO_COMPUTE_FILES_TO_RECOMPILE, e)
}
@@ -209,23 +194,48 @@
} else null
// Step 3: Compile incrementally
- exitCode = try {
+ val exitCode = try {
compileImpl(compilationMode as CompilationMode.Incremental, allSourceFiles, args, caches, abiSnapshotData, messageCollector)
} catch (e: Throwable) {
- closeCaches(caches, e)
return ICResult.Failed(IC_FAILED_TO_COMPILE_INCREMENTALLY, e)
}
+
+ return ICResult.Completed(exitCode)
+ }
+
+ fun closeCaches(caches: CacheManager, activeException: Throwable) {
+ try {
+ caches.close()
+ } catch (e: Throwable) {
+ activeException.addSuppressed(e)
+ }
+ }
+
+ // Because `caches` is a Closeable resource, it is important to close them in both cases:
+ // 1. in the event of an exception
+ // 2. after a normal execution
+ // Note: Historically, closing caches used to throw exceptions sometimes, so currently we want to collect those exceptions. In the
+ // future, if closing caches is safe, we can simplify the code by using Kotlin's `Closable.use` function (similar to the code in
+ // `compileNonIncrementally`).
+ val icResult = try {
+ compile()
} catch (e: Throwable) {
+ // Case 1 - Close the caches upon an exception
closeCaches(caches, e)
throw e
}
+
+ // Case 2 - Close the caches after a normal execution
try {
caches.close()
} catch (e: Throwable) {
- return ICResult.Failed(IC_FAILED_TO_CLOSE_CACHES, e)
+ return ICResult.Failed(
+ IC_FAILED_TO_CLOSE_CACHES,
+ RuntimeException("Failed to close caches, previous ICResult `$icResult` was discarded", e)
+ )
}
- return ICResult.Completed(exitCode)
+ return icResult
}
private fun compileNonIncrementally(
