diff --git a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Compiler.kt b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Compiler.kt index 23a066f..eaf3e71 100644 --- a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Compiler.kt +++ b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Compiler.kt @@ -6446,7 +6446,7 @@ class Compiler( WhenStatement(value, cases, elseCase, whenPos) } else { // when { cond -> ... } - TODO("when without object is not yet implemented") + throw ScriptError(t.pos, "when without subject is not implemented") } return wrapBytecode(stmt) } diff --git a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/CmdRuntime.kt b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/CmdRuntime.kt index 9059207..cf8ac18 100644 --- a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/CmdRuntime.kt +++ b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/CmdRuntime.kt @@ -3930,7 +3930,7 @@ class CmdFrame( } else { val raw = frame.getRawObj(localIndex) if (raw == null && name != null) { - val record = scope.get(name) + val record = findNamedExistingRecord(scope, name) if (record != null) { val value = record.value return@mapIndexed when (value) { @@ -3939,6 +3939,12 @@ class CmdFrame( else -> ObjRecord(value, isMutable) } } + if (hasNamedScopeBinding(scope, name)) { + throw ScriptError( + ensureScope().pos, + "captured binding '$name' is not available in the execution scope; prepare the script imports/module bindings explicitly" + ) + } } when (raw) { is FrameSlotRef -> ObjRecord(raw, isMutable) @@ -3952,22 +3958,27 @@ class CmdFrame( val target = moduleScope val name = captureNames?.getOrNull(index) if (name != null) { - target.tryGetLocalRecord(target, name, target.currentClassCtx)?.let { return@mapIndexed it } - target.getSlotIndexOf(name)?.let { return@mapIndexed target.getSlotRecord(it) } - target.get(name)?.let { return@mapIndexed it } + findNamedExistingRecord(target, name)?.let { return@mapIndexed it } // Fallback to current scope in case the module scope isn't in the parent chain // or doesn't carry the imported symbol yet. - scope.tryGetLocalRecord(scope, name, scope.currentClassCtx)?.let { return@mapIndexed it } - scope.get(name)?.let { return@mapIndexed it } + findNamedExistingRecord(scope, name)?.let { return@mapIndexed it } } if (slotId < target.slotCount) { - return@mapIndexed target.getSlotRecord(slotId) + val existing = target.getSlotRecord(slotId) + if (name == null || existing.value !== ObjUnset || hasResolvedNamedScopeBinding(target, name)) { + return@mapIndexed existing + } } if (name != null) { - target.applySlotPlan(mapOf(name to slotId)) - return@mapIndexed target.getSlotRecord(slotId) + throw ScriptError( + ensureScope().pos, + "module capture '$name' is not available in the execution scope; prepare the script imports/module bindings explicitly" + ) } - error("Missing module capture slot $slotId") + throw ScriptError( + ensureScope().pos, + "missing module capture slot $slotId" + ) } } } @@ -4776,12 +4787,13 @@ class CmdFrame( private suspend fun getScopeSlotValue(slot: Int): Obj { val target = scopeTarget(slot) + val name = fn.scopeSlotNames[slot] + val hadNamedBinding = name != null && hasResolvedNamedScopeBinding(target, name) val index = ensureScopeSlot(target, slot) val record = target.getSlotRecord(index) val direct = record.value if (direct is FrameSlotRef) return direct.read() if (direct is RecordSlotRef) return direct.read() - val name = fn.scopeSlotNames[slot] if (name != null && record.memberName != null && record.memberName != name) { val resolved = target.get(name) if (resolved != null) { @@ -4802,9 +4814,15 @@ class CmdFrame( return direct } if (name == null) return record.value - val resolved = target.get(name) ?: return record.value + val resolved = target.get(name) + if (resolved == null) { + failMissingPreparedModuleBinding(slot, name, hadNamedBinding, record) + return record.value + } if (resolved.value !== ObjUnset) { target.updateSlotFor(name, resolved) + } else { + failMissingPreparedModuleBinding(slot, name, hadNamedBinding, resolved) } return resolved.value } @@ -4812,12 +4830,13 @@ class CmdFrame( private suspend fun getScopeSlotValueAtAddr(addrSlot: Int): Obj { val target = addrScopes[addrSlot] ?: error("Address slot $addrSlot is not resolved") val index = addrIndices[addrSlot] + val slotId = addrScopeSlots[addrSlot] + val name = fn.scopeSlotNames.getOrNull(slotId) + val hadNamedBinding = name != null && hasResolvedNamedScopeBinding(target, name) val record = target.getSlotRecord(index) val direct = record.value if (direct is FrameSlotRef) return direct.read() if (direct is RecordSlotRef) return direct.read() - val slotId = addrScopeSlots[addrSlot] - val name = fn.scopeSlotNames.getOrNull(slotId) if (name != null && record.memberName != null && record.memberName != name) { val resolved = target.get(name) if (resolved != null) { @@ -4838,9 +4857,15 @@ class CmdFrame( return direct } if (name == null) return record.value - val resolved = target.get(name) ?: return record.value + val resolved = target.get(name) + if (resolved == null) { + failMissingPreparedModuleBinding(slotId, name, hadNamedBinding, record) + return record.value + } if (resolved.value !== ObjUnset) { target.updateSlotFor(name, resolved) + } else { + failMissingPreparedModuleBinding(slotId, name, hadNamedBinding, resolved) } return resolved.value } @@ -4875,6 +4900,41 @@ class CmdFrame( return index } + private fun hasNamedScopeBinding(target: Scope, name: String): Boolean { + if (target.tryGetLocalRecord(target, name, target.currentClassCtx) != null) return true + if (target.getSlotIndexOf(name) != null) return true + if (target.get(name) != null) return true + return false + } + + private fun hasResolvedNamedScopeBinding(target: Scope, name: String): Boolean = + findNamedExistingRecord(target, name) != null + + private fun findNamedExistingRecord(target: Scope, name: String): ObjRecord? { + target.tryGetLocalRecord(target, name, target.currentClassCtx)?.let { return it } + target.get(name)?.let { record -> + if (record.value !== ObjUnset || record.type == ObjRecord.Type.Delegated || record.type == ObjRecord.Type.Property) { + return record + } + } + return null + } + + private fun failMissingPreparedModuleBinding( + slot: Int, + name: String, + hadNamedBinding: Boolean, + record: ObjRecord + ) { + if (hadNamedBinding) return + if (record.value !== ObjUnset) return + if (fn.scopeSlotIsModule.getOrNull(slot) != true) return + throw ScriptError( + ensureScope().pos, + "module binding '$name' is not available in the execution scope; prepare the script imports/module bindings explicitly" + ) + } + private fun ensureLocalMutable(localIndex: Int) { val name = fn.localSlotNames.getOrNull(localIndex) ?: return val isMutable = fn.localSlotMutables.getOrNull(localIndex) ?: true diff --git a/lynglib/src/commonTest/kotlin/CompilerVmReviewRegressionTest.kt b/lynglib/src/commonTest/kotlin/CompilerVmReviewRegressionTest.kt new file mode 100644 index 0000000..f3ca041 --- /dev/null +++ b/lynglib/src/commonTest/kotlin/CompilerVmReviewRegressionTest.kt @@ -0,0 +1,88 @@ +/* + * Copyright 2026 Sergey S. Chernov + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import kotlinx.coroutines.test.runTest +import net.sergeych.lyng.Compiler +import net.sergeych.lyng.Scope +import net.sergeych.lyng.Script +import net.sergeych.lyng.ScriptError +import net.sergeych.lyng.Source +import net.sergeych.lyng.asFacade +import net.sergeych.lyng.obj.ObjString +import net.sergeych.lyng.obj.toInt +import net.sergeych.lyng.pacman.ImportManager +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class CompilerVmReviewRegressionTest { + + @Test + fun missingModuleCaptureFailsFastInsteadOfBecomingUnset() = runTest { + val manager = ImportManager() + manager.addTextPackages( + """ + package foo + + val answer = 42 + """.trimIndent() + ) + val script = Compiler.compile( + Source( + "", + """ + import foo + fun make() = { answer } + make() + """.trimIndent() + ), + manager + ) + + val prepared = manager.newModule() + script.importInto(prepared) + val preparedLambda = script.execute(prepared) + assertEquals(42, prepared.asFacade().call(preparedLambda).toInt()) + + val rawModule = manager.newModule() + val hostScope = Scope(parent = rawModule, thisObj = ObjString("receiver")) + + val lambda = script.execute(hostScope) + val ex = assertFailsWith { + hostScope.asFacade().call(lambda) + } + assertContains(ex.errorMessage, "module binding 'answer'") + } + + @Test + fun subjectlessWhenReportsScriptError() = runTest { + val ex = assertFailsWith { + Compiler.compile( + Source( + "", + """ + when { + true -> 1 + } + """.trimIndent() + ), + Script.defaultImportManager + ) + } + assertContains(ex.errorMessage, "when without subject") + } +} diff --git a/notes/lynglib_compiler_vm_review_2026-03-26.md b/notes/lynglib_compiler_vm_review_2026-03-26.md index f57670a..628b953 100644 --- a/notes/lynglib_compiler_vm_review_2026-03-26.md +++ b/notes/lynglib_compiler_vm_review_2026-03-26.md @@ -30,18 +30,20 @@ - Coverage added: `ScriptImportPreparationTest`. ### 3. Medium: missing module captures are silently converted into fresh `Unset` slots +- Status: fixed in worktree on 2026-03-27; covered by `CompilerVmReviewRegressionTest`. - File: `lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/CmdRuntime.kt:3950-3970` - In `CmdFrame.buildCaptureRecords()`, the module-capture path first tries several lookups. If the requested `slotId` is missing but the capture has a `name`, it calls `target.applySlotPlan(mapOf(name to slotId))` and immediately returns `target.getSlotRecord(slotId)`. - That record is a newly created placeholder from `Scope.applySlotPlan()` (`Scope.kt:460-471`) and defaults to `ObjUnset`. - Impact: a compiler/runtime disagreement in capture resolution is masked as a normal capture of `Unset`, so the failure moves far away from closure creation and becomes data corruption or an unrelated later exception. This will be difficult to debug when it happens. -- Suggested fix: if the named module capture cannot be resolved to an existing record, fail immediately with a Lyng error instead of manufacturing a placeholder slot. Add a regression test around missing imported/module captures. +- Resolution taken: the VM now refuses to treat synthetic placeholder slots as real module/captured bindings. Reads and capture construction fail with a source-positioned `ScriptError` when the execution scope was not prepared with the script's required module/import bindings. ### 4. Medium: subject-less `when { ... }` still crashes through a raw Kotlin `TODO` +- Status: fixed in worktree on 2026-03-27; covered by `CompilerVmReviewRegressionTest`. - File: `lynglib/src/commonMain/kotlin/net/sergeych/lyng/Compiler.kt:6447-6449` - The unsupported branch uses `TODO("when without object is not yet implemented")`. - Current docs explicitly say subject-less `when` is not implemented, so the language limitation itself is documented. The problem is the failure mode: the compiler throws a raw Kotlin `NotImplementedError` instead of a normal `ScriptError` or a feature diagnostic. - Impact: IDE/embedding callers get an implementation exception rather than a source-positioned language error, which is especially bad across non-JVM targets and for editor tooling. -- Suggested fix: replace the `TODO(...)` with a `ScriptError` at the current source position, or gate it earlier in parsing with a normal diagnostic. +- Resolution taken: the parser now throws a normal `ScriptError` at the `when` position with an explicit "when without subject is not implemented" message. ## Risks Worth Checking Next @@ -56,11 +58,12 @@ ## Test Status - `./gradlew :lynglib:jvmTest` passed during this review. - `./gradlew :lynglib:jvmTest --tests ScriptImportPreparationTest --tests SeedLocalsRegressionTest` passed after the fixes/API additions. +- `./gradlew :lynglib:jvmTest --tests CompilerVmReviewRegressionTest --tests ScriptImportPreparationTest --tests SeedLocalsRegressionTest` passed after fixing findings 3 and 4. - Finding 1 is covered directly; finding 2 is covered by explicit preparation API tests. ## Suggested Fix Order 1. Fix finding 1 first: it is a concrete slot-index bug with likely recursive failure modes. Done. 2. Keep `Script.execute(scope)` semantics stable and use explicit preparation APIs where script-owned import/module setup is needed. Done. -3. Tighten finding 3 next: fail fast on capture mismatches. -4. Replace the raw `TODO` in finding 4 so unsupported syntax produces normal diagnostics. +3. Tighten finding 3 next: fail fast on capture mismatches. Done. +4. Replace the raw `TODO` in finding 4 so unsupported syntax produces normal diagnostics. Done. 5. Decide whether finding 5 matters for current module-reload workflows; add a regression before changing behavior.