From 53d5a528f72f6f0cc64b99a80171accb1c41c255 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 17:24:36 +0000 Subject: [PATCH 1/4] test: diamond includes must reach a shared file twice without error Red evidence for two include defects seen on wfl 26.9.2. When two files both `include from` the same shared file, the second one fails while being analyzed ("'shout' is not a function") because actions already in the enclosing scope were seeded into its analyzer as plain variables, and even a direct double include re-runs the file into the same scope ("Variable 'shout' has already been defined at line 0"). Adds a Rust integration test (diamond, direct double include, include inside an action body per call, include already visible from an outer scope, genuine cycle still rejected) and a gated TestPrograms program with its fixture files under tests/fixtures/modules/diamond/. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018917NeaHViFzeZTQYcTNn5 --- TestPrograms/modules/include_diamond.wfl | 16 ++ tests/fixtures/modules/diamond/auth.wfl | 4 + tests/fixtures/modules/diamond/render.wfl | 4 + tests/fixtures/modules/diamond/util.wfl | 6 + tests/include_diamond_test.rs | 198 ++++++++++++++++++++++ 5 files changed, 228 insertions(+) create mode 100644 TestPrograms/modules/include_diamond.wfl create mode 100644 tests/fixtures/modules/diamond/auth.wfl create mode 100644 tests/fixtures/modules/diamond/render.wfl create mode 100644 tests/fixtures/modules/diamond/util.wfl create mode 100644 tests/include_diamond_test.rs diff --git a/TestPrograms/modules/include_diamond.wfl b/TestPrograms/modules/include_diamond.wfl new file mode 100644 index 00000000..e2e41ea0 --- /dev/null +++ b/TestPrograms/modules/include_diamond.wfl @@ -0,0 +1,16 @@ +// Diamond include: `auth.wfl` and `render.wfl` both `include from "util.wfl"`. +// The second arrival at `util.wfl` must be a no-op (its definitions are +// already in this scope), not an "already defined" failure, and both +// branches must be able to call the shared action. +include from "../../tests/fixtures/modules/diamond/auth.wfl" +include from "../../tests/fixtures/modules/diamond/render.wfl" + +describe "diamond include": + test "both branches see the shared action": + expect auth_check of "alice" to equal "alice!" + expect render_page of "home" to equal "home!" + end test + test "the shared file ran exactly once": + expect util_loads to equal 1 + end test +end describe diff --git a/tests/fixtures/modules/diamond/auth.wfl b/tests/fixtures/modules/diamond/auth.wfl new file mode 100644 index 00000000..6424fd78 --- /dev/null +++ b/tests/fixtures/modules/diamond/auth.wfl @@ -0,0 +1,4 @@ +include from "util.wfl" +define action called auth_check with parameters who: + give back shout of who +end action diff --git a/tests/fixtures/modules/diamond/render.wfl b/tests/fixtures/modules/diamond/render.wfl new file mode 100644 index 00000000..aa9229eb --- /dev/null +++ b/tests/fixtures/modules/diamond/render.wfl @@ -0,0 +1,4 @@ +include from "util.wfl" +define action called render_page with parameters title: + give back shout of title +end action diff --git a/tests/fixtures/modules/diamond/util.wfl b/tests/fixtures/modules/diamond/util.wfl new file mode 100644 index 00000000..d3d86dda --- /dev/null +++ b/tests/fixtures/modules/diamond/util.wfl @@ -0,0 +1,6 @@ +// Shared leaf of the include diamond. Both `auth.wfl` and `render.wfl` +// include this file; it must be safe to reach it twice. +store util_loads as 1 +define action called shout with parameters msg: + give back msg with "!" +end action diff --git a/tests/include_diamond_test.rs b/tests/include_diamond_test.rs new file mode 100644 index 00000000..7b4e59ce --- /dev/null +++ b/tests/include_diamond_test.rs @@ -0,0 +1,198 @@ +//! Diamond includes: two files that both `include from` the same shared file +//! must be usable together. The shared file's definitions already live in the +//! scope after the first include, so a second include of the same file into +//! that scope (or a scope that can already see it) is a no-op rather than a +//! fatal "already defined" error. A genuine cycle is still rejected. + +use std::fs; +use std::path::Path; +use tempfile::TempDir; +use wfl::interpreter::Interpreter; +use wfl::interpreter::value::Value; +use wfl::lexer::lex_wfl_with_positions; +use wfl::parser::Parser; + +async fn run(main_file: &Path) -> Result { + let source = fs::read_to_string(main_file).expect("read main"); + let tokens = lex_wfl_with_positions(&source); + let ast = Parser::new(&tokens) + .parse() + .unwrap_or_else(|e| panic!("Parse failed: {e:?}")); + let mut interpreter = Interpreter::new(); + interpreter.set_source_file(main_file.to_path_buf()); + match interpreter.interpret(&ast).await { + Ok(_) => Ok(interpreter), + Err(errors) => Err(errors + .iter() + .map(|e| e.message.clone()) + .collect::>() + .join("\n")), + } +} + +fn global(interpreter: &Interpreter, name: &str) -> Value { + interpreter + .global_env() + .borrow() + .get(name) + .unwrap_or_else(|| panic!("`{name}` is not defined in the global scope")) +} + +fn assert_text(interpreter: &Interpreter, name: &str, expected: &str) { + match global(interpreter, name) { + Value::Text(t) => assert_eq!(&*t, expected, "value of `{name}`"), + other => panic!("`{name}` should be text, got {other:?}"), + } +} + +fn assert_number(interpreter: &Interpreter, name: &str, expected: f64) { + match global(interpreter, name) { + Value::Number(n) => assert_eq!(n, expected, "value of `{name}`"), + other => panic!("`{name}` should be a number, got {other:?}"), + } +} + +fn write_diamond(dir: &Path) { + fs::write( + dir.join("util.wfl"), + r#" +store util_loads as 1 +define action called shout with parameters msg: + give back msg with "!" +end action +"#, + ) + .unwrap(); + fs::write( + dir.join("auth.wfl"), + r#" +include from "util.wfl" +define action called auth_check with parameters who: + give back shout of who +end action +"#, + ) + .unwrap(); + fs::write( + dir.join("render.wfl"), + r#" +include from "util.wfl" +define action called render_page with parameters title: + give back shout of title +end action +"#, + ) + .unwrap(); +} + +#[tokio::test] +async fn diamond_include_reaches_shared_file_twice_without_error() { + let dir = TempDir::new().unwrap(); + write_diamond(dir.path()); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +include from "auth.wfl" +include from "render.wfl" +store a as auth_check of "alice" +store r as render_page of "home" +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("diamond include failed: {e}")); + assert_text(&interpreter, "a", "alice!"); + assert_text(&interpreter, "r", "home!"); + // The shared leaf executed once: its `store` did not run a second time. + assert_number(&interpreter, "util_loads", 1.0); +} + +#[tokio::test] +async fn direct_double_include_of_same_file_is_a_no_op() { + let dir = TempDir::new().unwrap(); + write_diamond(dir.path()); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +include from "util.wfl" +include from "util.wfl" +store s as shout of "x" +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("double include failed: {e}")); + assert_text(&interpreter, "s", "x!"); +} + +#[tokio::test] +async fn include_inside_action_body_repeats_per_call_scope() { + // Each call of `prepare_greeting` gets a fresh local scope that cannot see the + // previous call's include, so the file must run again for that scope. + let dir = TempDir::new().unwrap(); + write_diamond(dir.path()); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +define action called prepare_greeting with parameters who: + include from "util.wfl" + give back shout of who +end action +store first as prepare_greeting of "a" +store second as prepare_greeting of "b" +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("per-call include failed: {e}")); + assert_text(&interpreter, "first", "a!"); + assert_text(&interpreter, "second", "b!"); +} + +#[tokio::test] +async fn include_already_visible_from_outer_scope_is_skipped_inside_action() { + // `util.wfl` is included at top level; an action body that includes it + // again can already see its definitions, so the include must not re-run + // the file into the local scope (which would collide with the outer one). + let dir = TempDir::new().unwrap(); + write_diamond(dir.path()); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +include from "util.wfl" +define action called prepare_greeting with parameters who: + include from "util.wfl" + give back shout of who +end action +store first as prepare_greeting of "a" +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("nested re-include failed: {e}")); + assert_text(&interpreter, "first", "a!"); + assert_number(&interpreter, "util_loads", 1.0); +} + +#[tokio::test] +async fn genuine_include_cycle_is_still_rejected() { + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("a.wfl"), "include from \"b.wfl\"\n").unwrap(); + fs::write(dir.path().join("b.wfl"), "include from \"a.wfl\"\n").unwrap(); + let err = run(&dir.path().join("a.wfl")) + .await + .err() + .expect("a.wfl <-> b.wfl cycle must fail"); + assert!( + err.contains("Circular dependency detected"), + "unexpected error: {err}" + ); +} From b1e90b6a2af4e6e56ca5347b151e1ea60a431325 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 17:24:36 +0000 Subject: [PATCH 2/4] fix: make diamond includes work; include a file once per visible scope Two defects, one hiding the other, made any second file that used an action from an earlier include fail: 1. The interpreter seeded every enclosing-scope binding into an included file's analyzer as a plain variable, actions included. Calling one of them then hit the fatal "'x' is not a function" error. The scope is now snapshotted as typed variables plus action signatures (snapshot_parent_scope), and the analyzer registers the actions as real function symbols (register_parent_actions) with their true parameter lists, so calls resolve and a same-name definition is treated as an overload under the existing rules. 2. Re-including a file re-ran it into the same scope, colliding with its own earlier definitions. Environment now records the canonical paths `include from` has completed in a scope; an include whose file is already visible from the current scope (there or on an ancestor) is a no-op. Only a file that ran to completion is recorded, so a failed include can be retried. A genuine cycle is still rejected. `load module` is unchanged. Docs: new "Including the same file more than once (diamond includes)" section in Docs/04-advanced-features/modules.md, limitation and summary updated. Dev diary entry under History/dev-diary/2026/. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018917NeaHViFzeZTQYcTNn5 --- Docs/04-advanced-features/modules.md | 69 ++++++++- .../2026/2026-09-04-diamond-includes.md | 85 +++++++++++ src/analyzer/mod.rs | 36 +++++ src/interpreter/environment.rs | 33 ++++ src/interpreter/mod.rs | 141 +++++++++++++----- 5 files changed, 323 insertions(+), 41 deletions(-) create mode 100644 History/dev-diary/2026/2026-09-04-diamond-includes.md diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 352281da..80382fda 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -75,6 +75,68 @@ display line All three forms work at the top level and inside your own action bodies. Because the analyzer does not read included files, it emits a **non-fatal** `Undefined action ''` note for a name it cannot see statically — the program still runs and the action resolves at runtime. +### Including the same file more than once (diamond includes) + +Larger programs often end up including a shared file from more than one +place. The classic shape is a **diamond**: two library files both include the +same helper file, and the main program includes both libraries. + +```text + util.wfl + / \ + auth.wfl render.wfl + \ / + main.wfl +``` + +```wfl +# util.wfl +define action called shout with parameters msg: + give back msg with "!" +end action + +# auth.wfl +include from "util.wfl" +define action called auth_check with parameters who: + give back shout of who +end action + +# render.wfl +include from "util.wfl" +define action called render_page with parameters title: + give back shout of title +end action + +# main.wfl +include from "auth.wfl" +include from "render.wfl" +display auth_check of "alice" # alice! +display render_page of "home" # home! +``` + +This works because an include runs **once per scope**. `auth.wfl` brings +`util.wfl` into the main program's scope. When `render.wfl` then asks for +`util.wfl` again, its definitions are already visible in that scope, so the +second `include from` does nothing — it does not run the file a second time, +and it does not raise an "already defined" error. Each file can therefore +honestly declare what it depends on, and the order in which the main program +includes its libraries does not matter. + +Two consequences worth knowing: + +- **Includes are for definitions, not for repeating side effects.** A file + included twice into the same scope runs once. If you want a file's + statements to run every time, use `load module from` instead. +- **"Same scope" includes scopes you can see.** An include inside an action + body is skipped when the file was already included by an enclosing scope + (its definitions are already reachable). A file included only inside an + action body runs again on each call, because each call starts with a + fresh local scope that has not seen it. + +A genuine cycle — `a.wfl` includes `b.wfl`, which includes `a.wfl` — is +still reported as a circular dependency (see +[Circular Dependency Protection](#circular-dependency-protection)). + ### Type Checking in Included Files Included files go through the same pipeline as the main program (parse, analyze, type check). Because `include from` runs the file in the parent scope — as if the code were written in the main program — type-check findings in an included file are reported the same way as in the main file: as **non-fatal warnings**. The program still runs. @@ -615,9 +677,12 @@ export constant VERSION - Must load entire module - Future: `load function1, function2 from "x.wfl"` planned -3. **No Module Caching** +3. **No Module Caching for `load module`** - Each `load module` re-parses and re-executes - Multiple loads of same file execute multiple times + - (`include from` is different: it runs a file once per scope, so + diamond includes are safe — see + [Including the same file more than once](#including-the-same-file-more-than-once-diamond-includes)) - Future: Optional caching planned 4. **Export Foundation Only** @@ -850,7 +915,7 @@ load module from "expensive.wfl" # Uses cached version WFL's hybrid module system provides flexible code organization: - **`load module from "path.wfl"`** - Isolated execution for initialization and side effects -- **`include from "path.wfl"`** - Parent scope execution for shared libraries and containers +- **`include from "path.wfl"`** - Parent scope execution for shared libraries and containers; a file is included once per scope, so diamond includes are safe - **`export container/action/constant NAME`** - Foundation for future namespace system - Paths resolve relative to the including file - Circular dependencies are automatically detected diff --git a/History/dev-diary/2026/2026-09-04-diamond-includes.md b/History/dev-diary/2026/2026-09-04-diamond-includes.md new file mode 100644 index 00000000..c81e5a78 --- /dev/null +++ b/History/dev-diary/2026/2026-09-04-diamond-includes.md @@ -0,0 +1,85 @@ +# 2026-09-04 — Diamond includes: the second branch always broke + +## Symptom + +A multi-file program where two library files both `include from` the same +shared file could not run: + +```text + util.wfl + / \ + auth.wfl render.wfl + \ / + main.wfl +``` + +```text +error[ERROR]: Semantic error in included file 'render.wfl': + Semantic error at line 3, column 21: 'shout' is not a function +``` + +The first branch (`auth.wfl`) worked; the second (`render.wfl`) failed while +being *analyzed*, before a single line of it ran. Verified on wfl 26.9.2. + +A downstream project had noticed this and concluded that "WFL includes form a +tree, and diamonds break", working around it by chaining every file into one +long line (`util <- db <- auth <- render <- site_ext <- main`). The chain works +only by accident: with every `include from` at the top of its file, nothing +is defined yet when each file is analyzed, so unknown names are downgraded to +warnings and resolve at runtime. + +## Root cause + +Two separate defects, one hiding the other. + +1. **Parent-scope actions were seeded into an included file's analyzer as + plain variables.** `extract_parent_variables` turned every runtime binding + into `SymbolKind::Variable`, actions included. Calling one of those + (`shout of title`) then hit the analyzer's "'shout' is not a function" + error, which is fatal for included files. This broke any second file that + used an action an earlier include had defined — the diamond, but also the + plain sibling order where `render.wfl` does not include `util.wfl` itself. + +2. **Re-including a file re-ran it into the same scope.** Once (1) is fixed, + the second arrival at `util.wfl` executes `store util_loads as 1` and + `define action called shout` again in a scope that already has both: + "Variable 'shout' has already been defined at line 0". The cycle check + only knew about files *currently* loading, not files already finished. + +## Fix + +- The interpreter now snapshots the enclosing scope as typed variables + **and** action signatures (`snapshot_parent_scope`), and the analyzer gets + the actions through `register_parent_actions` as real function symbols + with their true parameter lists. A same-name definition in the analyzed + file is treated as an overload under the existing distinctness rules, + which matches what the runtime already did. +- `Environment` records the canonical paths `include from` has completed in + that scope. An include whose file is already visible from the current + scope (recorded there or on an ancestor) is a no-op. Only a file that ran + to completion is recorded, so a failed include can be retried. `load + module` is unchanged: it exists to run a file for its side effects. + +Behavior that changes: a file included twice into the same scope used to +run twice if it contained only side effects (anything with a definition +already failed). It now runs once; `load module from` is the documented tool +for "run this file every time". + +## Evidence + +- Red: `tests/include_diamond_test.rs` (5 tests) and + `TestPrograms/modules/include_diamond.wfl` fail on the unmodified + interpreter with the errors quoted above. +- Green: same tests pass after the change; `cargo test --workspace`, + `cargo clippy --all-targets --all-features -- -D warnings`, and the + gated `TestPrograms/` run are clean. +- Risk class R3 (backward compatibility). Negative paths covered: a genuine + include cycle is still rejected; an include inside an action body still + runs per call when nothing enclosing has included the file. + +## Residual + +The type checker does not follow includes, so a container defined in a +shared file still produces non-fatal "Container type 'X' not found" warnings +in a sibling file that instantiates it. That predates this change and the +program runs correctly; it is a separate issue. diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index dd36c36d..9f08a557 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -733,6 +733,42 @@ impl Analyzer { analyzer } + /// Seeds actions that already exist in the enclosing runtime scope (for + /// example, actions an earlier `include from` brought in) as real + /// function symbols with their true signatures. Seeding them as plain + /// variables would make every call to them a fatal "is not a function" + /// error in the file being analyzed, which is what broke diamond and + /// sibling includes. Like the other parent-scope symbols these carry + /// position 0:0; a same-name definition in the analyzed file is treated + /// as an overload under the usual distinctness rules. + pub fn register_parent_actions(&mut self, actions: Vec<(String, Vec)>) { + for (name, signatures) in actions { + let param_types = signatures + .first() + .map(|sig| { + sig.parameters + .iter() + .map(|p| p.param_type.clone().unwrap_or(Type::Unknown)) + .collect() + }) + .unwrap_or_default(); + let symbol = Symbol { + name: name.clone(), + kind: SymbolKind::Function { signatures }, + symbol_type: Some(Type::Function { + parameters: param_types, + return_type: Box::new(Type::Unknown), + }), + line: 0, + column: 0, + }; + self.current_scope + .symbols + .insert(name.clone(), symbol.clone()); + self.baseline_symbols.insert(name, symbol); + } + } + pub fn is_builtin_function(name: &str) -> bool { crate::builtins::is_builtin_function(name) } diff --git a/src/interpreter/environment.rs b/src/interpreter/environment.rs index 98c4cdbd..8c73e4ce 100644 --- a/src/interpreter/environment.rs +++ b/src/interpreter/environment.rs @@ -1,6 +1,7 @@ use super::value::Value; use std::cell::RefCell; use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; use std::rc::{Rc, Weak}; #[derive(Debug)] @@ -11,6 +12,12 @@ pub struct Environment { /// When true, provides module isolation: values from parent scopes are deep cloned /// to prevent mutations, and assignment to parent variables is prevented. pub isolated: bool, + /// Canonical paths of the files `include from` has already run in this + /// scope. An include of a file whose definitions are already visible + /// here (recorded on this scope or an ancestor) is a no-op, which is + /// what makes diamond includes work: two files that both include the + /// same shared file reach it once, not twice. + pub included_files: HashSet, } impl Environment { @@ -23,6 +30,7 @@ impl Environment { constants: HashSet::new(), parent: None, isolated: false, + included_files: HashSet::new(), })) } @@ -35,6 +43,7 @@ impl Environment { constants: HashSet::new(), parent: Some(Rc::downgrade(parent)), isolated: false, + included_files: HashSet::new(), })) } @@ -48,6 +57,7 @@ impl Environment { constants: HashSet::new(), parent: Some(Rc::downgrade(parent)), isolated: false, + included_files: HashSet::new(), })) } @@ -64,9 +74,32 @@ impl Environment { constants: HashSet::new(), parent: Some(Rc::downgrade(parent)), isolated: true, + included_files: HashSet::new(), })) } + /// True when `path` was already included into this scope or into any + /// scope this one can see, so its definitions are already reachable. + pub fn has_included(&self, path: &Path) -> bool { + if self.included_files.contains(path) { + return true; + } + let mut parent = self.parent.as_ref().and_then(|weak| weak.upgrade()); + while let Some(env) = parent { + let env_ref = env.borrow(); + if env_ref.included_files.contains(path) { + return true; + } + parent = env_ref.parent.as_ref().and_then(|weak| weak.upgrade()); + } + false + } + + /// Records that `path` has been included into this scope. + pub fn mark_included(&mut self, path: PathBuf) { + self.included_files.insert(path); + } + pub fn define(&mut self, name: &str, value: Value) -> Result<(), String> { // Check if the variable already exists in current scope if self.values.contains_key(name) { diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index d577b598..76e3e18b 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1249,6 +1249,14 @@ impl Drop for OutboundStreamCleanup { } /// RAII guard that ensures module loading context is restored on scope exit. +/// What an included or loaded file can see from its enclosing scope, +/// split for the analyzer: typed variables and action signatures. +#[derive(Default)] +struct ParentScopeSnapshot { + variables: HashMap, + actions: Vec<(String, Vec)>, +} + /// Automatically removes its loading_stack entry and restores /// current_source_file when dropped. /// @@ -4913,42 +4921,80 @@ impl Interpreter { /// Extract variables from the environment for module analyzer /// Returns a HashMap of variable names to (inferred type, is_mutable) - fn extract_parent_variables( - env: &Rc>, - ) -> HashMap { - let mut vars = HashMap::new(); - let env_borrowed = env.borrow(); - - for (name, value) in &env_borrowed.values { - // Skip native builtins (e.g. `year`, `month`, `day`, `length`, ...). - // The analyzer already resolves these through `is_builtin_function`, - // so seeding them as parent *variables* only makes an included file - // stricter than the main file: an action-local `store year as ...` - // would fatally conflict with the builtin's outer-scope binding even - // though the same code runs fine in a main program (#557). Leaving - // them out lets locals shadow builtins consistently in both paths. - if matches!(value, Value::NativeFunction(_, _)) { - continue; - } - let inferred_type = Self::infer_type_from_value(value); - // Check if this variable is a constant (immutable) - let is_mutable = !env_borrowed.constants.contains(name); - vars.insert(name.clone(), (inferred_type, is_mutable)); - } - - // Also extract from parent scopes - if let Some(parent_weak) = &env_borrowed.parent - && let Some(parent_rc) = parent_weak.upgrade() - { - drop(env_borrowed); // Release borrow before recursive call - let parent_vars = Self::extract_parent_variables(&parent_rc); - // Parent variables are added first, can be shadowed by current scope - for (name, (ty, is_mut)) in parent_vars { - vars.entry(name).or_insert((ty, is_mut)); + /// Snapshot of what an included/loaded file can see from its enclosing + /// runtime scope, split the way the analyzer wants it: plain bindings + /// as typed variables and actions as function signatures. + fn snapshot_parent_scope(env: &Rc>) -> ParentScopeSnapshot { + let mut snapshot = ParentScopeSnapshot::default(); + // Nearest scope wins: a name bound here shadows the same name further + // out, whatever kind of binding each one is. + let mut seen: HashSet = HashSet::new(); + let mut current = Some(Rc::clone(env)); + while let Some(scope) = current { + let scope_ref = scope.borrow(); + for (name, value) in &scope_ref.values { + if !seen.insert(name.clone()) { + continue; + } + match value { + // Skip native builtins (e.g. `year`, `month`, `day`, `length`, ...). + // The analyzer already resolves these through `is_builtin_function`, + // so seeding them as parent *variables* only makes an included file + // stricter than the main file: an action-local `store year as ...` + // would fatally conflict with the builtin's outer-scope binding even + // though the same code runs fine in a main program (#557). Leaving + // them out lets locals shadow builtins consistently in both paths. + Value::NativeFunction(_, _) => {} + Value::Function(func) => { + snapshot + .actions + .push((name.clone(), vec![Self::signature_of(func)])); + } + Value::Overloaded(set) => { + let signatures = set + .overloads + .iter() + .map(|f| Self::signature_of(f)) + .collect(); + snapshot.actions.push((name.clone(), signatures)); + } + _ => { + let inferred_type = Self::infer_type_from_value(value); + // Check if this variable is a constant (immutable) + let is_mutable = !scope_ref.constants.contains(name); + snapshot + .variables + .insert(name.clone(), (inferred_type, is_mutable)); + } + } } + current = scope_ref.parent.as_ref().and_then(|weak| weak.upgrade()); } - vars + snapshot + } + + /// The analyzer-side signature of a runtime action value. + fn signature_of( + func: &crate::interpreter::value::FunctionValue, + ) -> crate::analyzer::FunctionSignature { + use crate::parser::ast::Parameter; + let parameters = func + .params + .iter() + .zip(func.param_types.iter().chain(std::iter::repeat(&None))) + .map(|(param_name, param_type)| Parameter { + name: param_name.clone(), + param_type: param_type.clone(), + default_value: None, + line: func.line, + column: func.column, + }) + .collect(); + crate::analyzer::FunctionSignature { + parameters, + return_type: None, + } } /// Infer AST Type from runtime Value @@ -8535,8 +8581,9 @@ impl Interpreter { use crate::analyzer::Analyzer; // Extract parent variables from current environment for module analyzer - let parent_vars = Self::extract_parent_variables(&env); - let mut analyzer = Analyzer::with_parent_variables(parent_vars); + let parent_scope = Self::snapshot_parent_scope(&env); + let mut analyzer = Analyzer::with_parent_variables(parent_scope.variables); + analyzer.register_parent_actions(parent_scope.actions); if let Err(errors) = analyzer.analyze(&program) { // Use the semantic error's position from the module file, not the load site let first_error = errors.first(); @@ -8679,6 +8726,15 @@ impl Interpreter { return Err(self.budget_error(exceeded, *line, *column)); } + // 3b. Include once per visible scope. The file's definitions + // already live in this scope (or one it can see), so running + // it again would only collide with them. This is what lets + // two files both `include from` the same shared file (a + // diamond) — the second arrival is a no-op. + if env.borrow().has_included(&resolved_path) { + return Ok((Value::Null, ControlFlow::None)); + } + // 4. Read file content under the shared source-size ceiling. let content = self .read_source_bounded(&resolved_path, *line, *column) @@ -8713,8 +8769,9 @@ impl Interpreter { // 6. Analyze semantics use crate::analyzer::Analyzer; - let parent_vars = Self::extract_parent_variables(&env); - let mut analyzer = Analyzer::with_parent_variables_mutable(parent_vars); + let parent_scope = Self::snapshot_parent_scope(&env); + let mut analyzer = Analyzer::with_parent_variables_mutable(parent_scope.variables); + analyzer.register_parent_actions(parent_scope.actions); if let Err(errors) = analyzer.analyze(&program) { let first_error = errors.first(); let (error_line, error_column) = @@ -8779,12 +8836,18 @@ impl Interpreter { .execute_block(&program.statements, Rc::clone(&env)) .await; - // 10. Handle result + // 10. Handle result. Only a file that ran to completion is + // recorded as included: a failed include (even one caught by + // `when error`) leaves the scope free to try it again. match result { - Ok((_, ControlFlow::None)) => Ok((Value::Null, ControlFlow::None)), + Ok((_, ControlFlow::None)) => { + env.borrow_mut().mark_included(resolved_path); + Ok((Value::Null, ControlFlow::None)) + } Ok((val, ControlFlow::Return(_))) => { // Return statements in included files are allowed and simply return the value // This enables utility functions in included files to use return statements + env.borrow_mut().mark_included(resolved_path); Ok((val, ControlFlow::None)) } Ok((_, ControlFlow::Break)) => Err(RuntimeError::new( From 6545314355bf6085a5cd45fa52f85f1e4f705ff1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:11:41 +0000 Subject: [PATCH 3/4] test: cover review findings on include-once bookkeeping Red evidence for four findings raised in review of the diamond-include change: a loop-body include must run again in each recycled iteration scope; a side-effect-only file must keep running on every include; an already-visible include must be a no-op even at the import-depth ceiling; and a `load module` file that redefines an outer action must be rejected at analysis time, before its earlier statements run. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018917NeaHViFzeZTQYcTNn5 --- tests/include_diamond_test.rs | 121 +++++++++++++++++++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/tests/include_diamond_test.rs b/tests/include_diamond_test.rs index 7b4e59ce..629b3504 100644 --- a/tests/include_diamond_test.rs +++ b/tests/include_diamond_test.rs @@ -6,19 +6,24 @@ use std::fs; use std::path::Path; +use std::sync::Arc; use tempfile::TempDir; +use wfl::exec::budget::{BudgetLimits, ExecutionBudget}; use wfl::interpreter::Interpreter; use wfl::interpreter::value::Value; use wfl::lexer::lex_wfl_with_positions; use wfl::parser::Parser; async fn run(main_file: &Path) -> Result { + run_with(main_file, Interpreter::new()).await +} + +async fn run_with(main_file: &Path, mut interpreter: Interpreter) -> Result { let source = fs::read_to_string(main_file).expect("read main"); let tokens = lex_wfl_with_positions(&source); let ast = Parser::new(&tokens) .parse() .unwrap_or_else(|e| panic!("Parse failed: {e:?}")); - let mut interpreter = Interpreter::new(); interpreter.set_source_file(main_file.to_path_buf()); match interpreter.interpret(&ast).await { Ok(_) => Ok(interpreter), @@ -196,3 +201,117 @@ async fn genuine_include_cycle_is_still_rejected() { "unexpected error: {err}" ); } + +#[tokio::test] +async fn include_in_a_loop_body_runs_again_in_each_recycled_iteration_scope() { + // Loop iteration scopes are recycled (cleared and reused). A file that + // installs only a plain variable leaves no weak references behind, so + // the recycled scope must forget the include along with the variable. + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("val.wfl"), "store loop_val as 7\n").unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +store total as 0 +count from 1 to 3: + include from "val.wfl" + change total to total plus loop_val +end count +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("loop include failed: {e}")); + assert_number(&interpreter, "total", 21.0); +} + +#[tokio::test] +async fn side_effect_only_file_still_runs_on_every_include() { + // Backward compatibility: a file that defines nothing has nothing for a + // later include to collide with, so it keeps running each time. + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("bump.wfl"), "change n to n plus 1\n").unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +store n as 0 +include from "bump.wfl" +include from "bump.wfl" +"#, + ) + .unwrap(); + let interpreter = run(&main) + .await + .unwrap_or_else(|e| panic!("repeated side-effect include failed: {e}")); + assert_number(&interpreter, "n", 2.0); +} + +#[tokio::test] +async fn already_visible_include_is_a_no_op_even_at_the_import_depth_ceiling() { + // With a ceiling of one nested file, `wrapper.wfl` may not enter another + // file. Its include of `util.wfl` is already satisfied by the top-level + // include, so it must be a no-op rather than a depth error. + let dir = TempDir::new().unwrap(); + write_diamond(dir.path()); + fs::write( + dir.path().join("wrapper.wfl"), + "include from \"util.wfl\"\nstore via_wrapper as shout of \"w\"\n", + ) + .unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + "include from \"util.wfl\"\ninclude from \"wrapper.wfl\"\n", + ) + .unwrap(); + let mut interpreter = Interpreter::new(); + interpreter.set_budget(Arc::new(ExecutionBudget::new(BudgetLimits { + max_import_depth: 1, + ..BudgetLimits::default() + }))); + let interpreter = run_with(&main, interpreter) + .await + .unwrap_or_else(|e| panic!("no-op include must not consume import depth: {e}")); + assert_text(&interpreter, "via_wrapper", "w!"); +} + +#[tokio::test] +async fn load_module_cannot_overload_an_outer_action_and_is_rejected_before_running() { + // The module runs in an isolated scope where the runtime rejects any + // same-name definition. The analyzer must reject it first, so the + // module's earlier statements (here: a mutation of `ran`) never run. + let dir = TempDir::new().unwrap(); + fs::write( + dir.path().join("mod.wfl"), + r#" +change ran to "yes" +define action called greet with parameters a and b: + give back a with b +end action +"#, + ) + .unwrap(); + let main = dir.path().join("main.wfl"); + fs::write( + &main, + r#" +store ran as "no" +define action called greet with parameters who: + give back "hi " with who +end action +load module from "mod.wfl" +"#, + ) + .unwrap(); + let err = run(&main) + .await + .err() + .expect("a module redefining an outer action must fail"); + assert!( + err.contains("Semantic error in module") && err.contains("outer scope"), + "expected an analysis-time rejection, got: {err}" + ); +} From 675b45d6e664ab1780d17d0a762e12ad5d0d5c66 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:11:41 +0000 Subject: [PATCH 4/4] fix: refine include-once bookkeeping from review findings - Environment::clear also clears the include record, so a recycled loop scope does not skip an include whose definitions it no longer has. - The already-visible no-op is decided before the import-depth ceiling is charged: a no-op never enters another file. - A file is recorded as included only when it installed at least one definition in the scope. A side-effect-only file keeps running on every include, so no existing program changes behavior; the docs and diary now describe the rule in terms of definitions. - `load module` analysis sees outer actions as callable but rejects a same-name definition up front (the isolated runtime scope would reject it anyway, after the module's earlier statements had run). - The four-file diamond example from the docs now lives under TestPrograms/docs_examples/modules/diamond/ and is registered in the manifest; the shared leaf runs all five layers, the three dependent files run parse, lint, and real execution (single-file analysis reports a cross-file action as a non-fatal warning with a nonzero exit). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018917NeaHViFzeZTQYcTNn5 --- Docs/04-advanced-features/modules.md | 46 ++++++++----- .../2026/2026-09-04-diamond-includes.md | 26 +++++--- .../docs_examples/_meta/manifest.json | 66 +++++++++++++++++++ .../docs_examples/modules/diamond/auth.wfl | 5 ++ .../docs_examples/modules/diamond/main.wfl | 5 ++ .../docs_examples/modules/diamond/render.wfl | 5 ++ .../docs_examples/modules/diamond/util.wfl | 5 ++ src/analyzer/mod.rs | 43 +++++++++++- src/interpreter/environment.rs | 3 + src/interpreter/mod.rs | 56 +++++++++++----- 10 files changed, 211 insertions(+), 49 deletions(-) create mode 100644 TestPrograms/docs_examples/modules/diamond/auth.wfl create mode 100644 TestPrograms/docs_examples/modules/diamond/main.wfl create mode 100644 TestPrograms/docs_examples/modules/diamond/render.wfl create mode 100644 TestPrograms/docs_examples/modules/diamond/util.wfl diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 80382fda..89d14fd7 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -114,24 +114,34 @@ display auth_check of "alice" # alice! display render_page of "home" # home! ``` -This works because an include runs **once per scope**. `auth.wfl` brings -`util.wfl` into the main program's scope. When `render.wfl` then asks for -`util.wfl` again, its definitions are already visible in that scope, so the -second `include from` does nothing — it does not run the file a second time, -and it does not raise an "already defined" error. Each file can therefore -honestly declare what it depends on, and the order in which the main program -includes its libraries does not matter. - -Two consequences worth knowing: - -- **Includes are for definitions, not for repeating side effects.** A file - included twice into the same scope runs once. If you want a file's - statements to run every time, use `load module from` instead. +This works because a file's **definitions are included once per scope**. +`auth.wfl` brings `util.wfl` into the main program's scope. When +`render.wfl` then asks for `util.wfl` again, its definitions are already +visible in that scope, so the second `include from` does nothing — it does +not run the file a second time, and it does not raise an "already defined" +error. Each file can therefore honestly declare what it depends on, and the +order in which the main program includes its libraries does not matter. + +Three details worth knowing: + +- **The rule is about definitions.** A file that defines actions, + containers, or variables is brought in once per scope. A file that + defines nothing — it only displays something, writes a file, or changes + variables that already exist — has nothing a later include could collide + with, so it runs every time it is included, exactly as it always has. + (`load module from` remains the clearest way to say "run this file for + its side effects".) - **"Same scope" includes scopes you can see.** An include inside an action body is skipped when the file was already included by an enclosing scope (its definitions are already reachable). A file included only inside an - action body runs again on each call, because each call starts with a - fresh local scope that has not seen it. + action body or a loop body runs again on each call or iteration, because + each one starts with a fresh local scope that has not seen it. +- **A failed include is not remembered.** If the included file stops with + an error, a later include of it runs it again. Anything it defined before + the error stays in the scope, as it did before. + +The four files above are kept under `TestPrograms/docs_examples/modules/diamond/` +and validated with the rest of the documentation examples. A genuine cycle — `a.wfl` includes `b.wfl`, which includes `a.wfl` — is still reported as a circular dependency (see @@ -680,8 +690,8 @@ export constant VERSION 3. **No Module Caching for `load module`** - Each `load module` re-parses and re-executes - Multiple loads of same file execute multiple times - - (`include from` is different: it runs a file once per scope, so - diamond includes are safe — see + - (`include from` is different: it brings a file's definitions in once + per scope, so diamond includes are safe — see [Including the same file more than once](#including-the-same-file-more-than-once-diamond-includes)) - Future: Optional caching planned @@ -915,7 +925,7 @@ load module from "expensive.wfl" # Uses cached version WFL's hybrid module system provides flexible code organization: - **`load module from "path.wfl"`** - Isolated execution for initialization and side effects -- **`include from "path.wfl"`** - Parent scope execution for shared libraries and containers; a file is included once per scope, so diamond includes are safe +- **`include from "path.wfl"`** - Parent scope execution for shared libraries and containers; a file's definitions are included once per scope, so diamond includes are safe - **`export container/action/constant NAME`** - Foundation for future namespace system - Paths resolve relative to the including file - Circular dependencies are automatically detected diff --git a/History/dev-diary/2026/2026-09-04-diamond-includes.md b/History/dev-diary/2026/2026-09-04-diamond-includes.md index c81e5a78..587a2e1b 100644 --- a/History/dev-diary/2026/2026-09-04-diamond-includes.md +++ b/History/dev-diary/2026/2026-09-04-diamond-includes.md @@ -55,19 +55,25 @@ Two separate defects, one hiding the other. file is treated as an overload under the existing distinctness rules, which matches what the runtime already did. - `Environment` records the canonical paths `include from` has completed in - that scope. An include whose file is already visible from the current - scope (recorded there or on an ancestor) is a no-op. Only a file that ran - to completion is recorded, so a failed include can be retried. `load - module` is unchanged: it exists to run a file for its side effects. - -Behavior that changes: a file included twice into the same scope used to -run twice if it contained only side effects (anything with a definition -already failed). It now runs once; `load module from` is the documented tool -for "run this file every time". + that scope **and that installed at least one definition there**. An + include whose file is already recorded for the current scope (or an + ancestor it can see) is a no-op, decided before the import-depth ceiling + is charged. Recycled loop scopes clear the record along with their + values. A file that defines nothing is never recorded, so a side-effect- + only file keeps running on every include — no existing program changes + behavior (anything with a definition already failed). A failed include is + not recorded either; what it defined before failing stays in the scope, + as it always did. +- `load module` still runs in an isolated child scope. Its analyzer now + sees outer actions as callable functions (so calls resolve) but rejects a + same-name definition up front — the runtime would reject it anyway, and + the rejection must land before the module's earlier statements run. ## Evidence -- Red: `tests/include_diamond_test.rs` (5 tests) and +- Red: `tests/include_diamond_test.rs` (9 tests, four added from review + findings: loop-scope recycling, side-effect-only re-include, the + import-depth boundary, and `load module` outer-action rejection) and `TestPrograms/modules/include_diamond.wfl` fail on the unmodified interpreter with the errors quoted above. - Green: same tests pass after the change; `cargo test --workspace`, diff --git a/TestPrograms/docs_examples/_meta/manifest.json b/TestPrograms/docs_examples/_meta/manifest.json index 06dadad0..c0c5b7b4 100644 --- a/TestPrograms/docs_examples/_meta/manifest.json +++ b/TestPrograms/docs_examples/_meta/manifest.json @@ -673,5 +673,71 @@ "properties" ], "doc_purpose": "Demonstrates reading instance properties with object.property" + }, + "docs_examples/modules/diamond/util.wfl": { + "doc_section": "Docs/04-advanced-features/modules.md", + "type": "executable", + "validate_layers": [ + 1, + 2, + 3, + 4, + 5 + ], + "expected_exit_code": 0, + "tags": [ + "modules", + "include", + "diamond" + ], + "doc_purpose": "Shared leaf file of the diamond include example" + }, + "docs_examples/modules/diamond/auth.wfl": { + "doc_section": "Docs/04-advanced-features/modules.md", + "type": "executable", + "validate_layers": [ + 1, + 4, + 5 + ], + "expected_exit_code": 0, + "tags": [ + "modules", + "include", + "diamond" + ], + "doc_purpose": "First branch of the diamond include example. Layers 2-3 are skipped: it calls an action that lives in a file it includes, and single-file analysis reports that as a non-fatal warning with a nonzero exit; layer 5 runs the real multi-file program." + }, + "docs_examples/modules/diamond/render.wfl": { + "doc_section": "Docs/04-advanced-features/modules.md", + "type": "executable", + "validate_layers": [ + 1, + 4, + 5 + ], + "expected_exit_code": 0, + "tags": [ + "modules", + "include", + "diamond" + ], + "doc_purpose": "Second branch of the diamond include example. Layers 2-3 are skipped: it calls an action that lives in a file it includes, and single-file analysis reports that as a non-fatal warning with a nonzero exit; layer 5 runs the real multi-file program." + }, + "docs_examples/modules/diamond/main.wfl": { + "doc_section": "Docs/04-advanced-features/modules.md", + "type": "executable", + "validate_layers": [ + 1, + 4, + 5 + ], + "expected_exit_code": 0, + "tags": [ + "modules", + "include", + "diamond" + ], + "doc_purpose": "Top of the diamond include example: both branches include util.wfl once. Layers 2-3 are skipped: it calls an action that lives in a file it includes, and single-file analysis reports that as a non-fatal warning with a nonzero exit; layer 5 runs the real multi-file program." } } diff --git a/TestPrograms/docs_examples/modules/diamond/auth.wfl b/TestPrograms/docs_examples/modules/diamond/auth.wfl new file mode 100644 index 00000000..a868cebb --- /dev/null +++ b/TestPrograms/docs_examples/modules/diamond/auth.wfl @@ -0,0 +1,5 @@ +// One branch of the diamond: depends on util.wfl directly. +include from "util.wfl" +define action called auth_check with parameters who: + give back shout of who +end action diff --git a/TestPrograms/docs_examples/modules/diamond/main.wfl b/TestPrograms/docs_examples/modules/diamond/main.wfl new file mode 100644 index 00000000..b461e3b1 --- /dev/null +++ b/TestPrograms/docs_examples/modules/diamond/main.wfl @@ -0,0 +1,5 @@ +// Top of the diamond: both branches bring in util.wfl; it is included once. +include from "auth.wfl" +include from "render.wfl" +display auth_check of "alice" # alice! +display render_page of "home" # home! diff --git a/TestPrograms/docs_examples/modules/diamond/render.wfl b/TestPrograms/docs_examples/modules/diamond/render.wfl new file mode 100644 index 00000000..c16b5647 --- /dev/null +++ b/TestPrograms/docs_examples/modules/diamond/render.wfl @@ -0,0 +1,5 @@ +// The other branch of the diamond: also depends on util.wfl directly. +include from "util.wfl" +define action called render_page with parameters title: + give back shout of title +end action diff --git a/TestPrograms/docs_examples/modules/diamond/util.wfl b/TestPrograms/docs_examples/modules/diamond/util.wfl new file mode 100644 index 00000000..689012bb --- /dev/null +++ b/TestPrograms/docs_examples/modules/diamond/util.wfl @@ -0,0 +1,5 @@ +// Shared leaf of the include diamond documented in +// Docs/04-advanced-features/modules.md ("Including the same file more than once"). +define action called shout with parameters msg: + give back msg with "!" +end action diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index 9f08a557..322e67dc 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -364,6 +364,12 @@ pub struct Analyzer { /// before every independent program so prior declarations and diagnostics /// cannot leak across runs. baseline_symbols: HashMap, + /// Actions seeded from an enclosing runtime scope that the analyzed file + /// may call but must not redefine or overload: a `load module` file runs + /// in an isolated child scope whose runtime `define` rejects shadowing a + /// parent binding, so the analyzer rejects it first — before any of the + /// module's side effects run. + outer_actions: HashSet, /// Monotone lexical-scope identity source. Scope IDs remain stable through /// snapshots/clones and are never reused within one analyzer run. next_scope_id: u64, @@ -665,6 +671,7 @@ impl Analyzer { Analyzer { current_scope: global_scope, baseline_symbols, + outer_actions: HashSet::new(), next_scope_id: 1, errors: Vec::new(), warnings: Vec::new(), @@ -739,10 +746,23 @@ impl Analyzer { /// variables would make every call to them a fatal "is not a function" /// error in the file being analyzed, which is what broke diamond and /// sibling includes. Like the other parent-scope symbols these carry - /// position 0:0; a same-name definition in the analyzed file is treated - /// as an overload under the usual distinctness rules. - pub fn register_parent_actions(&mut self, actions: Vec<(String, Vec)>) { + /// position 0:0. + /// + /// `mergeable` mirrors the runtime binding rule of the file being + /// analyzed: an `include from` file runs in the enclosing scope itself, + /// so a same-name definition there is an overload under the usual + /// distinctness rules; a `load module` file runs in an isolated child + /// scope where the runtime rejects any same-name definition, so the + /// analyzer rejects it too (see [`Self::outer_actions`]). + pub fn register_parent_actions( + &mut self, + actions: Vec<(String, Vec)>, + mergeable: bool, + ) { for (name, signatures) in actions { + if !mergeable { + self.outer_actions.insert(name.clone()); + } let param_types = signatures .first() .map(|sig| { @@ -3149,6 +3169,23 @@ impl Analyzer { return_type: return_type.clone(), }; + // An action seeded from the loading file's scope cannot be + // redefined or overloaded by an isolated module (the runtime + // would reject the shadowing after the module's earlier + // statements had already run). + if self.outer_actions.contains(name) { + self.errors.push(SemanticError::new( + format!( + "Action '{name}' has already been defined in an outer scope. \ + A file run with 'load module' cannot redefine or overload an action \ + from the file that loaded it; use a different name." + ), + *line, + *column, + )); + return; + } + // Same-scope redefinition of an existing action is the overload // path: accumulate the signature instead of erroring, unless the // pair is an exact duplicate or cannot be told apart at a call diff --git a/src/interpreter/environment.rs b/src/interpreter/environment.rs index 8c73e4ce..40456f02 100644 --- a/src/interpreter/environment.rs +++ b/src/interpreter/environment.rs @@ -384,6 +384,9 @@ impl Environment { pub fn clear(&mut self) { self.values.clear(); self.constants.clear(); + // A recycled loop scope starts logically fresh: the definitions an + // include installed are gone, so its marker must go with them. + self.included_files.clear(); // Parent, isolated status, and other flags remain unchanged } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 76e3e18b..b8455700 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -8583,7 +8583,9 @@ impl Interpreter { // Extract parent variables from current environment for module analyzer let parent_scope = Self::snapshot_parent_scope(&env); let mut analyzer = Analyzer::with_parent_variables(parent_scope.variables); - analyzer.register_parent_actions(parent_scope.actions); + // The module runs in an isolated child scope: outer actions are + // callable but a same-name definition is rejected, as at runtime. + analyzer.register_parent_actions(parent_scope.actions, false); if let Err(errors) = analyzer.analyze(&program) { // Use the semantic error's position from the module file, not the load site let first_error = errors.first(); @@ -8716,9 +8718,22 @@ impl Interpreter { // 2. Resolve absolute path let resolved_path = self.resolve_module_path(&path_str, *line, *column).await?; - // 3. Check circular dependencies and the shared import-depth - // ceiling (loading_stack length is the depth already entered). + // 3. Reject a genuine cycle (the file is still being loaded). self.check_circular_dependency(&resolved_path, *line, *column)?; + + // 3b. Include definitions once per visible scope. When the + // file's definitions already live in this scope (or one it + // can see), running it again would only collide with them, + // so the include is a no-op. This is what lets two files + // both `include from` the same shared file (a diamond). It + // is decided before the depth ceiling because a no-op never + // enters another file. + if env.borrow().has_included(&resolved_path) { + return Ok((Value::Null, ControlFlow::None)); + } + + // 3c. Shared import-depth ceiling (loading_stack length is the + // depth already entered). if let Err(exceeded) = self .budget .check_import_depth(self.loading_stack.borrow().len()) @@ -8726,15 +8741,6 @@ impl Interpreter { return Err(self.budget_error(exceeded, *line, *column)); } - // 3b. Include once per visible scope. The file's definitions - // already live in this scope (or one it can see), so running - // it again would only collide with them. This is what lets - // two files both `include from` the same shared file (a - // diamond) — the second arrival is a no-op. - if env.borrow().has_included(&resolved_path) { - return Ok((Value::Null, ControlFlow::None)); - } - // 4. Read file content under the shared source-size ceiling. let content = self .read_source_bounded(&resolved_path, *line, *column) @@ -8771,7 +8777,9 @@ impl Interpreter { let parent_scope = Self::snapshot_parent_scope(&env); let mut analyzer = Analyzer::with_parent_variables_mutable(parent_scope.variables); - analyzer.register_parent_actions(parent_scope.actions); + // The file runs in this very scope, so a same-name definition + // merges into the overload set exactly as the runtime does. + analyzer.register_parent_actions(parent_scope.actions, true); if let Err(errors) = analyzer.analyze(&program) { let first_error = errors.first(); let (error_line, error_column) = @@ -8832,22 +8840,34 @@ impl Interpreter { // 9. Execute included file in PARENT scope (key difference from load module) // This allows containers/variables to be exposed to parent + let bindings_before = env.borrow().values.len(); let result = self .execute_block(&program.statements, Rc::clone(&env)) .await; - // 10. Handle result. Only a file that ran to completion is - // recorded as included: a failed include (even one caught by - // `when error`) leaves the scope free to try it again. + // 10. Handle result. A file is recorded as included only when + // it ran to completion AND installed at least one definition + // in this scope: that is what a later include of it would + // collide with. A file that defines nothing (side effects + // only, or mutations of existing variables) keeps running on + // every include exactly as it always has. A failed include is + // never recorded; note that whatever it defined before failing + // stays in the scope, as it did before include-once existed. + let record_if_defined = |env: &Rc>| { + let mut env_mut = env.borrow_mut(); + if env_mut.values.len() > bindings_before { + env_mut.mark_included(resolved_path.clone()); + } + }; match result { Ok((_, ControlFlow::None)) => { - env.borrow_mut().mark_included(resolved_path); + record_if_defined(&env); Ok((Value::Null, ControlFlow::None)) } Ok((val, ControlFlow::Return(_))) => { // Return statements in included files are allowed and simply return the value // This enables utility functions in included files to use return statements - env.borrow_mut().mark_included(resolved_path); + record_if_defined(&env); Ok((val, ControlFlow::None)) } Ok((_, ControlFlow::Break)) => Err(RuntimeError::new(