diff --git a/docs/embedding.md b/docs/embedding.md index 74b0fd6..3c1cea5 100644 --- a/docs/embedding.md +++ b/docs/embedding.md @@ -72,6 +72,14 @@ For module-level APIs, the default workflow is: 1. declare globals in Lyng using `extern fun` / `extern val` / `extern var`; 2. bind Kotlin implementation via `ModuleScope.globalBinder()`. +This is also the recommended way to expose a Kotlin-backed value that should behave like a true +Lyng global variable/property. If you need `x` to read/write through Kotlin on every access, use +`extern var` / `extern val` plus `bindGlobalVar(...)`. + +Do not use `addConst(...)` for this case: `addConst(...)` installs a value, not a Kotlin-backed +property accessor. It is appropriate for fixed values and objects, but not for a global that should +delegate reads/writes back into Kotlin state. + ```kotlin import net.sergeych.lyng.bridge.* import net.sergeych.lyng.obj.ObjInt @@ -117,6 +125,12 @@ assertEquals("changed", globalProp) assertEquals("1.0.0", globalVersion) ``` +Minimal rule of thumb: + +- use `bindGlobalFun(...)` for global functions +- use `bindGlobalVar(...)` for Kotlin-backed global variables/properties +- use `addConst(...)` only for fixed values/objects that do not need getter/setter behavior + For custom argument handling and full runtime access: ```kotlin @@ -462,14 +476,21 @@ im.addPackage("my.tools") { module: ModuleScope -> module.eval( """ extern val version: String + extern var status: String extern fun triple(x: Int): Int """.trimIndent() ) val binder = module.globalBinder() + var status = "ready" binder.bindGlobalVar( name = "version", get = { "1.0" } ) + binder.bindGlobalVar( + name = "status", + get = { status }, + set = { status = it } + ) binder.bindGlobalFun1("triple") { x -> ObjInt.of((x * 3).toLong()) } @@ -479,6 +500,7 @@ im.addPackage("my.tools") { module: ModuleScope -> scope.eval(""" import my.tools.* val v = triple(14) + status = "busy" """) val v = scope.eval("v").toKotlin(scope) // -> 42 ``` diff --git a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Script.kt b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Script.kt index 8ee7e6f..d880117 100644 --- a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Script.kt +++ b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/Script.kt @@ -46,6 +46,28 @@ class Script( // private val catchReturn: Boolean = false, ) : Statement() { fun statements(): List = statements + + /** + * Explicitly apply this script's import/module bindings to [scope] without executing the script. + * This is intended for embedding scenarios where the host owns scope lifecycle and wants + * script-specific imports to be a separate, opt-in preparation step. + */ + suspend fun importInto(scope: Scope, seedScope: Scope = scope): Scope { + prepareScriptScope(scope, seedScope) + return scope + } + + /** + * Create a fresh raw module scope and apply this script's import/module bindings to it. + * If [seedScope] is provided, its import provider is reused and seed-bound imports resolve from it. + */ + suspend fun instantiateModule(seedScope: Scope? = null, pos: Pos = this.pos): ModuleScope { + val provider = seedScope?.currentImportProvider ?: defaultImportManager + val module = provider.newModuleAt(pos) + prepareScriptScope(module, seedScope ?: module) + return module + } + override suspend fun execute(scope: Scope): Obj { scope.pos = pos val execScope = resolveModuleScope(scope) ?: scope @@ -75,6 +97,19 @@ class Script( return ObjVoid } + private suspend fun prepareScriptScope(scope: Scope, seedScope: Scope) { + if (importBindings.isNotEmpty() || importedModules.isNotEmpty()) { + seedImportBindings(scope, seedScope) + } + if (moduleSlotPlan.isNotEmpty()) { + scope.applySlotPlan(moduleSlotPlan) + for (name in moduleSlotPlan.keys) { + val record = scope.objects[name] ?: scope.localBindings[name] ?: continue + scope.updateSlotFor(name, record) + } + } + } + private suspend fun seedModuleSlots(scope: Scope, seedScope: Scope) { if (importBindings.isEmpty() && importedModules.isEmpty()) return seedImportBindings(scope, seedScope) diff --git a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/SeedLocals.kt b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/SeedLocals.kt index 4001a35..eaca35c 100644 --- a/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/SeedLocals.kt +++ b/lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/SeedLocals.kt @@ -36,7 +36,7 @@ internal suspend fun seedFrameLocalsFromScope(frame: CmdFrame, scope: Scope) { } else { record.value } - if (value is net.sergeych.lyng.FrameSlotRef && value.refersTo(frame.frame, base + i)) { + if (value is net.sergeych.lyng.FrameSlotRef && value.refersTo(frame.frame, i)) { continue } frame.setObjUnchecked(base + i, value) diff --git a/lynglib/src/commonTest/kotlin/ScriptImportPreparationTest.kt b/lynglib/src/commonTest/kotlin/ScriptImportPreparationTest.kt new file mode 100644 index 0000000..6a82ff2 --- /dev/null +++ b/lynglib/src/commonTest/kotlin/ScriptImportPreparationTest.kt @@ -0,0 +1,87 @@ +/* + * 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.Script +import net.sergeych.lyng.Source +import net.sergeych.lyng.obj.toInt +import net.sergeych.lyng.pacman.ImportManager +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class ScriptImportPreparationTest { + + @Test + fun scriptImportIntoExplicitlyPreparesExistingScope() = runTest { + val manager = ImportManager() + manager.addTextPackages( + """ + package foo + + val answer = 42 + """.trimIndent() + ) + val script = Compiler.compile( + Source( + "", + """ + import foo + answer + """.trimIndent() + ), + manager + ) + val scope = manager.newModule() + + assertNull(scope["answer"]) + + script.importInto(scope) + + val record = assertNotNull(scope["answer"]) + assertEquals(42, scope.resolve(record, "answer").toInt()) + } + + @Test + fun scriptInstantiateModuleUsesSeedScopeImportProvider() = runTest { + val manager = ImportManager() + manager.addTextPackages( + """ + package foo + + val answer = 42 + """.trimIndent() + ) + val script = Compiler.compile( + Source( + "", + """ + import foo + answer + """.trimIndent() + ), + manager + ) + val seedScope = manager.newModule() + + val module = script.instantiateModule(seedScope) + + val record = assertNotNull(module["answer"]) + assertEquals(42, module.resolve(record, "answer").toInt()) + } +} diff --git a/lynglib/src/commonTest/kotlin/SeedLocalsRegressionTest.kt b/lynglib/src/commonTest/kotlin/SeedLocalsRegressionTest.kt new file mode 100644 index 0000000..24964c5 --- /dev/null +++ b/lynglib/src/commonTest/kotlin/SeedLocalsRegressionTest.kt @@ -0,0 +1,60 @@ +/* + * 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.FrameSlotRef +import net.sergeych.lyng.Pos +import net.sergeych.lyng.Scope +import net.sergeych.lyng.bytecode.BytecodeConst +import net.sergeych.lyng.bytecode.CmdFrame +import net.sergeych.lyng.bytecode.CmdFunction +import net.sergeych.lyng.bytecode.CmdVm +import net.sergeych.lyng.bytecode.seedFrameLocalsFromScope +import kotlin.test.Test +import kotlin.test.assertNull + +class SeedLocalsRegressionTest { + + @Test + fun seedFrameLocalsSkipsSelfReferentialFrameSlotRef() = runTest { + val fn = CmdFunction( + name = "seed-self-ref", + localCount = 1, + addrCount = 0, + returnLabels = emptySet(), + scopeSlotCount = 1, + scopeSlotIndices = intArrayOf(0), + scopeSlotNames = arrayOf(null), + scopeSlotIsModule = booleanArrayOf(false), + localSlotNames = arrayOf("x"), + localSlotMutables = booleanArrayOf(true), + localSlotDelegated = booleanArrayOf(false), + localSlotCaptures = booleanArrayOf(false), + constants = listOf(BytecodeConst.IntVal(0)), + cmds = emptyArray(), + posByIp = emptyArray() + ) + val scope = Scope(null, pos = Pos.builtIn) + val record = scope.addConst("x", net.sergeych.lyng.obj.ObjInt.of(1)) + val frame = CmdFrame(CmdVm(), fn, scope, emptyList()) + record.value = FrameSlotRef(frame.frame, 0) + scope.updateSlotFor("x", record) + + seedFrameLocalsFromScope(frame, scope) + + assertNull(frame.frame.getRawObj(0)) + } +} diff --git a/notes/lynglib_compiler_vm_review_2026-03-26.md b/notes/lynglib_compiler_vm_review_2026-03-26.md new file mode 100644 index 0000000..f57670a --- /dev/null +++ b/notes/lynglib_compiler_vm_review_2026-03-26.md @@ -0,0 +1,66 @@ +# :lynglib Compiler and VM Review (2026-03-26) + +## Scope +- Reviewed `:lynglib` compiler and bytecode VM paths, focusing on compile/execution correctness. +- Read core files around `Compiler`, `Script`, `BytecodeCompiler`, `CmdRuntime`, `Scope`, `ModuleScope`, and capture/slot plumbing. +- Ran `./gradlew :lynglib:jvmTest` on 2026-03-26: PASS. + +## Findings + +### 1. High: local seeding uses the wrong slot index when checking for self-referential `FrameSlotRef` +- Status: fixed in worktree on 2026-03-26; covered by `SeedLocalsRegressionTest`. +- File: `lynglib/src/commonMain/kotlin/net/sergeych/lyng/bytecode/SeedLocals.kt:39` +- The self-reference guard compares `FrameSlotRef` against `base + i`, but `FrameSlotRef` stores the underlying `BytecodeFrame` local slot index, not the VM absolute slot id. +- The same slot is then written using `setObjUnchecked(base + i, value)` at `SeedLocals.kt:42`, which means a self-reference is not filtered out before being reinserted. +- Impact: this can seed a local with a reference to itself, creating recursive `FrameSlotRef` chains. Reads then recurse through `BytecodeFrame.getObj()` / `FrameSlotRef.read()` instead of resolving to a value, which is a realistic path to stack overflow or non-terminating execution in bytecode-only helper execution (`executeBytecodeWithSeed`). +- Why this looks real: the equivalent check in `CmdFrame.applyCaptureRecords()` uses the local index directly (`CmdRuntime.kt:3867`), and `Script.seedModuleLocals()` also compares with the local index, not `scopeSlotCount + i` (`Script.kt:109`). +- Suggested fix: compare with `i`, not `base + i`, and add a regression test around `executeBytecodeWithSeed` seeding a scope that already contains a `FrameSlotRef` to the same local. + +### 2. Design note: script-specific import/module preparation should stay explicit, not hidden in `execute(scope)` +- Status: resolved by API addition, without changing `Script.execute(scope)` semantics. +- File: `lynglib/src/commonMain/kotlin/net/sergeych/lyng/Script.kt:51-57` +- `shouldSeedModule` is only true when `execScope` is a `ModuleScope` or `thisObj === ObjVoid`. +- If a compiled script is executed against a non-module scope with a real receiver object, `seedModuleSlots()` is skipped entirely, so script imports and explicit module bindings are never installed into `moduleTarget`. +- Original concern: top-level bytecode that expects imported names or module globals can see different behavior depending on whether the host prepared the target scope explicitly. +- Resolution taken: do **not** change `Script.execute(scope)`, because in this codebase it is expected to run on exactly the provided scope and many embedding flows already rely on explicit scope setup via `Script.newScope()`, `Scope.eval(...)`, `addFn(...)`, `addConst(...)`, and direct import-manager configuration. +- New explicit APIs added instead: + - `Script.importInto(scope, seedScope = scope)` + - `Script.instantiateModule(seedScope = null, pos = script.pos)` +- This keeps existing execution behavior stable while giving hosts an opt-in way to apply a script's import/module bindings when they actually want that preparation step. +- Coverage added: `ScriptImportPreparationTest`. + +### 3. Medium: missing module captures are silently converted into fresh `Unset` slots +- 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. + +### 4. Medium: subject-less `when { ... }` still crashes through a raw Kotlin `TODO` +- 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. + +## Risks Worth Checking Next + +### 5. Medium risk: module frame growth only retargets records stored in `objects`/`localBindings` +- File: `lynglib/src/commonMain/kotlin/net/sergeych/lyng/ModuleScope.kt:39-67` +- When `ensureModuleFrame()` replaces the old `BytecodeFrame` with a larger one, it retargets `FrameSlotRef`s only in `objects` and `localBindings`. +- Existing closures/capture records that already hold `FrameSlotRef(oldFrame, slot)` are not retargeted here. +- Impact: if a module scope survives across recompilation/re-execution with a larger local frame, previously exported closures can keep reading stale values from the old frame instance. +- Confidence is lower because this depends on module reuse across differing compiled shapes, but the reference-retargeting logic is clearly incomplete for that scenario. +- Suggested check: add a regression covering module re-execution with increased local count and an exported closure captured before the resize. + +## Test Status +- `./gradlew :lynglib:jvmTest` passed during this review. +- `./gradlew :lynglib:jvmTest --tests ScriptImportPreparationTest --tests SeedLocalsRegressionTest` passed after the fixes/API additions. +- 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. +5. Decide whether finding 5 matters for current module-reload workflows; add a regression before changing behavior. diff --git a/proposals/wasm_vs_lyng_memory.md b/proposals/wasm_vs_lyng_memory.md new file mode 100644 index 0000000..e69de29 diff --git a/proposals/why_not_wasm.md b/proposals/why_not_wasm.md new file mode 100644 index 0000000..e69de29