From 65504dec8f542e56cc523710f70dfb746398ad8e Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 23:01:09 +0900 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix(lang):=20reject=20duplic?= =?UTF-8?q?ate=20def=20and=20conflicting=20members=20across=20module=20dec?= =?UTF-8?q?larations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module now errors when it binds the same name through `def`, or through a `def` and a `let`/`var`. Redeclaring `module m` keeps extending the existing module, but a member already bound by an earlier declaration in the same scope is rejected instead of silently replaced. Document the rule in modules_and_imports.md. --- crates/mq-lang/src/tarn/compiler.rs | 43 ++++++++++++++++++- crates/mq-lang/src/tarn/tests.rs | 27 ++++++++++++ .../src/reference/modules_and_imports.md | 8 ++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/crates/mq-lang/src/tarn/compiler.rs b/crates/mq-lang/src/tarn/compiler.rs index 127d8e5e1..2f9007e85 100644 --- a/crates/mq-lang/src/tarn/compiler.rs +++ b/crates/mq-lang/src/tarn/compiler.rs @@ -26,7 +26,7 @@ pub(crate) enum CompileError { Unsupported(&'static str, TokenId), Module(ModuleError), AssignToImmutable(String, TokenId), - /// A module binds the same `let`/`var` name more than once. + /// A module binds the same `let`/`var`/`def` name more than once. DuplicateModuleBinding(String, TokenId), InvalidBytecode(String), /// `yield` outside any `def`/`fn` body. @@ -1166,6 +1166,7 @@ impl Compiler { } let mut deferred = Vec::with_capacity(program.len()); let mut defs: Program = Vec::new(); + Self::check_module_extensions(program)?; for node in program { self.current_token_id = node.token_id; match &node.expr { @@ -2259,11 +2260,18 @@ impl Compiler { } self.push_module_barrier(depth, module_path[0]); + Self::check_module_extensions(program)?; let mut def_slots = FxHashMap::default(); let mut let_slots = FxHashMap::default(); for node in program { match &node.expr { Expr::Def(def_ident, _, _) => { + if def_slots.contains_key(&def_ident.name) || let_slots.contains_key(&def_ident.name) { + return Err(CompileError::DuplicateModuleBinding( + def_ident.name.to_string(), + node.token_id, + )); + } let slot = self.scope_mut().declare(def_ident.name); self.scope_mut().mark_immutable(slot); def_slots.insert(def_ident.name, slot); @@ -2274,7 +2282,7 @@ impl Compiler { names.sort(); names.dedup(); for name in names { - if let_slots.contains_key(&name) { + if let_slots.contains_key(&name) || def_slots.contains_key(&name) { return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); } let slot = self.scope_mut().declare(name); @@ -2352,6 +2360,37 @@ impl Compiler { Ok((rest, let_slots, barrier)) } + /// Rejects a member bound by more than one declaration of the same inline module in `program`. + fn check_module_extensions(program: &Program) -> CompileResult<()> { + let mut seen: FxHashMap> = FxHashMap::default(); + for node in program { + let Expr::Module(ident, body) = &node.expr else { + continue; + }; + let mut names = Vec::new(); + for member in body { + match &member.expr { + Expr::Def(def_ident, _, _) => names.push(def_ident.name), + Expr::Let(pattern, _) | Expr::Var(pattern, _) => { + let mut pattern_names = Vec::new(); + collect_pattern_idents(pattern, &mut pattern_names); + pattern_names.sort(); + pattern_names.dedup(); + names.extend(pattern_names); + } + _ => {} + } + } + let bound = seen.entry(ident.name).or_default(); + for name in names { + if !bound.insert(name) { + return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); + } + } + } + Ok(()) + } + /// Opens a module whose members are declared from here on. fn push_module_barrier(&mut self, depth: usize, root: Ident) { self.module_barriers.push(ModuleBarrier { diff --git a/crates/mq-lang/src/tarn/tests.rs b/crates/mq-lang/src/tarn/tests.rs index a59e13b66..e68722567 100644 --- a/crates/mq-lang/src/tarn/tests.rs +++ b/crates/mq-lang/src/tarn/tests.rs @@ -4774,6 +4774,33 @@ fn let_rebinding_does_not_change_existing_closures(#[case] code: &str, #[case] e assert_eq!(run(code), expected); } +#[rstest] +#[case::def_def("module m: def a(): 1; def a(): 2; end | m::a()", "a")] +#[case::def_then_let("module m: def a(): 1; let a = 2 end | m::a", "a")] +#[case::let_then_def("module m: let a = 2 | def a(): 1; end | m::a", "a")] +fn module_rejects_binding_the_same_name_twice_across_kinds(#[case] code: &str, #[case] name: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(n, _))) if n == name), + "{result:?}" + ); +} + +#[rstest] +#[case::def_def("module m: def a(): 1; end | module m: def a(): 2; end | m::a()")] +#[case::let_def("module m: let a = 1 end | module m: def a(): 2; end | m::a")] +fn module_extension_rejects_existing_member(#[case] code: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(name, _))) if name == "a"), + "{result:?}" + ); +} + #[test] fn module_rejects_binding_the_same_name_twice() { let token_arena = Shared::new(SharedCell::new(Arena::new(16))); diff --git a/docs/books/src/reference/modules_and_imports.md b/docs/books/src/reference/modules_and_imports.md index 3ab5b21a7..471395c26 100644 --- a/docs/books/src/reference/modules_and_imports.md +++ b/docs/books/src/reference/modules_and_imports.md @@ -55,6 +55,14 @@ end Module-level `let` values are computed once per run. +A module may bind each name only once across its `let`, `var` and `def` declarations. Declaring `module m` again extends the existing module with the new members. A member name already bound by an earlier declaration is a compile error: + +```mq +module m: def a(): 1; def a(): 2; end # error: module binds "a" more than once +module m: def a(): 1; end | module m: def b(): 2; end # ok: m has a and b +module m: def a(): 1; end | module m: def a(): 2; end # error: module binds "a" more than once +``` + ## Import Loads a module from an external file using the syntax `import "module_path"`. From 08a2721dc3d92e08ab181e19a822bf845392c454 Mon Sep 17 00:00:00 2001 From: harehare Date: Wed, 28 Oct 2026 09:54:24 +0900 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20fix(lang):=20reject=20duplic?= =?UTF-8?q?ate=20members=20in=20nested,=20scoped=20and=20imported=20module?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sibling-extension check now tracks full qualified member paths and descends into nested module bodies, so `outer::inner::a` declared across two `outer` extensions is rejected instead of silently replaced. The check also runs for function and block bodies and for the directive list of an imported or included file. An imported file that binds the same name through `def` or `let` more than once is rejected before its functions are compiled. --- crates/mq-lang/src/tarn/compiler.rs | 66 ++++++++++++++++++++--------- crates/mq-lang/src/tarn/tests.rs | 37 ++++++++++++++++ 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/crates/mq-lang/src/tarn/compiler.rs b/crates/mq-lang/src/tarn/compiler.rs index 2f9007e85..fb81dad2e 100644 --- a/crates/mq-lang/src/tarn/compiler.rs +++ b/crates/mq-lang/src/tarn/compiler.rs @@ -1354,6 +1354,7 @@ impl Compiler { } fn compile_body(&mut self, body: &Program) -> CompileResult<()> { + Self::check_module_extensions(body)?; for node in body { if let Expr::Def(ident, _, _) = &node.expr { self.scope_mut().declare(ident.name); @@ -2057,7 +2058,8 @@ impl Compiler { fn compile_module_directives(&mut self, modules: &Program, parent_module_path: &[Ident]) -> CompileResult<()> { let previously_pruning = std::mem::replace(&mut self.prune_module_functions, false); - let result = self.compile_discarding_in_module_path(modules, parent_module_path); + let result = Self::check_module_extensions(modules) + .and_then(|()| self.compile_discarding_in_module_path(modules, parent_module_path)); self.prune_module_functions = previously_pruning; result } @@ -2184,6 +2186,7 @@ impl Compiler { )); }; let module = self.load_module_or_reload(path)?; + Self::check_imported_module(&module)?; let module_alias = alias.map(|a| a.name).unwrap_or_else(|| Ident::new(&module.name)); let depth = self.scopes.len() - 1; self.push_module_barrier(depth, module_alias); @@ -2360,37 +2363,62 @@ impl Compiler { Ok((rest, let_slots, barrier)) } - /// Rejects a member bound by more than one declaration of the same inline module in `program`. + /// Rejects a member bound by more than one declaration of the same inline module in `program`, + /// descending into nested modules. fn check_module_extensions(program: &Program) -> CompileResult<()> { - let mut seen: FxHashMap> = FxHashMap::default(); + Self::check_module_members(program, &mut Vec::new(), &mut FxHashMap::default()) + } + + /// Rejects an imported file that binds a name more than once, or whose nested modules do. + fn check_imported_module(module: &Module) -> CompileResult<()> { + let mut bound = FxHashSet::default(); + for node in module.functions.iter().chain(&module.vars) { + for name in Self::bound_names(node) { + if !bound.insert(name) { + return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); + } + } + } + Self::check_module_extensions(&module.modules) + } + + fn check_module_members( + program: &Program, + path: &mut Vec, + seen: &mut FxHashMap, FxHashSet>, + ) -> CompileResult<()> { for node in program { let Expr::Module(ident, body) = &node.expr else { continue; }; - let mut names = Vec::new(); - for member in body { - match &member.expr { - Expr::Def(def_ident, _, _) => names.push(def_ident.name), - Expr::Let(pattern, _) | Expr::Var(pattern, _) => { - let mut pattern_names = Vec::new(); - collect_pattern_idents(pattern, &mut pattern_names); - pattern_names.sort(); - pattern_names.dedup(); - names.extend(pattern_names); - } - _ => {} - } - } - let bound = seen.entry(ident.name).or_default(); - for name in names { + path.push(ident.name); + let bound = seen.entry(path.clone()).or_default(); + for name in body.iter().flat_map(|member| Self::bound_names(member)) { if !bound.insert(name) { return Err(CompileError::DuplicateModuleBinding(name.to_string(), node.token_id)); } } + Self::check_module_members(body, path, seen)?; + path.pop(); } Ok(()) } + /// Names a module member binds directly. + fn bound_names(member: &Node) -> Vec { + match &member.expr { + Expr::Def(ident, _, _) => vec![ident.name], + Expr::Let(pattern, _) | Expr::Var(pattern, _) => { + let mut names = Vec::new(); + collect_pattern_idents(pattern, &mut names); + names.sort(); + names.dedup(); + names + } + _ => Vec::new(), + } + } + /// Opens a module whose members are declared from here on. fn push_module_barrier(&mut self, depth: usize, root: Ident) { self.module_barriers.push(ModuleBarrier { diff --git a/crates/mq-lang/src/tarn/tests.rs b/crates/mq-lang/src/tarn/tests.rs index e68722567..11aeb90be 100644 --- a/crates/mq-lang/src/tarn/tests.rs +++ b/crates/mq-lang/src/tarn/tests.rs @@ -4801,6 +4801,43 @@ fn module_extension_rejects_existing_member(#[case] code: &str) { ); } +#[rstest] +#[case::nested( + "module outer: module inner: def a(): 1; end end | module outer: module inner: def a(): 2; end end | outer::inner::a()" +)] +#[case::in_function("def f(): module m: def a(): 1; end | module m: def a(): 2; end | m::a(); | f()")] +fn module_extension_rejects_duplicate_member_in_nested_scope(#[case] code: &str) { + let token_arena = Shared::new(SharedCell::new(Arena::new(16))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let result = compile_and_run(&program, token_arena); + assert!( + matches!(&result, Err(Error::Compile(compiler::CompileError::DuplicateModuleBinding(name, _))) if name == "a"), + "{result:?}" + ); +} + +#[rstest] +#[case::def_def("def a(): 1; def a(): 2;", r#"import "lib" | lib::a()"#)] +#[case::def_let("def a(): 1; let a = 2", r#"import "lib" | lib::a"#)] +#[case::inline_module( + "module m: def a(): 1; end | module m: def a(): 2; end", + r#"import "lib" | lib::m::a()"# +)] +fn import_rejects_duplicate_binding(#[case] lib: &str, #[case] code: &str) { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::write(dir.path().join("lib.mq"), lib).unwrap(); + let token_arena = Shared::new(SharedCell::new(Arena::new(100))); + let program = crate::parse(code, Shared::clone(&token_arena)).unwrap(); + let resolver = + crate::module::resolver::local_fs_resolver::LocalFsModuleResolver::new(Some(vec![dir.path().to_path_buf()])); + let result = compiler::compile_program(&program, token_arena, ModuleLoader::new(resolver)); + assert!( + matches!(&result, Err(compiler::CompileError::DuplicateModuleBinding(name, _)) if name == "a"), + "{:?}", + result.err() + ); +} + #[test] fn module_rejects_binding_the_same_name_twice() { let token_arena = Shared::new(SharedCell::new(Arena::new(16)));