PR-1021 review: minor fixes

# Conflicts: # compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/repl/GenericReplCompilingEvaluator.kt # compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/repl/GenericReplEvaluator.kt # compiler/cli/src/org/jetbrains/kotlin/cli/jvm/repl/GenericReplCompiler.kt
2017-01-27 14:03:39 +01:00
parent 5ad06e1e92
commit 0b689a4ecb
10 changed files with 48 additions and 60 deletions
@@ -16,7 +16,6 @@

 package org.jetbrains.kotlin.cli.common.repl

-
 import java.io.File
 import java.util.concurrent.locks.ReentrantReadWriteLock
 import kotlin.concurrent.write
@@ -26,19 +25,18 @@ class GenericReplCompilingEvaluator(val compiler: ReplCompiler,
                                    baseClassloader: ClassLoader?,
                                    protected val fallbackScriptArgs: ScriptArgsWithTypes? = null,
                                    repeatingMode: ReplRepeatingMode = ReplRepeatingMode.REPEAT_ONLY_MOST_RECENT,
-                                    protected val stateLock: ReentrantReadWriteLock = ReentrantReadWriteLock()) : ReplFullEvaluator {
+                                    protected val stateLock: ReentrantReadWriteLock = ReentrantReadWriteLock()
+) : ReplFullEvaluator {
    val evaluator = GenericReplEvaluator(baseClasspath, baseClassloader, fallbackScriptArgs, repeatingMode, stateLock)

    override fun compileAndEval(codeLine: ReplCodeLine, scriptArgs: ScriptArgsWithTypes?, verifyHistory: List<ReplCodeLine>?, invokeWrapper: InvokeWrapper?): ReplEvalResult {
        return stateLock.write {
-            @Suppress("DEPRECATION")
            val compiled = compiler.compile(codeLine, verifyHistory)
            when (compiled) {
                is ReplCompileResult.Error -> ReplEvalResult.Error.CompileTime(compiled.compiledHistory, compiled.message, compiled.location)
                is ReplCompileResult.HistoryMismatch -> ReplEvalResult.HistoryMismatch(compiled.compiledHistory, compiled.lineNo)
                is ReplCompileResult.Incomplete -> ReplEvalResult.Incomplete(compiled.compiledHistory)
                is ReplCompileResult.CompiledClasses -> {
-                    @Suppress("DEPRECATION")
                    val result = eval(compiled, scriptArgs, invokeWrapper)
                    when (result) {
                        is ReplEvalResult.Error,
@@ -50,10 +48,10 @@ class GenericReplCompilingEvaluator(val compiler: ReplCompiler,
                        is ReplEvalResult.ValueResult,
                        is ReplEvalResult.UnitResult ->
                            result
-                        else -> throw IllegalStateException("Unknown evaluator result type ${compiled}")
+                        else -> throw IllegalStateException("Unknown evaluator result type $compiled")
                    }
                }
-                else -> throw IllegalStateException("Unknown compiler result type ${compiled}")
+                else -> throw IllegalStateException("Unknown compiler result type $compiled")
            }
        }
    }
@@ -65,7 +63,7 @@ class GenericReplCompilingEvaluator(val compiler: ReplCompiler,

            removedCompiledLines.zip(removedEvaluatorLines).forEach {
                if (it.first != it.second) {
-                    throw IllegalStateException("History mistmatch when resetting lines")
+                    throw IllegalStateException("History mismatch when resetting lines")
                }
            }

@@ -75,7 +73,6 @@ class GenericReplCompilingEvaluator(val compiler: ReplCompiler,

    override fun resetToLine(line: ReplCodeLine): List<ReplCodeLine> = resetToLine(line.no)

-
    override val lastEvaluatedScripts: List<EvalHistoryType> get() = evaluator.lastEvaluatedScripts
    override val history: List<ReplCodeLine> get() = evaluator.history
    override val currentClasspath: List<File> get() = evaluator.currentClasspath
@@ -83,21 +80,16 @@ class GenericReplCompilingEvaluator(val compiler: ReplCompiler,
    override val compiledHistory: List<ReplCodeLine> get() = compiler.history
    override val evaluatedHistory: List<ReplCodeLine> get() = evaluator.history

-    override fun eval(compileResult: ReplCompileResult.CompiledClasses, scriptArgs: ScriptArgsWithTypes?, invokeWrapper: InvokeWrapper?): ReplEvalResult {
-        return evaluator.eval(compileResult, scriptArgs, invokeWrapper)
-    }
+    override fun eval(compileResult: ReplCompileResult.CompiledClasses, scriptArgs: ScriptArgsWithTypes?, invokeWrapper: InvokeWrapper?): ReplEvalResult =
+            evaluator.eval(compileResult, scriptArgs, invokeWrapper)

-    override fun check(codeLine: ReplCodeLine): ReplCheckResult {
-        return compiler.check(codeLine)
-    }
+    override fun check(codeLine: ReplCodeLine): ReplCheckResult = compiler.check(codeLine)

    override fun compileToEvaluable(codeLine: ReplCodeLine, defaultScriptArgs: ScriptArgsWithTypes?, verifyHistory: List<ReplCodeLine>?): Pair<ReplCompileResult, Evaluable?> {
        val compiled = compiler.compile(codeLine, verifyHistory)
-        return if (compiled is ReplCompileResult.CompiledClasses) {
-            Pair(compiled, DelayedEvaluation(compiled, stateLock, evaluator, defaultScriptArgs ?: fallbackScriptArgs))
-        }
-        else {
-            Pair(compiled, null)
+        return when (compiled) {
+            is ReplCompileResult.CompiledClasses -> Pair(compiled, DelayedEvaluation(compiled, stateLock, evaluator, defaultScriptArgs ?: fallbackScriptArgs))
+            else -> Pair(compiled, null)
        }
    }

@@ -27,7 +27,8 @@ open class GenericReplEvaluator(baseClasspath: Iterable<File>,
                                baseClassloader: ClassLoader?,
                                protected val fallbackScriptArgs: ScriptArgsWithTypes? = null,
                                protected val repeatingMode: ReplRepeatingMode = ReplRepeatingMode.REPEAT_ONLY_MOST_RECENT,
-                                protected val stateLock: ReentrantReadWriteLock = ReentrantReadWriteLock()) : ReplEvaluator {
+                                protected val stateLock: ReentrantReadWriteLock = ReentrantReadWriteLock()
+) : ReplEvaluator {

    private val topClassLoader: ReplClassLoader = makeReplClassLoader(baseClassloader, baseClasspath)

@@ -42,7 +43,7 @@ open class GenericReplEvaluator(baseClasspath: Iterable<File>,
    override val history: List<ReplCodeLine> get() = stateLock.read { evaluatedHistory.copySources() }

    override val currentClasspath: List<File> get() = stateLock.read {
-        evaluatedHistory.copyValues().lastOrNull()?.let { it.classLoader.listAllUrlsAsFiles() }
+        evaluatedHistory.copyValues().lastOrNull()?.classLoader?.listAllUrlsAsFiles()
        ?: topClassLoader.listAllUrlsAsFiles()
    }

@@ -57,8 +58,8 @@ open class GenericReplEvaluator(baseClasspath: Iterable<File>,
        return stateLock.write {
            var mainLineClassName: String? = null
            val classLoader = makeReplClassLoader(effectiveHistory.lastOrNull()?.classLoader ?: topClassLoader, compileResult.classpathAddendum)
-            fun classNameFromPath(path: String) = JvmClassName.byInternalName(path.replaceFirst("\\.class$".toRegex(), ""))
-            fun compiledClassesNames() = compileResult.classes.map { classNameFromPath(it.path).fqNameForClassNameWithoutDollars.asString() }
+            fun classNameFromPath(path: String) = JvmClassName.byInternalName(path.removeSuffix(".class"))
+            fun compiledClassesNames() = compileResult.classes.map { classNameFromPath(it.path).internalName.replace('/', '.') }
            val expectedClassName = compileResult.generatedClassname
            compileResult.classes.filter { it.path.endsWith(".class") }
                    .forEach {
@@ -154,8 +155,7 @@ open class GenericReplEvaluator(baseClasspath: Iterable<File>,
                historyActor.processClasses(compileResult)
            }
            catch (e: Exception) {
-                return@eval ReplEvalResult.Error.Runtime(evaluatedHistory.copySources(),
-                                                         e.message!!, e)
+                return@eval ReplEvalResult.Error.Runtime(evaluatedHistory.copySources(), e.message ?: "unknown", e)
            }

            val currentScriptArgs = scriptArgs ?: fallbackScriptArgs
@@ -167,6 +167,7 @@ open class GenericReplEvaluator(baseClasspath: Iterable<File>,
                                                     ).toTypedArray()
            val constructorArgs: Array<Any?> = (historyActor.effectiveHistory.map { it.instance } + useScriptArgs.orEmpty()).toTypedArray()

+            // TODO: try/catch ?
            val scriptInstanceConstructor = scriptClass.getConstructor(*constructorParams)

            historyActor.addPlaceholder(compileResult.compiledCodeLine, EvalClassWithInstanceAndLoader(scriptClass.kotlin, null, classLoader, invokeWrapper))
@@ -32,7 +32,7 @@ interface KotlinJsr223JvmInvocableScriptEngine : Invocable {
    val replScriptEvaluator: ReplEvaluatorExposedInternalHistory

    private fun prioritizedHistory(receiverClass: KClass<*>?, receiverInstance: Any?): List<EvalClassWithInstanceAndLoader> {
-        return replScriptEvaluator.lastEvaluatedScripts.map { it.second }.filter { it.instance != null }.reversed().assertNotEmpty("no script ").let { history ->
+        return replScriptEvaluator.lastEvaluatedScripts.map { it.second }.filter { it.instance != null }.reversed().ensureNotEmpty("no script ").let { history ->
            if (receiverInstance != null) {
                val receiverKlass = receiverClass ?: receiverInstance.javaClass.kotlin
                val receiverInHistory = history.find { it.instance == receiverInstance } ?:
@@ -94,7 +94,7 @@ interface KotlinJsr223JvmInvocableScriptEngine : Invocable {
    }

    private fun <T : Any> proxyInterface(thiz: Any?, clasz: Class<T>?): T? {
-        replScriptEvaluator.lastEvaluatedScripts.assertNotEmpty("no script")
+        replScriptEvaluator.lastEvaluatedScripts.ensureNotEmpty("no script")
        val priority = prioritizedHistory(thiz?.javaClass?.kotlin, thiz)

        if (clasz == null) throw IllegalArgumentException("class object cannot be null")
@@ -25,10 +25,11 @@ val KOTLIN_SCRIPT_HISTORY_BINDINGS_KEY = "kotlin.script.history"


 // TODO consider additional error handling
-@Suppress("UNCHECKED_CAST")
 val Bindings.kotlinScriptHistory: MutableList<ReplCodeLine>
+    @Suppress("UNCHECKED_CAST")
    get() = getOrPut(KOTLIN_SCRIPT_HISTORY_BINDINGS_KEY, { arrayListOf<ReplCodeLine>() }) as MutableList<ReplCodeLine>

+
 abstract class KotlinJsr223JvmScriptEngineBase(protected val myFactory: ScriptEngineFactory) : AbstractScriptEngine(), ScriptEngine, Compilable {

    protected var codeLineNumber = AtomicInteger(0)
@@ -17,23 +17,22 @@
 package org.jetbrains.kotlin.cli.common.repl

 import java.io.Serializable
-import java.util.concurrent.ConcurrentLinkedDeque
+import java.util.*


 typealias CompiledHistoryItem<T> = Pair<CompiledReplCodeLine, T>
 typealias SourceHistoryItem<T> = Pair<ReplCodeLine, T>

-typealias CompiledHistoryStorage<T> = ConcurrentLinkedDeque<CompiledHistoryItem<T>>
+typealias CompiledHistoryStorage<T> = ArrayDeque<CompiledHistoryItem<T>>
 typealias CompiledHistoryList<T> = List<CompiledHistoryItem<T>>
 typealias SourceHistoryList<T> = List<SourceHistoryItem<T>>
 typealias SourceList = List<ReplCodeLine>

 /*
-   Not thread safe, the caller is assumed to lock access.  Even though ConcurrentLinkedDeque is used, there are mutliple
-   actions that require locking.
+   WARNING: Not thread safe, the caller is assumed to lock access.
 */
 class ReplHistory<T>(startingHistory: CompiledHistoryList<T> = emptyList()) : Serializable {
-    private val history: CompiledHistoryStorage<T> = ConcurrentLinkedDeque(startingHistory)
+    private val history: CompiledHistoryStorage<T> = ArrayDeque(startingHistory)

    fun isEmpty(): Boolean = history.isEmpty()
    fun isNotEmpty(): Boolean = history.isNotEmpty()
@@ -62,9 +61,7 @@ class ReplHistory<T>(startingHistory: CompiledHistoryList<T> = emptyList()) : Se
        return removed.reversed()
    }

-    fun resetToLine(line: ReplCodeLine): SourceHistoryList<T> {
-        return resetToLine(line.no)
-    }
+    fun resetToLine(line: ReplCodeLine): SourceHistoryList<T> = resetToLine(line.no)

    fun resetToLine(line: CompiledReplCodeLine): CompiledHistoryList<T> {
        val removed = arrayListOf<CompiledHistoryItem<T>>()
@@ -86,13 +83,11 @@ class ReplHistory<T>(startingHistory: CompiledHistoryList<T> = emptyList()) : Se
    }

    // return from the compareHistory the first line that does not match or null
-    fun firstMismatchingHistory(compareHistory: SourceList?): Int? {
-        if (compareHistory == null) return null
-
-        val firstMismatch = history.zip(compareHistory).firstOrNull { it.first.first.source != it.second }?.second?.no
-        if (compareHistory.size == history.size) return firstMismatch
-        if (compareHistory.size > history.size) return compareHistory[history.size].no
-        return history.toList()[compareHistory.size].first.source.no
+    fun firstMismatchingHistory(compareHistory: SourceList?): Int? = when {
+        compareHistory == null -> null
+        compareHistory.size == history.size -> history.zip(compareHistory).firstOrNull { it.first.first.source != it.second }?.second?.no
+        compareHistory.size > history.size -> compareHistory[history.size].no
+        else -> history.toList()[compareHistory.size].first.source.no
    }

    fun copySources(): SourceList = history.map { it.first.source }
@@ -20,8 +20,8 @@ import com.google.common.base.Throwables
 import java.io.File
 import java.net.URLClassLoader

-fun makeSriptBaseName(codeLine: ReplCodeLine, generation: Long) =
-        "Line_${codeLine.no}" + if (generation > 1) "_gen_${generation}" else ""
+fun makeScriptBaseName(codeLine: ReplCodeLine, generation: Long) =
+        "Line_${codeLine.no}${if (generation > 1) "_gen_$generation" else ""}"

 fun renderReplStackTrace(cause: Throwable, startFromMethodName: String): String {
    val newTrace = arrayListOf<StackTraceElement>()
@@ -55,10 +55,10 @@ internal fun ClassLoader.listAllUrlsAsFiles(): List<File> {
 }

 internal fun URLClassLoader.listLocalUrlsAsFiles(): List<File> {
-    return this.urLs.map { it.toString().removePrefix("file:") }.filterNotNull().map { File(it) }
+    return this.urLs.map { it.toString().removePrefix("file:") }.filterNotNull().map(::File)
 }

-internal fun <T : Any> List<T>.assertNotEmpty(error: String): List<T> {
+internal fun <T : Any> List<T>.ensureNotEmpty(error: String): List<T> {
    if (this.isEmpty()) throw IllegalStateException(error)
    return this
 }
@@ -53,7 +53,7 @@ class GenericReplAnalyzer(environment: KotlinCoreEnvironment,
    private val topDownAnalyzer: LazyTopDownAnalyzer
    private val resolveSession: ResolveSession
    private val scriptDeclarationFactory: ScriptMutableDeclarationProviderFactory
-    private val replState = ResetableReplState()
+    private val replState = ResettableReplState()

    val module: ModuleDescriptorImpl
        get() = stateLock.read { field }
@@ -63,8 +63,8 @@ class GenericReplAnalyzer(environment: KotlinCoreEnvironment,

    init {
        // Module source scope is empty because all binary classes are in the dependency module, and all source classes are guaranteed
-        // to be found via ResolveSession. The latter is true as long as light classes are not needed in NONE (which is currently true
-        // because no symbol declared in the NONE session can be used from Java)
+        // to be found via ResolveSession. The latter is true as long as light classes are not needed in REPL (which is currently true
+        // because no symbol declared in the REPL session can be used from Java)
        val container = TopDownAnalyzerFacadeForJVM.createContainer(
                environment.project,
                emptyList(),
@@ -178,7 +178,8 @@ class GenericReplAnalyzer(environment: KotlinCoreEnvironment,
        }
    }

-    class ResetableReplState() {
+    // TODO: merge with org.jetbrains.kotlin.resolve.repl.ReplState when switching to new REPL infrastruct everywhere
+    class ResettableReplState {
        private val successfulLines = ReplHistory<LineInfo.SuccessfulLine>()
        private val submittedLines = hashMapOf<KtFile, LineInfo>()

@@ -232,5 +233,3 @@ class GenericReplAnalyzer(environment: KotlinCoreEnvironment,
        }
    }
 }
-
-
@@ -27,7 +27,7 @@ import org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport
 import org.jetbrains.kotlin.cli.common.messages.MessageCollector
 import org.jetbrains.kotlin.cli.common.repl.ReplCheckResult
 import org.jetbrains.kotlin.cli.common.repl.ReplCodeLine
-import org.jetbrains.kotlin.cli.common.repl.makeSriptBaseName
+import org.jetbrains.kotlin.cli.common.repl.makeScriptBaseName
 import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles
 import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
 import org.jetbrains.kotlin.cli.jvm.repl.messages.DiagnosticMessageHolder
@@ -74,9 +74,9 @@ open class GenericReplChecker(

    fun check(codeLine: ReplCodeLine, generation: Long): ReplCheckResult {
        stateLock.write {
-            val scriptFileName = makeSriptBaseName(codeLine, generation)
+            val scriptFileName = makeScriptBaseName(codeLine, generation)
            val virtualFile =
-                    LightVirtualFile("${scriptFileName}${KotlinParserDefinition.STD_SCRIPT_EXT}", KotlinLanguage.INSTANCE, codeLine.code).apply {
+                    LightVirtualFile("$scriptFileName${KotlinParserDefinition.STD_SCRIPT_EXT}", KotlinLanguage.INSTANCE, codeLine.code).apply {
                        charset = CharsetToolkit.UTF8_CHARSET
                    }
            val psiFile: KtFile = psiFileFactory.trySetupPsiForFile(virtualFile, KotlinLanguage.INSTANCE, true, false) as KtFile?
@@ -68,7 +68,7 @@ open class GenericReplCompiler(disposable: Disposable,

    override val history: List<ReplCodeLine> get() = stateLock.read { descriptorsHistory.copySources() }

-    override fun check(codeLine: ReplCodeLine): ReplCheckResult {
+    override fun check(codeLine: ReplCodeLine): ReplCheckResult = stateLock.read {
        return checker.check(codeLine, generation.get())
    }

@@ -87,7 +87,7 @@ open class GenericReplCompiler(disposable: Disposable,
                    when (res) {
                        is ReplCheckResult.Incomplete -> return@compile ReplCompileResult.Incomplete(descriptorsHistory.copySources())
                        is ReplCheckResult.Error -> return@compile ReplCompileResult.Error(descriptorsHistory.copySources(), res.message, res.location)
-                        is ReplCheckResult.Ok -> NO_ACTION()
+                        is ReplCheckResult.Ok -> {} // continue
                    }
                }
                Pair(checker.lineState!!.psiFile, checker.lineState!!.errorHolder)
@@ -125,7 +125,7 @@ open class GenericReplCompiler(disposable: Disposable,
                    setOf(psiFile.script!!.getContainingKtFile()),
                    org.jetbrains.kotlin.codegen.CompilationErrorHandler.THROW_EXCEPTION)

-            val generatedClassname = makeSriptBaseName(codeLine, currentGeneration)
+            val generatedClassname = makeScriptBaseName(codeLine, currentGeneration)
            val compiledCodeLine = CompiledReplCodeLine(generatedClassname, codeLine)
            descriptorsHistory.add(compiledCodeLine, scriptDescriptor)

@@ -38,7 +38,7 @@ import org.jetbrains.kotlin.config.Services
 import org.jetbrains.kotlin.daemon.common.*
 import org.jetbrains.kotlin.daemon.incremental.RemoteAnnotationsFileUpdater
 import org.jetbrains.kotlin.daemon.incremental.RemoteArtifactChangesProvider
-import org.jetbrains.kotlin.daemon.incremental.RemoteChangesRegostry
+import org.jetbrains.kotlin.daemon.incremental.RemoteChangesRegistry
 import org.jetbrains.kotlin.daemon.report.CompileServicesFacadeMessageCollector
 import org.jetbrains.kotlin.daemon.report.DaemonMessageReporter
 import org.jetbrains.kotlin.daemon.report.DaemonMessageReporterPrintStreamAdapter