From 7b0bcdfdd0680965f94191e56b9274499d972dcc Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 9 Sep 2026 14:41:12 +0200 Subject: [PATCH 01/19] feat: symbol table --- ndc_analyser/src/lib.rs | 1 + ndc_analyser/src/symbols.rs | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 ndc_analyser/src/symbols.rs diff --git a/ndc_analyser/src/lib.rs b/ndc_analyser/src/lib.rs index 133ebf62..1b24c638 100644 --- a/ndc_analyser/src/lib.rs +++ b/ndc_analyser/src/lib.rs @@ -1,5 +1,6 @@ mod analyser; mod scope; +mod symbols; pub use analyser::{Analyser, AnalysisError, AnalysisResult}; pub use scope::ScopeTree; diff --git a/ndc_analyser/src/symbols.rs b/ndc_analyser/src/symbols.rs new file mode 100644 index 00000000..fc9bba09 --- /dev/null +++ b/ndc_analyser/src/symbols.rs @@ -0,0 +1,43 @@ +#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)] +pub(crate) struct SymbolId(usize); + +#[derive(Debug, Default)] +pub(crate) struct SymbolTable { + names: Vec, +} + +impl SymbolTable { + pub(crate) fn declare(&mut self, name: String) -> SymbolId { + self.names.push(name); + SymbolId(self.names.len() - 1) + } + + pub(crate) fn get(&self, id: SymbolId) -> &str { + &self.names[id.0] + } +} + +#[cfg(test)] +mod test { + use crate::symbols::SymbolTable; + + #[test] + fn it_tracks_multiple_identifiers() { + let mut table = SymbolTable::default(); + let a = table.declare("foo".to_string()); + let b = table.declare("bar".to_string()); + + assert_ne!(a, b); + assert_eq!(table.get(a), "foo"); + assert_eq!(table.get(b), "bar"); + } + + #[test] + fn identifiers_with_the_same_name_are_not_equal() { + let mut table = SymbolTable::default(); + let a = table.declare("foo".to_string()); + let b = table.declare("foo".to_string()); + + assert_ne!(a, b); + } +} From 8a52d1d73fe8f654c541418861259ec18bc8a442 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 9 Sep 2026 15:19:35 +0200 Subject: [PATCH 02/19] feat: scope stack v2 --- ndc_analyser/src/lib.rs | 1 + ndc_analyser/src/name_resolution.rs | 187 ++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 ndc_analyser/src/name_resolution.rs diff --git a/ndc_analyser/src/lib.rs b/ndc_analyser/src/lib.rs index 1b24c638..cd1a5e08 100644 --- a/ndc_analyser/src/lib.rs +++ b/ndc_analyser/src/lib.rs @@ -1,4 +1,5 @@ mod analyser; +mod name_resolution; mod scope; mod symbols; diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs new file mode 100644 index 00000000..85898b17 --- /dev/null +++ b/ndc_analyser/src/name_resolution.rs @@ -0,0 +1,187 @@ +use crate::symbols::{SymbolId, SymbolTable}; + +#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] +pub(crate) struct ScopeId(usize); +#[derive(Debug, Default)] +pub(crate) struct Scope { + parent: Option, + bindings: Vec, +} + +#[derive(Debug)] +pub(crate) struct ScopeTree { + current: ScopeId, + scopes: Vec, +} + +impl Scope { + pub(crate) fn new_with_parent(parent: ScopeId) -> Self { + Self { + parent: Some(parent), + bindings: Vec::default(), + } + } + pub(crate) fn bind(&mut self, symbol_id: SymbolId) { + self.bindings.push(symbol_id); + } + + pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { + for id in self.bindings.iter().rev() { + if table.get(*id) == name { + return Some(*id); + } + } + None + } +} + +impl ScopeTree { + pub(crate) fn new() -> Self { + Self { + current: ScopeId(0), + scopes: vec![Scope::default()], + } + } + + pub(crate) fn bind(&mut self, symbol_id: SymbolId) { + self.scopes[self.current.0].bind(symbol_id); + } + + pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { + let mut current_scope = &self.scopes[self.current.0]; + loop { + if let Some(id) = current_scope.lookup(name, table) { + return Some(id); + } + + current_scope = if let Some(parent) = current_scope.parent { + &self.scopes[parent.0] + } else { + return None; + } + } + } + + pub(crate) fn enter_scope(&mut self) { + self.scopes.push(Scope::new_with_parent(self.current)); + self.current = ScopeId(self.scopes.len() - 1); + } + + pub(crate) fn leave_scope(&mut self) { + self.current = self.scopes[self.current.0] + .parent + .expect("internal error: scope stack is empty"); + } +} +#[cfg(test)] +mod test { + use crate::name_resolution::Scope; + use crate::name_resolution::ScopeTree; + use crate::symbols::SymbolTable; + + #[test] + fn binding_follows_shadowing_rules() { + let mut table = SymbolTable::default(); + let first = table.declare("foo".to_string()); + let second = table.declare("foo".to_string()); + let other = table.declare("another".to_string()); + + let mut scope = Scope::default(); + scope.bind(first); + scope.bind(second); + scope.bind(other); + + // The second binding shadows the first + assert_eq!(scope.lookup("foo", &table), Some(second)); + + // Invalid bindings return None + assert_eq!(scope.lookup("nope", &table), None); + + // Bindings don't interfere + assert_eq!(scope.lookup("another", &table), Some(other)); + } + + #[test] + fn scope_tree_traverses() { + let mut table = SymbolTable::default(); + let first = table.declare("foo".to_string()); + let middle = table.declare("bar".to_string()); + + let mut tree = ScopeTree::new(); + tree.bind(first); + tree.enter_scope(); + tree.bind(middle); + tree.enter_scope(); + assert_eq!(tree.lookup("foo", &table), Some(first)); + assert_eq!(tree.lookup("bar", &table), Some(middle)); + assert_eq!(tree.lookup("missing", &table), None); + + tree.leave_scope(); + assert_eq!(tree.lookup("bar", &table), Some(middle)); + tree.leave_scope(); + assert_eq!(tree.lookup("foo", &table), Some(first)); + assert_eq!(tree.lookup("bar", &table), None); + } + + #[test] + fn scope_tree_shadows() { + let mut table = SymbolTable::default(); + let first = table.declare("foo".to_string()); + let second = table.declare("foo".to_string()); + + let mut tree = ScopeTree::new(); + tree.bind(first); + tree.enter_scope(); + tree.bind(second); + assert_eq!(tree.lookup("foo", &table), Some(second)); + + tree.leave_scope(); + assert_eq!(tree.lookup("foo", &table), Some(first)); + } + + #[test] + fn sibling_scopes_do_not_share_bindings() { + let mut table = SymbolTable::default(); + let first = table.declare("first".to_string()); + let second = table.declare("second".to_string()); + + let mut tree = ScopeTree::new(); + tree.enter_scope(); + tree.bind(first); + tree.leave_scope(); + assert_eq!(tree.lookup("first", &table), None); + + tree.enter_scope(); + assert_eq!(tree.lookup("first", &table), None); + tree.bind(second); + assert_eq!(tree.lookup("second", &table), Some(second)); + tree.leave_scope(); + assert_eq!(tree.lookup("first", &table), None); + assert_eq!(tree.lookup("second", &table), None); + } + + #[test] + fn lookup_does_not_change_where_new_bindings_are_created() { + let mut table = SymbolTable::default(); + let outer = table.declare("outer".to_string()); + let inner = table.declare("inner".to_string()); + + let mut tree = ScopeTree::new(); + tree.bind(outer); + tree.enter_scope(); + assert_eq!(tree.lookup("outer", &table), Some(outer)); + assert_eq!(tree.lookup("missing", &table), None); + + tree.bind(inner); + assert_eq!(tree.lookup("inner", &table), Some(inner)); + tree.leave_scope(); + assert_eq!(tree.lookup("inner", &table), None); + assert_eq!(tree.lookup("outer", &table), Some(outer)); + } + + #[test] + #[should_panic(expected = "internal error: scope stack is empty")] + fn leaving_root_scope_is_rejected() { + ScopeTree::new().leave_scope(); + } +} From 3b5e444ccd4afebd3ff30137847aedafd59e2e7e Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Thu, 10 Sep 2026 08:43:30 +0200 Subject: [PATCH 03/19] feat: name resolver v1 --- Cargo.lock | 1 + ndc_analyser/Cargo.toml | 1 + ndc_analyser/src/name_resolution.rs | 266 ++++++++++++++- .../src/name_resolution/resolver_tests.rs | 302 ++++++++++++++++++ 4 files changed, 569 insertions(+), 1 deletion(-) create mode 100644 ndc_analyser/src/name_resolution/resolver_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 55f81929..a2f2af9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1137,6 +1137,7 @@ dependencies = [ name = "ndc_analyser" version = "0.3.0" dependencies = [ + "ahash", "itertools 0.15.0", "ndc_core", "ndc_lexer", diff --git a/ndc_analyser/Cargo.toml b/ndc_analyser/Cargo.toml index 48c56f64..d29d34fe 100644 --- a/ndc_analyser/Cargo.toml +++ b/ndc_analyser/Cargo.toml @@ -9,3 +9,4 @@ ndc_core.workspace = true ndc_lexer.workspace = true ndc_parser.workspace = true thiserror.workspace = true +ahash.workspace = true diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 85898b17..b2031bdc 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -1,19 +1,41 @@ use crate::symbols::{SymbolId, SymbolTable}; +use ahash::AHashMap; +use ndc_lexer::Span; +use ndc_parser::{Expression, ExpressionLocation, ForBody, ForIteration, Lvalue, NodeId}; #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] pub(crate) struct ScopeId(usize); + #[derive(Debug, Default)] pub(crate) struct Scope { parent: Option, bindings: Vec, } -#[derive(Debug)] pub(crate) struct ScopeTree { current: ScopeId, scopes: Vec, } +pub(crate) struct NameResolver { + symbols: SymbolTable, + scopes: ScopeTree, + references: AHashMap, + // Lvalues have source spans but no NodeId. Keep their declaration/write + // bindings separately so later passes do not have to resolve them again. + lvalue_bindings: Vec<(Span, SymbolId)>, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum NameResolutionError { + #[error("identifier '{name}' has not been declared")] + UnknownName { name: String, span: Span }, + #[error("index and member targets cannot declare names")] + InvalidDeclaration { span: Span }, + #[error("name resolution for {feature} is not implemented yet")] + Unsupported { feature: &'static str, span: Span }, +} + impl Scope { pub(crate) fn new_with_parent(parent: ScopeId) -> Self { Self { @@ -73,6 +95,248 @@ impl ScopeTree { .expect("internal error: scope stack is empty"); } } + +impl NameResolver { + pub(crate) fn new() -> Self { + Self { + symbols: Default::default(), + scopes: ScopeTree::new(), + references: Default::default(), + lvalue_bindings: Default::default(), + } + } + + /// Resolve lexical value names. Type annotations and member selection + /// belong to later passes; this does not validate types or control flow. + pub(crate) fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { + match &loc.expression { + Expression::BoolLiteral(_) + | Expression::StringLiteral(_) + | Expression::NumericLiteral(_) + | Expression::Break + | Expression::Continue => {} + Expression::Identifier { name, .. } => { + let symbol = self.lookup(name, loc.span)?; + self.references.insert(loc.id, symbol); + } + Expression::Statement(inner) + | Expression::Grouping(inner) + | Expression::Cast { value: inner, .. } + | Expression::Return { value: inner } => self.resolve(inner)?, + Expression::Logical { left, right, .. } => { + self.resolve(left)?; + self.resolve(right)?; + } + Expression::VariableDeclaration { l_value, value, .. } => { + self.resolve(value)?; + self.declare_pattern(l_value, loc.span)?; + } + Expression::Assignment { l_value, r_value } => { + self.resolve_target(l_value)?; + self.resolve(r_value)?; + } + Expression::Block { statements } => { + self.in_scope(|resolver| { + for statement in statements { + resolver.resolve(statement)?; + } + Ok(()) + })?; + } + Expression::If { + condition, + on_true, + on_false, + } => { + self.resolve(condition)?; + self.resolve(on_true)?; + if let Some(on_false) = on_false { + self.resolve(on_false)?; + } + } + Expression::While { + expression, + loop_body, + } => { + self.resolve(expression)?; + self.resolve(loop_body)?; + } + Expression::For { iterations, body } => self.resolve_for(iterations, body, loc.span)?, + Expression::Call { + function, + arguments, + } + | Expression::OperatorCall { + function, + arguments, + } => { + self.resolve(function)?; + for argument in arguments { + self.resolve(argument)?; + } + } + Expression::MemberAccess { receiver, .. } => self.resolve(receiver)?, + Expression::Tuple { values } | Expression::List { values } => { + for value in values { + self.resolve(value)?; + } + } + Expression::Map { values, default } => { + for (key, value) in values { + self.resolve(key)?; + if let Some(value) = value { + self.resolve(value)?; + } + } + if let Some(default) = default { + self.resolve(default)?; + } + } + Expression::RangeInclusive { start, end } + | Expression::RangeExclusive { start, end } => { + if let Some(start) = start { + self.resolve(start)?; + } + if let Some(end) = end { + self.resolve(end)?; + } + } + Expression::FunctionDeclaration { name, body, .. } => { + s + return Err(NameResolutionError::Unsupported { + feature: "function declarations", + span: loc.span, + }); + } + Expression::StructDeclaration { .. } => { + return Err(NameResolutionError::Unsupported { + feature: "struct declarations", + span: loc.span, + }); + } + Expression::OpAssignment { .. } => { + return Err(NameResolutionError::Unsupported { + feature: "augmented assignment operator selection", + span: loc.span, + }); + } + } + Ok(()) + } + + fn lookup(&self, name: &str, span: Span) -> Result { + self.scopes + .lookup(name, &self.symbols) + .ok_or_else(|| NameResolutionError::UnknownName { + name: name.to_string(), + span, + }) + } + + fn declare_pattern(&mut self, pattern: &Lvalue, span: Span) -> Result<(), NameResolutionError> { + // Validate the whole pattern before publishing any of its names. + // The parser already rejects these targets in declarations, but a + // malformed AST should still produce an error rather than new names. + if pattern.non_binding_target().is_some() { + return Err(NameResolutionError::InvalidDeclaration { span }); + } + match pattern { + Lvalue::Identifier { + identifier, span, .. + } => { + let symbol = self.symbols.declare(identifier.clone()); + self.scopes.bind(symbol); + self.lvalue_bindings.push((*span, symbol)); + } + Lvalue::Sequence(items) => { + for item in items { + self.declare_pattern(item, span)?; + } + } + Lvalue::Index { .. } | Lvalue::Member { .. } => { + unreachable!("validated declaration pattern"); + } + } + Ok(()) + } + + fn resolve_target(&mut self, target: &Lvalue) -> Result<(), NameResolutionError> { + match target { + Lvalue::Identifier { + identifier, span, .. + } => { + let symbol = self.lookup(identifier, *span)?; + self.lvalue_bindings.push((*span, symbol)); + } + Lvalue::Sequence(items) => { + for item in items { + self.resolve_target(item)?; + } + } + Lvalue::Index { value, index, .. } => { + self.resolve(value)?; + self.resolve(index)?; + } + Lvalue::Member { receiver, .. } => self.resolve(receiver)?, + } + Ok(()) + } + + fn in_scope( + &mut self, + resolve: impl FnOnce(&mut Self) -> Result<(), NameResolutionError>, + ) -> Result<(), NameResolutionError> { + self.scopes.enter_scope(); + let result = resolve(self); + // Restore visibility on errors as well as successful traversal. + self.scopes.leave_scope(); + result + } + + fn resolve_for( + &mut self, + iterations: &[ForIteration], + body: &ForBody, + span: Span, + ) -> Result<(), NameResolutionError> { + match iterations.split_first() { + Some((ForIteration::Iteration { l_value, sequence }, tail)) => { + // A binder is not visible in its own iterable. Later + // iterables and guards can see the earlier binders. + self.resolve(sequence)?; + self.in_scope(|resolver| { + resolver.declare_pattern(l_value, span)?; + resolver.resolve_for(tail, body, span) + }) + } + Some((ForIteration::Guard(guard), tail)) => { + self.resolve(guard)?; + self.resolve_for(tail, body, span) + } + None => match body { + ForBody::Block(block) | ForBody::List { expr: block } => self.resolve(block), + ForBody::Map { + key, + value, + default, + } => { + self.resolve(key)?; + if let Some(value) = value { + self.resolve(value)?; + } + if let Some(default) = default { + self.resolve(default)?; + } + Ok(()) + } + }, + } + } +} + +#[cfg(test)] +mod resolver_tests; + #[cfg(test)] mod test { use crate::name_resolution::Scope; diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs new file mode 100644 index 00000000..e27ad44e --- /dev/null +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -0,0 +1,302 @@ +use super::{NameResolutionError, NameResolver}; +use ndc_lexer::{Lexer, SourceId}; +use ndc_parser::Parser; + +fn resolve_source(resolver: &mut NameResolver, source: &str) -> Result<(), NameResolutionError> { + let tokens = Lexer::new(source, SourceId::new(0)) + .collect::, _>>() + .expect("source lexes"); + let expressions = Parser::from_tokens(tokens).parse().expect("source parses"); + for expression in &expressions { + resolver.resolve(expression)?; + } + Ok(()) +} + +#[test] +fn initializer_reads_outer_binding_before_shadowing() { + let mut resolver = NameResolver::new(); + resolve_source(&mut resolver, "let x = 1; { let x = x; x; } x;").unwrap(); + + let outer = resolver.lvalue_bindings[0].1; + let inner = resolver.lvalue_bindings[1].1; + assert_ne!(outer, inner); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == outer) + .count(), + 2 + ); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == inner) + .count(), + 1 + ); + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(outer)); +} + +#[test] +fn self_reference_reports_the_initializer_location() { + let mut resolver = NameResolver::new(); + let source = "let missing = missing;"; + let error = resolve_source(&mut resolver, source).unwrap_err(); + assert_eq!( + error.to_string(), + "identifier 'missing' has not been declared" + ); + let NameResolutionError::UnknownName { name, span } = error else { + panic!("expected an unknown name"); + }; + assert_eq!(name, "missing"); + assert_eq!(span.source_id(), SourceId::new(0)); + assert_eq!(&source[span.range()], "missing"); + assert_eq!(span.offset(), source.rfind("missing").unwrap()); + assert!(resolver.lvalue_bindings.is_empty()); +} + +#[test] +fn destructuring_resolves_all_initializers_before_binding_names() { + let mut resolver = NameResolver::new(); + resolve_source( + &mut resolver, + "let x = 1; let y = 2; let (x, (y, z)) = (y, (x, 3)); x; y; z;", + ) + .unwrap(); + + assert_eq!(resolver.lvalue_bindings.len(), 5); + for (_, symbol) in &resolver.lvalue_bindings { + assert_eq!( + resolver + .references + .values() + .filter(|&id| id == symbol) + .count(), + 1 + ); + } + assert_eq!( + resolver.scopes.lookup("x", &resolver.symbols), + Some(resolver.lvalue_bindings[2].1) + ); + assert_eq!( + resolver.scopes.lookup("y", &resolver.symbols), + Some(resolver.lvalue_bindings[3].1) + ); +} + +#[test] +fn assignment_reuses_declared_symbols() { + let mut resolver = NameResolver::new(); + resolve_source( + &mut resolver, + "let x = 1; let y = 2; (x, y) = (y, x); x = 3;", + ) + .unwrap(); + let bindings = &resolver.lvalue_bindings; + assert_eq!(bindings.len(), 5); + assert_eq!(bindings[0].1, bindings[2].1); + assert_eq!(bindings[1].1, bindings[3].1); + assert_eq!(bindings[0].1, bindings[4].1); +} + +#[test] +fn member_and_index_targets_resolve_receivers_without_declaring_names() { + let mut resolver = NameResolver::new(); + // This supplied name stands for a value in the surrounding environment. + let object = resolver.symbols.declare("object".to_string()); + resolver.scopes.bind(object); + resolve_source( + &mut resolver, + "let i = 0; object.field = i; object[i] = i; object.field;", + ) + .unwrap(); + assert_eq!(resolver.lvalue_bindings.len(), 1); + assert_eq!(resolver.scopes.lookup("field", &resolver.symbols), None); + assert_eq!( + resolver.scopes.lookup("object", &resolver.symbols), + Some(object) + ); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == object) + .count(), + 3 + ); +} + +#[test] +fn traversal_reports_unknown_names_in_expression_children() { + let sources = [ + "(missing);", + "true and missing;", + "missing or false;", + "missing as Int;", + "missing(1);", + "callee(missing);", + "1 + missing;", + "missing.field;", + "[1, missing];", + "(1, missing);", + "%{missing: 1};", + "%{1: missing};", + "%{:missing};", + "%{missing};", + "if missing { 1; };", + "if true { missing; };", + "if true { 1; } else { missing; };", + "while missing { 1; };", + "while true { missing; };", + "return missing;", + "missing..;", + "let xs = []; xs[0..missing] = 1;", + "1..=missing;", + "missing = 1;", + "let x = 1; x = missing;", + "let xs = []; xs[missing] = 1;", + "missing[0] = 1;", + "missing.field = 1;", + ]; + for source in sources { + let mut resolver = NameResolver::new(); + for name in ["callee", "+"] { + let symbol = resolver.symbols.declare(name.to_string()); + resolver.scopes.bind(symbol); + } + let error = resolve_source(&mut resolver, source).expect_err(source); + let NameResolutionError::UnknownName { name, span } = error else { + panic!("expected an unknown name in {source:?}, got {error:?}"); + }; + assert_eq!(name, "missing", "{source}"); + assert!(source[span.range()].contains("missing"), "{source}"); + } +} + +#[test] +fn errors_restore_visibility_after_nested_blocks() { + let mut resolver = NameResolver::new(); + resolve_source( + &mut resolver, + "let x = 1; { let x = 2; { let local = 3; missing; } }", + ) + .unwrap_err(); + let outer = resolver.lvalue_bindings[0].1; + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(outer)); + assert_eq!(resolver.scopes.lookup("local", &resolver.symbols), None); + resolve_source(&mut resolver, "x;").unwrap(); + assert_eq!( + resolver.references.values().copied().collect::>(), + vec![outer] + ); +} + +#[test] +fn later_comprehension_clauses_see_earlier_binders() { + let mut resolver = NameResolver::new(); + resolve_source( + &mut resolver, + "let xs = [[true]]; [y for x in xs, y in x, if y];", + ) + .unwrap(); + let bindings = &resolver.lvalue_bindings; + assert_eq!(bindings.len(), 3); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == bindings[0].1) + .count(), + 1 + ); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == bindings[1].1) + .count(), + 1 + ); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == bindings[2].1) + .count(), + 2 + ); + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), None); + assert_eq!(resolver.scopes.lookup("y", &resolver.symbols), None); +} + +#[test] +fn iterable_uses_the_outer_binding_of_its_own_name() { + let mut resolver = NameResolver::new(); + resolve_source(&mut resolver, "let x = [1]; [x for x in x]; x;").unwrap(); + let outer = resolver.lvalue_bindings[0].1; + let inner = resolver.lvalue_bindings[1].1; + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == outer) + .count(), + 2 + ); + assert_eq!( + resolver + .references + .values() + .filter(|&&id| id == inner) + .count(), + 1 + ); +} + +#[test] +fn loop_body_forms_resolve_names_and_restore_scope_on_errors() { + for source in [ + "for x in [1] { missing; }", + "[missing for x in [1]];", + "[x for x in [1], if missing];", + "%{missing: x for x in [1]};", + "%{x: missing for x in [1]};", + "%{:missing, x: x for x in [1]};", + ] { + let mut resolver = NameResolver::new(); + let error = resolve_source(&mut resolver, source).unwrap_err(); + assert!( + matches!(error, NameResolutionError::UnknownName { ref name, .. } if name == "missing"), + "{source}: {error:?}" + ); + assert_eq!( + resolver.scopes.lookup("x", &resolver.symbols), + None, + "{source}" + ); + } +} + +#[test] +fn unfinished_language_features_report_errors_instead_of_panicking() { + for (source, feature) in [ + ("fn identity(x) => x;", "function declarations"), + ("struct Point { x: Int }", "struct declarations"), + ( + "let x = 1; x += 2;", + "augmented assignment operator selection", + ), + ] { + let mut resolver = NameResolver::new(); + let error = resolve_source(&mut resolver, source).unwrap_err(); + assert!( + matches!(error, NameResolutionError::Unsupported { feature: found, span } if found == feature && span.source_id() == SourceId::new(0)), + "{error:?}" + ); + } +} From bedd33617f8fc12014bb8282d9d539a03b83f4f8 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 10:37:27 +0200 Subject: [PATCH 04/19] =?UTF-8?q?refactor(analyser):=20port=20name=20resol?= =?UTF-8?q?ver=20to=20binding=20patterns=20and=20assignment=20targets=20?= =?UTF-8?q?=F0=9F=A7=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key declarations and references by NodeId now that patterns and targets carry their own node identities, replacing the span-keyed lvalue list. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 78 ++++----- .../src/name_resolution/resolver_tests.rs | 149 ++++++------------ ndc_analyser/src/symbols.rs | 2 +- 3 files changed, 86 insertions(+), 143 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index b2031bdc..c39b11fd 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -1,7 +1,10 @@ use crate::symbols::{SymbolId, SymbolTable}; use ahash::AHashMap; use ndc_lexer::Span; -use ndc_parser::{Expression, ExpressionLocation, ForBody, ForIteration, Lvalue, NodeId}; +use ndc_parser::{ + AssignmentTarget, AssignmentTargetLocation, BindingPattern, BindingPatternLocation, Expression, + ExpressionLocation, ForBody, ForIteration, NodeId, +}; #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] pub(crate) struct ScopeId(usize); @@ -20,18 +23,18 @@ pub(crate) struct ScopeTree { pub(crate) struct NameResolver { symbols: SymbolTable, scopes: ScopeTree, + /// The symbol each binding pattern identifier introduces, keyed by the + /// pattern's node. In `let (a, b) = ...`, the `a` and `b` nodes. + declarations: AHashMap, + /// The symbol each name use refers to, keyed by the node naming it. In + /// `x = y`, both the `x` target node and the `y` identifier node. references: AHashMap, - // Lvalues have source spans but no NodeId. Keep their declaration/write - // bindings separately so later passes do not have to resolve them again. - lvalue_bindings: Vec<(Span, SymbolId)>, } #[derive(Debug, thiserror::Error)] pub(crate) enum NameResolutionError { #[error("identifier '{name}' has not been declared")] UnknownName { name: String, span: Span }, - #[error("index and member targets cannot declare names")] - InvalidDeclaration { span: Span }, #[error("name resolution for {feature} is not implemented yet")] Unsupported { feature: &'static str, span: Span }, } @@ -76,11 +79,7 @@ impl ScopeTree { return Some(id); } - current_scope = if let Some(parent) = current_scope.parent { - &self.scopes[parent.0] - } else { - return None; - } + current_scope = &self.scopes[current_scope.parent?.0]; } } @@ -101,8 +100,8 @@ impl NameResolver { Self { symbols: Default::default(), scopes: ScopeTree::new(), + declarations: Default::default(), references: Default::default(), - lvalue_bindings: Default::default(), } } @@ -129,7 +128,7 @@ impl NameResolver { } Expression::VariableDeclaration { l_value, value, .. } => { self.resolve(value)?; - self.declare_pattern(l_value, loc.span)?; + self.declare_pattern(l_value); } Expression::Assignment { l_value, r_value } => { self.resolve_target(l_value)?; @@ -161,7 +160,7 @@ impl NameResolver { self.resolve(expression)?; self.resolve(loop_body)?; } - Expression::For { iterations, body } => self.resolve_for(iterations, body, loc.span)?, + Expression::For { iterations, body } => self.resolve_for(iterations, body)?, Expression::Call { function, arguments, @@ -201,8 +200,7 @@ impl NameResolver { self.resolve(end)?; } } - Expression::FunctionDeclaration { name, body, .. } => { - s + Expression::FunctionDeclaration { .. } => { return Err(NameResolutionError::Unsupported { feature: "function declarations", span: loc.span, @@ -233,51 +231,42 @@ impl NameResolver { }) } - fn declare_pattern(&mut self, pattern: &Lvalue, span: Span) -> Result<(), NameResolutionError> { - // Validate the whole pattern before publishing any of its names. - // The parser already rejects these targets in declarations, but a - // malformed AST should still produce an error rather than new names. - if pattern.non_binding_target().is_some() { - return Err(NameResolutionError::InvalidDeclaration { span }); - } - match pattern { - Lvalue::Identifier { - identifier, span, .. - } => { + fn declare_pattern(&mut self, pattern: &BindingPatternLocation) { + match &pattern.pattern { + BindingPattern::Identifier { identifier, .. } => { let symbol = self.symbols.declare(identifier.clone()); self.scopes.bind(symbol); - self.lvalue_bindings.push((*span, symbol)); + self.declarations.insert(pattern.id, symbol); } - Lvalue::Sequence(items) => { + BindingPattern::Sequence(items) => { for item in items { - self.declare_pattern(item, span)?; + self.declare_pattern(item); } } - Lvalue::Index { .. } | Lvalue::Member { .. } => { - unreachable!("validated declaration pattern"); - } } - Ok(()) } - fn resolve_target(&mut self, target: &Lvalue) -> Result<(), NameResolutionError> { - match target { - Lvalue::Identifier { + fn resolve_target( + &mut self, + target: &AssignmentTargetLocation, + ) -> Result<(), NameResolutionError> { + match &target.target { + AssignmentTarget::Identifier { identifier, span, .. } => { let symbol = self.lookup(identifier, *span)?; - self.lvalue_bindings.push((*span, symbol)); + self.references.insert(target.id, symbol); } - Lvalue::Sequence(items) => { + AssignmentTarget::Sequence(items) => { for item in items { self.resolve_target(item)?; } } - Lvalue::Index { value, index, .. } => { + AssignmentTarget::Index { value, index, .. } => { self.resolve(value)?; self.resolve(index)?; } - Lvalue::Member { receiver, .. } => self.resolve(receiver)?, + AssignmentTarget::Member { receiver, .. } => self.resolve(receiver)?, } Ok(()) } @@ -297,7 +286,6 @@ impl NameResolver { &mut self, iterations: &[ForIteration], body: &ForBody, - span: Span, ) -> Result<(), NameResolutionError> { match iterations.split_first() { Some((ForIteration::Iteration { l_value, sequence }, tail)) => { @@ -305,13 +293,13 @@ impl NameResolver { // iterables and guards can see the earlier binders. self.resolve(sequence)?; self.in_scope(|resolver| { - resolver.declare_pattern(l_value, span)?; - resolver.resolve_for(tail, body, span) + resolver.declare_pattern(l_value); + resolver.resolve_for(tail, body) }) } Some((ForIteration::Guard(guard), tail)) => { self.resolve(guard)?; - self.resolve_for(tail, body, span) + self.resolve_for(tail, body) } None => match body { ForBody::Block(block) | ForBody::List { expr: block } => self.resolve(block), diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index e27ad44e..6b609529 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,4 +1,6 @@ use super::{NameResolutionError, NameResolver}; +use crate::symbols::SymbolId; +use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; use ndc_parser::Parser; @@ -13,30 +15,35 @@ fn resolve_source(resolver: &mut NameResolver, source: &str) -> Result<(), NameR Ok(()) } +/// Declared symbols in declaration order; the symbol table hands out ids sequentially. +fn declared(resolver: &NameResolver) -> [SymbolId; N] { + let symbols = resolver + .declarations + .values() + .copied() + .sorted() + .collect_vec(); + symbols.try_into().unwrap_or_else(|symbols: Vec<_>| { + panic!("expected {N} declarations, got {}", symbols.len()) + }) +} + +fn uses(resolver: &NameResolver, symbol: SymbolId) -> usize { + resolver + .references + .values() + .filter(|&&id| id == symbol) + .count() +} + #[test] fn initializer_reads_outer_binding_before_shadowing() { let mut resolver = NameResolver::new(); resolve_source(&mut resolver, "let x = 1; { let x = x; x; } x;").unwrap(); - let outer = resolver.lvalue_bindings[0].1; - let inner = resolver.lvalue_bindings[1].1; - assert_ne!(outer, inner); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == outer) - .count(), - 2 - ); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == inner) - .count(), - 1 - ); + let [outer, inner] = declared(&resolver); + assert_eq!(uses(&resolver, outer), 2); + assert_eq!(uses(&resolver, inner), 1); assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(outer)); } @@ -56,7 +63,7 @@ fn self_reference_reports_the_initializer_location() { assert_eq!(span.source_id(), SourceId::new(0)); assert_eq!(&source[span.range()], "missing"); assert_eq!(span.offset(), source.rfind("missing").unwrap()); - assert!(resolver.lvalue_bindings.is_empty()); + assert!(resolver.declarations.is_empty()); } #[test] @@ -68,25 +75,13 @@ fn destructuring_resolves_all_initializers_before_binding_names() { ) .unwrap(); - assert_eq!(resolver.lvalue_bindings.len(), 5); - for (_, symbol) in &resolver.lvalue_bindings { - assert_eq!( - resolver - .references - .values() - .filter(|&id| id == symbol) - .count(), - 1 - ); + let symbols: [_; 5] = declared(&resolver); + for symbol in symbols { + assert_eq!(uses(&resolver, symbol), 1); } - assert_eq!( - resolver.scopes.lookup("x", &resolver.symbols), - Some(resolver.lvalue_bindings[2].1) - ); - assert_eq!( - resolver.scopes.lookup("y", &resolver.symbols), - Some(resolver.lvalue_bindings[3].1) - ); + let [_, _, x, y, _] = symbols; + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(x)); + assert_eq!(resolver.scopes.lookup("y", &resolver.symbols), Some(y)); } #[test] @@ -97,11 +92,11 @@ fn assignment_reuses_declared_symbols() { "let x = 1; let y = 2; (x, y) = (y, x); x = 3;", ) .unwrap(); - let bindings = &resolver.lvalue_bindings; - assert_eq!(bindings.len(), 5); - assert_eq!(bindings[0].1, bindings[2].1); - assert_eq!(bindings[1].1, bindings[3].1); - assert_eq!(bindings[0].1, bindings[4].1); + + // Assignment targets refer to the existing symbols instead of declaring new ones. + let [x, y] = declared(&resolver); + assert_eq!(uses(&resolver, x), 3); + assert_eq!(uses(&resolver, y), 2); } #[test] @@ -115,20 +110,15 @@ fn member_and_index_targets_resolve_receivers_without_declaring_names() { "let i = 0; object.field = i; object[i] = i; object.field;", ) .unwrap(); - assert_eq!(resolver.lvalue_bindings.len(), 1); + + let [i] = declared(&resolver); + assert_eq!(uses(&resolver, i), 3); + assert_eq!(uses(&resolver, object), 3); assert_eq!(resolver.scopes.lookup("field", &resolver.symbols), None); assert_eq!( resolver.scopes.lookup("object", &resolver.symbols), Some(object) ); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == object) - .count(), - 3 - ); } #[test] @@ -186,7 +176,7 @@ fn errors_restore_visibility_after_nested_blocks() { "let x = 1; { let x = 2; { let local = 3; missing; } }", ) .unwrap_err(); - let outer = resolver.lvalue_bindings[0].1; + let [outer, _, _] = declared(&resolver); assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(outer)); assert_eq!(resolver.scopes.lookup("local", &resolver.symbols), None); resolve_source(&mut resolver, "x;").unwrap(); @@ -204,32 +194,11 @@ fn later_comprehension_clauses_see_earlier_binders() { "let xs = [[true]]; [y for x in xs, y in x, if y];", ) .unwrap(); - let bindings = &resolver.lvalue_bindings; - assert_eq!(bindings.len(), 3); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == bindings[0].1) - .count(), - 1 - ); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == bindings[1].1) - .count(), - 1 - ); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == bindings[2].1) - .count(), - 2 - ); + + let [xs, x, y] = declared(&resolver); + assert_eq!(uses(&resolver, xs), 1); + assert_eq!(uses(&resolver, x), 1); + assert_eq!(uses(&resolver, y), 2); assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), None); assert_eq!(resolver.scopes.lookup("y", &resolver.symbols), None); } @@ -238,24 +207,10 @@ fn later_comprehension_clauses_see_earlier_binders() { fn iterable_uses_the_outer_binding_of_its_own_name() { let mut resolver = NameResolver::new(); resolve_source(&mut resolver, "let x = [1]; [x for x in x]; x;").unwrap(); - let outer = resolver.lvalue_bindings[0].1; - let inner = resolver.lvalue_bindings[1].1; - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == outer) - .count(), - 2 - ); - assert_eq!( - resolver - .references - .values() - .filter(|&&id| id == inner) - .count(), - 1 - ); + + let [outer, inner] = declared(&resolver); + assert_eq!(uses(&resolver, outer), 2); + assert_eq!(uses(&resolver, inner), 1); } #[test] diff --git a/ndc_analyser/src/symbols.rs b/ndc_analyser/src/symbols.rs index fc9bba09..65b1ac48 100644 --- a/ndc_analyser/src/symbols.rs +++ b/ndc_analyser/src/symbols.rs @@ -1,4 +1,4 @@ -#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)] +#[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] pub(crate) struct SymbolId(usize); #[derive(Debug, Default)] From 891222281ac669b8ed198a60e867dedd1c11e0d5 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 12:11:07 +0200 Subject: [PATCH 05/19] =?UTF-8?q?feat(analyser):=20record=20binding=20kind?= =?UTF-8?q?s=20in=20the=20symbol=20table=20=F0=9F=8F=B7=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Function declarations join overload sets while plain bindings shadow, so each symbol now records which of the two it is. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 32 ++++++----- .../src/name_resolution/resolver_tests.rs | 12 +++- ndc_analyser/src/symbols.rs | 56 ++++++++++++++----- 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index c39b11fd..f14da2bc 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -1,4 +1,4 @@ -use crate::symbols::{SymbolId, SymbolTable}; +use crate::symbols::{BindingKind, SymbolId, SymbolTable}; use ahash::AHashMap; use ndc_lexer::Span; use ndc_parser::{ @@ -52,7 +52,7 @@ impl Scope { pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { for id in self.bindings.iter().rev() { - if table.get(*id) == name { + if table.name(*id) == name { return Some(*id); } } @@ -234,7 +234,9 @@ impl NameResolver { fn declare_pattern(&mut self, pattern: &BindingPatternLocation) { match &pattern.pattern { BindingPattern::Identifier { identifier, .. } => { - let symbol = self.symbols.declare(identifier.clone()); + let symbol = self + .symbols + .declare(identifier.to_owned(), BindingKind::Plain); self.scopes.bind(symbol); self.declarations.insert(pattern.id, symbol); } @@ -329,14 +331,14 @@ mod resolver_tests; mod test { use crate::name_resolution::Scope; use crate::name_resolution::ScopeTree; - use crate::symbols::SymbolTable; + use crate::symbols::{BindingKind, SymbolTable}; #[test] fn binding_follows_shadowing_rules() { let mut table = SymbolTable::default(); - let first = table.declare("foo".to_string()); - let second = table.declare("foo".to_string()); - let other = table.declare("another".to_string()); + let first = table.declare("foo".to_string(), BindingKind::Plain); + let second = table.declare("foo".to_string(), BindingKind::Plain); + let other = table.declare("another".to_string(), BindingKind::Plain); let mut scope = Scope::default(); scope.bind(first); @@ -356,8 +358,8 @@ mod test { #[test] fn scope_tree_traverses() { let mut table = SymbolTable::default(); - let first = table.declare("foo".to_string()); - let middle = table.declare("bar".to_string()); + let first = table.declare("foo".to_string(), BindingKind::Plain); + let middle = table.declare("bar".to_string(), BindingKind::Plain); let mut tree = ScopeTree::new(); tree.bind(first); @@ -378,8 +380,8 @@ mod test { #[test] fn scope_tree_shadows() { let mut table = SymbolTable::default(); - let first = table.declare("foo".to_string()); - let second = table.declare("foo".to_string()); + let first = table.declare("foo".to_string(), BindingKind::Plain); + let second = table.declare("foo".to_string(), BindingKind::Plain); let mut tree = ScopeTree::new(); tree.bind(first); @@ -394,8 +396,8 @@ mod test { #[test] fn sibling_scopes_do_not_share_bindings() { let mut table = SymbolTable::default(); - let first = table.declare("first".to_string()); - let second = table.declare("second".to_string()); + let first = table.declare("first".to_string(), BindingKind::Plain); + let second = table.declare("second".to_string(), BindingKind::Plain); let mut tree = ScopeTree::new(); tree.enter_scope(); @@ -415,8 +417,8 @@ mod test { #[test] fn lookup_does_not_change_where_new_bindings_are_created() { let mut table = SymbolTable::default(); - let outer = table.declare("outer".to_string()); - let inner = table.declare("inner".to_string()); + let outer = table.declare("outer".to_string(), BindingKind::Plain); + let inner = table.declare("inner".to_string(), BindingKind::Plain); let mut tree = ScopeTree::new(); tree.bind(outer); diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 6b609529..6add9bf8 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,5 +1,5 @@ use super::{NameResolutionError, NameResolver}; -use crate::symbols::SymbolId; +use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; use ndc_parser::Parser; @@ -42,6 +42,8 @@ fn initializer_reads_outer_binding_before_shadowing() { resolve_source(&mut resolver, "let x = 1; { let x = x; x; } x;").unwrap(); let [outer, inner] = declared(&resolver); + assert_eq!(resolver.symbols.kind(outer), BindingKind::Plain); + assert_eq!(resolver.symbols.kind(inner), BindingKind::Plain); assert_eq!(uses(&resolver, outer), 2); assert_eq!(uses(&resolver, inner), 1); assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), Some(outer)); @@ -103,7 +105,9 @@ fn assignment_reuses_declared_symbols() { fn member_and_index_targets_resolve_receivers_without_declaring_names() { let mut resolver = NameResolver::new(); // This supplied name stands for a value in the surrounding environment. - let object = resolver.symbols.declare("object".to_string()); + let object = resolver + .symbols + .declare("object".to_string(), BindingKind::Plain); resolver.scopes.bind(object); resolve_source( &mut resolver, @@ -156,7 +160,9 @@ fn traversal_reports_unknown_names_in_expression_children() { for source in sources { let mut resolver = NameResolver::new(); for name in ["callee", "+"] { - let symbol = resolver.symbols.declare(name.to_string()); + let symbol = resolver + .symbols + .declare(name.to_string(), BindingKind::Function); resolver.scopes.bind(symbol); } let error = resolve_source(&mut resolver, source).expect_err(source); diff --git a/ndc_analyser/src/symbols.rs b/ndc_analyser/src/symbols.rs index 65b1ac48..7a70f737 100644 --- a/ndc_analyser/src/symbols.rs +++ b/ndc_analyser/src/symbols.rs @@ -1,43 +1,73 @@ #[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] pub(crate) struct SymbolId(usize); +#[derive(Debug)] +struct Symbol { + name: String, + kind: BindingKind, +} + #[derive(Debug, Default)] pub(crate) struct SymbolTable { - names: Vec, + symbols: Vec, } impl SymbolTable { - pub(crate) fn declare(&mut self, name: String) -> SymbolId { - self.names.push(name); - SymbolId(self.names.len() - 1) + pub(crate) fn declare(&mut self, name: String, kind: BindingKind) -> SymbolId { + self.symbols.push(Symbol { name, kind }); + SymbolId(self.symbols.len() - 1) + } + + pub(crate) fn name(&self, id: SymbolId) -> &str { + &self.symbols[id.0].name } - pub(crate) fn get(&self, id: SymbolId) -> &str { - &self.names[id.0] + pub(crate) fn kind(&self, id: SymbolId) -> BindingKind { + self.symbols[id.0].kind } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum BindingKind { + /// Example: function declaration like `fn x(y)`: these join into an overload set + Function, + /// Example: bindings, parameters, and loop variables: these don't participate in overloading + Plain, +} + #[cfg(test)] mod test { - use crate::symbols::SymbolTable; + use crate::symbols::{BindingKind, SymbolTable}; #[test] fn it_tracks_multiple_identifiers() { let mut table = SymbolTable::default(); - let a = table.declare("foo".to_string()); - let b = table.declare("bar".to_string()); + let a = table.declare("foo".to_string(), BindingKind::Plain); + let b = table.declare("bar".to_string(), BindingKind::Plain); assert_ne!(a, b); - assert_eq!(table.get(a), "foo"); - assert_eq!(table.get(b), "bar"); + assert_eq!(table.name(a), "foo"); + assert_eq!(table.name(b), "bar"); } #[test] fn identifiers_with_the_same_name_are_not_equal() { let mut table = SymbolTable::default(); - let a = table.declare("foo".to_string()); - let b = table.declare("foo".to_string()); + let a = table.declare("foo".to_string(), BindingKind::Function); + let b = table.declare("foo".to_string(), BindingKind::Plain); assert_ne!(a, b); } + + #[test] + fn it_tracks_binding_kinds() { + let mut table = SymbolTable::default(); + let function = table.declare("foo".to_string(), BindingKind::Function); + let plain = table.declare("bar".to_string(), BindingKind::Plain); + + assert_eq!(table.name(function), "foo"); + assert_eq!(table.kind(function), BindingKind::Function); + assert_eq!(table.name(plain), "bar"); + assert_eq!(table.kind(plain), BindingKind::Plain); + } } From 9601bca9d88d5861aabcf3f4045c630cedc92d62 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 12:35:07 +0200 Subject: [PATCH 06/19] =?UTF-8?q?feat(analyser):=20resolve=20identifiers?= =?UTF-8?q?=20to=20grouped=20candidates=20=F0=9F=A7=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An identifier now records every same-named symbol it can see, nearest scope first, so the type pass can choose between overloads and plain bindings. Assignment targets still resolve to the innermost symbol. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 68 +++++++++++++---- .../src/name_resolution/resolver_tests.rs | 73 ++++++++++++++++++- 2 files changed, 123 insertions(+), 18 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index f14da2bc..4b7f11b5 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -26,9 +26,12 @@ pub(crate) struct NameResolver { /// The symbol each binding pattern identifier introduces, keyed by the /// pattern's node. In `let (a, b) = ...`, the `a` and `b` nodes. declarations: AHashMap, - /// The symbol each name use refers to, keyed by the node naming it. In - /// `x = y`, both the `x` target node and the `y` identifier node. - references: AHashMap, + /// Every symbol an identifier use could mean, keyed by the identifier's + /// node. The type pass picks between them. + references: AHashMap, + /// The symbol each identifier assignment target writes to, keyed by the + /// target's node. In `x = 1`, the `x` node. + assignments: AHashMap, } #[derive(Debug, thiserror::Error)] @@ -39,6 +42,11 @@ pub(crate) enum NameResolutionError { Unsupported { feature: &'static str, span: Span }, } +/// Same-named symbols visible at a use, nearest scope first, newest first +/// within a scope. For `x` in `let x = 1; { let x = 2; x }`: `[[x2], [x1]]`. +#[derive(Debug)] +pub(crate) struct Candidates(Vec>); + impl Scope { pub(crate) fn new_with_parent(parent: ScopeId) -> Self { Self { @@ -58,6 +66,15 @@ impl Scope { } None } + + fn candidates(&self, name: &str, table: &SymbolTable) -> Vec { + self.bindings + .iter() + .rev() + .filter(|id| table.name(**id) == name) + .copied() + .collect() + } } impl ScopeTree { @@ -72,15 +89,15 @@ impl ScopeTree { self.scopes[self.current.0].bind(symbol_id); } - pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { - let mut current_scope = &self.scopes[self.current.0]; - loop { - if let Some(id) = current_scope.lookup(name, table) { - return Some(id); - } + /// The current scope followed by its ancestors, ending at the root. + fn chain(&self) -> impl Iterator { + std::iter::successors(Some(&self.scopes[self.current.0]), |scope| { + scope.parent.map(|parent| &self.scopes[parent.0]) + }) + } - current_scope = &self.scopes[current_scope.parent?.0]; - } + pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { + self.chain().find_map(|scope| scope.lookup(name, table)) } pub(crate) fn enter_scope(&mut self) { @@ -93,6 +110,15 @@ impl ScopeTree { .parent .expect("internal error: scope stack is empty"); } + + pub(crate) fn candidates(&self, name: &str, table: &SymbolTable) -> Candidates { + Candidates( + self.chain() + .map(|scope| scope.candidates(name, table)) + .filter(|group| !group.is_empty()) + .collect(), + ) + } } impl NameResolver { @@ -102,6 +128,7 @@ impl NameResolver { scopes: ScopeTree::new(), declarations: Default::default(), references: Default::default(), + assignments: Default::default(), } } @@ -115,8 +142,8 @@ impl NameResolver { | Expression::Break | Expression::Continue => {} Expression::Identifier { name, .. } => { - let symbol = self.lookup(name, loc.span)?; - self.references.insert(loc.id, symbol); + let candidates = self.candidates(name, loc.span)?; + self.references.insert(loc.id, candidates); } Expression::Statement(inner) | Expression::Grouping(inner) @@ -222,6 +249,19 @@ impl NameResolver { Ok(()) } + fn candidates(&self, name: &str, span: Span) -> Result { + let out = self.scopes.candidates(name, &self.symbols); + + if out.0.is_empty() { + return Err(NameResolutionError::UnknownName { + name: name.to_string(), + span, + }); + } + + Ok(out) + } + fn lookup(&self, name: &str, span: Span) -> Result { self.scopes .lookup(name, &self.symbols) @@ -257,7 +297,7 @@ impl NameResolver { identifier, span, .. } => { let symbol = self.lookup(identifier, *span)?; - self.references.insert(target.id, symbol); + self.assignments.insert(target.id, symbol); } AssignmentTarget::Sequence(items) => { for item in items { diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 6add9bf8..8a944903 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,4 +1,4 @@ -use super::{NameResolutionError, NameResolver}; +use super::{Candidates, NameResolutionError, NameResolver}; use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; @@ -28,12 +28,32 @@ fn declared(resolver: &NameResolver) -> [SymbolId; N] { }) } +/// The symbol a plain value use means: the newest binding in the nearest scope. +fn innermost(candidates: &Candidates) -> SymbolId { + candidates.0[0][0] +} + +/// Reads whose innermost candidate is `symbol`, plus writes to it. fn uses(resolver: &NameResolver, symbol: SymbolId) -> usize { - resolver + let reads = resolver .references .values() + .filter(|candidates| innermost(candidates) == symbol) + .count(); + let writes = resolver + .assignments + .values() .filter(|&&id| id == symbol) - .count() + .count(); + reads + writes +} + +fn only_reference(resolver: &NameResolver) -> &Candidates { + resolver + .references + .values() + .exactly_one() + .expect("exactly one reference") } #[test] @@ -187,7 +207,7 @@ fn errors_restore_visibility_after_nested_blocks() { assert_eq!(resolver.scopes.lookup("local", &resolver.symbols), None); resolve_source(&mut resolver, "x;").unwrap(); assert_eq!( - resolver.references.values().copied().collect::>(), + resolver.references.values().map(innermost).collect_vec(), vec![outer] ); } @@ -261,3 +281,48 @@ fn unfinished_language_features_report_errors_instead_of_panicking() { ); } } + +#[test] +fn candidates_in_one_scope_are_newest_first() { + let mut resolver = NameResolver::new(); + resolve_source(&mut resolver, "let x = 1; let x = 2; x;").unwrap(); + + let [first, second] = declared(&resolver); + assert_eq!(only_reference(&resolver).0, vec![vec![second, first]]); +} + +#[test] +fn candidates_are_grouped_by_scope_without_empty_scopes() { + let mut resolver = NameResolver::new(); + resolve_source(&mut resolver, "let x = 1; { let x = 2; let x = 3; { x; } }").unwrap(); + + let [x1, x2, x3] = declared(&resolver); + assert_eq!(only_reference(&resolver).0, vec![vec![x3, x2], vec![x1]]); +} + +#[test] +fn assignment_targets_resolve_to_the_innermost_symbol() { + let mut resolver = NameResolver::new(); + resolve_source(&mut resolver, "let x = 1; { let x = 2; x = 3; }").unwrap(); + + let [_, inner] = declared(&resolver); + assert_eq!( + resolver.assignments.values().copied().collect_vec(), + vec![inner] + ); +} + +#[test] +fn candidates_include_functions_and_plain_bindings() { + let mut resolver = NameResolver::new(); + // This supplied name stands for a native function. + let native = resolver + .symbols + .declare("len".to_string(), BindingKind::Function); + resolver.scopes.bind(native); + resolve_source(&mut resolver, "let len = 99; len;").unwrap(); + + // The resolver keeps both; choosing between them is the type pass's job. + let [local] = declared(&resolver); + assert_eq!(only_reference(&resolver).0, vec![vec![local, native]]); +} From 3682dff89e62ab2c1db940caed518f4d1a4c6f83 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 12:44:34 +0200 Subject: [PATCH 07/19] =?UTF-8?q?feat(analyser):=20declare=20builtins=20in?= =?UTF-8?q?=20a=20scope=20outside=20the=20program=20=F0=9F=8F=9B=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Natives now live in a root scope above the program's top level, so a top-level binding shadows a native instead of sharing its scope. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 18 +++- .../src/name_resolution/resolver_tests.rs | 91 ++++++++++++------- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 4b7f11b5..0a77dedc 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -122,10 +122,22 @@ impl ScopeTree { } impl NameResolver { - pub(crate) fn new() -> Self { + /// Creates a new name resolver with the supplied `builtins` declared in a scope outside + /// the program. With `new(["len"])`, a top-level `let len = 99` shadows the native + /// instead of sharing its scope, so a later `len` resolves to `[[let len], [native len]]`. + pub(crate) fn new<'a>(builtins: impl IntoIterator) -> Self { + let mut scopes = ScopeTree::new(); + let mut symbols = SymbolTable::default(); + + for builtin in builtins { + let symbol = symbols.declare(builtin.to_string(), BindingKind::Function); + scopes.bind(symbol); + } + + scopes.enter_scope(); Self { - symbols: Default::default(), - scopes: ScopeTree::new(), + symbols, + scopes, declarations: Default::default(), references: Default::default(), assignments: Default::default(), diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 8a944903..36d9904b 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -48,6 +48,14 @@ fn uses(resolver: &NameResolver, symbol: SymbolId) -> usize { reads + writes } +/// The newest built-in declared under `name`, before any source is resolved. +fn builtin(resolver: &NameResolver, name: &str) -> SymbolId { + resolver + .scopes + .lookup(name, &resolver.symbols) + .expect("builtin is declared") +} + fn only_reference(resolver: &NameResolver) -> &Candidates { resolver .references @@ -58,7 +66,7 @@ fn only_reference(resolver: &NameResolver) -> &Candidates { #[test] fn initializer_reads_outer_binding_before_shadowing() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source(&mut resolver, "let x = 1; { let x = x; x; } x;").unwrap(); let [outer, inner] = declared(&resolver); @@ -71,7 +79,7 @@ fn initializer_reads_outer_binding_before_shadowing() { #[test] fn self_reference_reports_the_initializer_location() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); let source = "let missing = missing;"; let error = resolve_source(&mut resolver, source).unwrap_err(); assert_eq!( @@ -90,7 +98,7 @@ fn self_reference_reports_the_initializer_location() { #[test] fn destructuring_resolves_all_initializers_before_binding_names() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source( &mut resolver, "let x = 1; let y = 2; let (x, (y, z)) = (y, (x, 3)); x; y; z;", @@ -108,7 +116,7 @@ fn destructuring_resolves_all_initializers_before_binding_names() { #[test] fn assignment_reuses_declared_symbols() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source( &mut resolver, "let x = 1; let y = 2; (x, y) = (y, x); x = 3;", @@ -123,19 +131,14 @@ fn assignment_reuses_declared_symbols() { #[test] fn member_and_index_targets_resolve_receivers_without_declaring_names() { - let mut resolver = NameResolver::new(); - // This supplied name stands for a value in the surrounding environment. - let object = resolver - .symbols - .declare("object".to_string(), BindingKind::Plain); - resolver.scopes.bind(object); + let mut resolver = NameResolver::new([]); resolve_source( &mut resolver, - "let i = 0; object.field = i; object[i] = i; object.field;", + "let object = (); let i = 0; object.field = i; object[i] = i; object.field;", ) .unwrap(); - let [i] = declared(&resolver); + let [object, i] = declared(&resolver); assert_eq!(uses(&resolver, i), 3); assert_eq!(uses(&resolver, object), 3); assert_eq!(resolver.scopes.lookup("field", &resolver.symbols), None); @@ -178,13 +181,7 @@ fn traversal_reports_unknown_names_in_expression_children() { "missing.field = 1;", ]; for source in sources { - let mut resolver = NameResolver::new(); - for name in ["callee", "+"] { - let symbol = resolver - .symbols - .declare(name.to_string(), BindingKind::Function); - resolver.scopes.bind(symbol); - } + let mut resolver = NameResolver::new(["callee", "+"]); let error = resolve_source(&mut resolver, source).expect_err(source); let NameResolutionError::UnknownName { name, span } = error else { panic!("expected an unknown name in {source:?}, got {error:?}"); @@ -196,7 +193,7 @@ fn traversal_reports_unknown_names_in_expression_children() { #[test] fn errors_restore_visibility_after_nested_blocks() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source( &mut resolver, "let x = 1; { let x = 2; { let local = 3; missing; } }", @@ -214,7 +211,7 @@ fn errors_restore_visibility_after_nested_blocks() { #[test] fn later_comprehension_clauses_see_earlier_binders() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source( &mut resolver, "let xs = [[true]]; [y for x in xs, y in x, if y];", @@ -231,7 +228,7 @@ fn later_comprehension_clauses_see_earlier_binders() { #[test] fn iterable_uses_the_outer_binding_of_its_own_name() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source(&mut resolver, "let x = [1]; [x for x in x]; x;").unwrap(); let [outer, inner] = declared(&resolver); @@ -249,7 +246,7 @@ fn loop_body_forms_resolve_names_and_restore_scope_on_errors() { "%{x: missing for x in [1]};", "%{:missing, x: x for x in [1]};", ] { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); let error = resolve_source(&mut resolver, source).unwrap_err(); assert!( matches!(error, NameResolutionError::UnknownName { ref name, .. } if name == "missing"), @@ -273,7 +270,7 @@ fn unfinished_language_features_report_errors_instead_of_panicking() { "augmented assignment operator selection", ), ] { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); let error = resolve_source(&mut resolver, source).unwrap_err(); assert!( matches!(error, NameResolutionError::Unsupported { feature: found, span } if found == feature && span.source_id() == SourceId::new(0)), @@ -284,7 +281,7 @@ fn unfinished_language_features_report_errors_instead_of_panicking() { #[test] fn candidates_in_one_scope_are_newest_first() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source(&mut resolver, "let x = 1; let x = 2; x;").unwrap(); let [first, second] = declared(&resolver); @@ -293,7 +290,7 @@ fn candidates_in_one_scope_are_newest_first() { #[test] fn candidates_are_grouped_by_scope_without_empty_scopes() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source(&mut resolver, "let x = 1; { let x = 2; let x = 3; { x; } }").unwrap(); let [x1, x2, x3] = declared(&resolver); @@ -302,7 +299,7 @@ fn candidates_are_grouped_by_scope_without_empty_scopes() { #[test] fn assignment_targets_resolve_to_the_innermost_symbol() { - let mut resolver = NameResolver::new(); + let mut resolver = NameResolver::new([]); resolve_source(&mut resolver, "let x = 1; { let x = 2; x = 3; }").unwrap(); let [_, inner] = declared(&resolver); @@ -314,15 +311,41 @@ fn assignment_targets_resolve_to_the_innermost_symbol() { #[test] fn candidates_include_functions_and_plain_bindings() { - let mut resolver = NameResolver::new(); - // This supplied name stands for a native function. - let native = resolver - .symbols - .declare("len".to_string(), BindingKind::Function); - resolver.scopes.bind(native); + let mut resolver = NameResolver::new(["len"]); + let native = builtin(&resolver, "len"); resolve_source(&mut resolver, "let len = 99; len;").unwrap(); // The resolver keeps both; choosing between them is the type pass's job. + // The native lives in the built-in scope, outside the program's top level. let [local] = declared(&resolver); - assert_eq!(only_reference(&resolver).0, vec![vec![local, native]]); + assert_eq!(only_reference(&resolver).0, vec![vec![local], vec![native]]); +} + +#[test] +fn builtins_resolve_as_functions() { + let mut resolver = NameResolver::new(["len"]); + let native = builtin(&resolver, "len"); + resolve_source(&mut resolver, "len;").unwrap(); + + assert_eq!(only_reference(&resolver).0, vec![vec![native]]); + assert_eq!(resolver.symbols.kind(native), BindingKind::Function); +} + +#[test] +fn overloaded_builtins_share_a_group_newest_first() { + let mut resolver = NameResolver::new(["max", "max"]); + resolve_source(&mut resolver, "max;").unwrap(); + + let [group] = only_reference(&resolver).0.as_slice() else { + panic!("expected one scope of candidates"); + }; + let [newest, oldest] = group[..] else { + panic!("expected both overloads, got {group:?}"); + }; + assert!(newest > oldest); + assert!( + group + .iter() + .all(|&id| resolver.symbols.kind(id) == BindingKind::Function) + ); } From 8f182f200b374742a734dd55b301f83beceb2055 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 13:10:27 +0200 Subject: [PATCH 08/19] =?UTF-8?q?feat(analyser):=20resolve=20function=20de?= =?UTF-8?q?clarations=20and=20parameters=20=F0=9F=A7=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Named functions bind before their body so recursion resolves, and parameters live in a scope that ends with the function. Duplicate parameters keep the current analyser's error message. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 42 +++++++-- .../src/name_resolution/resolver_tests.rs | 91 ++++++++++++++++++- ndc_parser/src/expression.rs | 9 ++ 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 0a77dedc..d603cb64 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -23,8 +23,10 @@ pub(crate) struct ScopeTree { pub(crate) struct NameResolver { symbols: SymbolTable, scopes: ScopeTree, - /// The symbol each binding pattern identifier introduces, keyed by the - /// pattern's node. In `let (a, b) = ...`, the `a` and `b` nodes. + /// The symbol each binding pattern identifier or named function + /// introduces. In `let (a, b) = ...`, keyed by the `a` and `b` pattern + /// nodes. In `fn a(x) { }`, keyed by the function declaration's node, + /// since the name has no node of its own. declarations: AHashMap, /// Every symbol an identifier use could mean, keyed by the identifier's /// node. The type pass picks between them. @@ -40,6 +42,8 @@ pub(crate) enum NameResolutionError { UnknownName { name: String, span: Span }, #[error("name resolution for {feature} is not implemented yet")] Unsupported { feature: &'static str, span: Span }, + #[error("Illegal redefinition of parameter {name}")] + DuplicateParameter { name: String, span: Span }, } /// Same-named symbols visible at a use, nearest scope first, newest first @@ -239,11 +243,35 @@ impl NameResolver { self.resolve(end)?; } } - Expression::FunctionDeclaration { .. } => { - return Err(NameResolutionError::Unsupported { - feature: "function declarations", - span: loc.span, - }); + Expression::FunctionDeclaration { + name, + parameters, + body, + .. + } => { + if let Some(name) = name { + let symbol = self.symbols.declare(name.clone(), BindingKind::Function); + self.scopes.bind(symbol); + self.declarations.insert(loc.id, symbol); + } + + // Parameter lists are short, so a linear scan beats hashing. + let mut names = Vec::new(); + self.in_scope(|resolver| { + for parameter in parameters { + let param_name = parameter.name(); + if names.contains(¶m_name) { + return Err(NameResolutionError::DuplicateParameter { + name: param_name.to_string(), + span: parameter.span, + }); + } + names.push(param_name); + resolver.declare_pattern(¶meter.lvalue); + } + + resolver.resolve(body) + })?; } Expression::StructDeclaration { .. } => { return Err(NameResolutionError::Unsupported { diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 36d9904b..d804b1e9 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -56,6 +56,15 @@ fn builtin(resolver: &NameResolver, name: &str) -> SymbolId { .expect("builtin is declared") } +/// The name of the `UnknownName` error that resolving `source` reports. +fn unknown_name(source: &str) -> String { + let mut resolver = NameResolver::new([]); + match resolve_source(&mut resolver, source) { + Err(NameResolutionError::UnknownName { name, .. }) => name, + other => panic!("expected an unknown name in {source:?}, got {other:?}"), + } +} + fn only_reference(resolver: &NameResolver) -> &Candidates { resolver .references @@ -263,7 +272,6 @@ fn loop_body_forms_resolve_names_and_restore_scope_on_errors() { #[test] fn unfinished_language_features_report_errors_instead_of_panicking() { for (source, feature) in [ - ("fn identity(x) => x;", "function declarations"), ("struct Point { x: Int }", "struct declarations"), ( "let x = 1; x += 2;", @@ -349,3 +357,84 @@ fn overloaded_builtins_share_a_group_newest_first() { .all(|&id| resolver.symbols.kind(id) == BindingKind::Function) ); } + +#[test] +fn function_names_are_visible_in_their_own_body() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn fact(n) { fact(n); }").unwrap(); + + let [fact, n] = declared(&resolver); + assert_eq!(resolver.symbols.kind(fact), BindingKind::Function); + assert_eq!(resolver.symbols.kind(n), BindingKind::Plain); + assert_eq!(uses(&resolver, fact), 1); + assert_eq!(uses(&resolver, n), 1); +} + +#[test] +fn parameters_are_only_visible_inside_the_function() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn f(x) { x; }").unwrap(); + + assert_eq!(unknown_name("fn f(x) { 1; } x;"), "x"); +} + +#[test] +fn function_overloads_share_a_group_newest_first() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn f(x) { 1; } fn f(x, y) { 1; } f;").unwrap(); + + let [f1, _, f2, _, _] = declared(&resolver); + assert_eq!(only_reference(&resolver).0, vec![vec![f2, f1]]); + assert_eq!(resolver.symbols.kind(f1), BindingKind::Function); + assert_eq!(resolver.symbols.kind(f2), BindingKind::Function); +} + +#[test] +fn lambdas_do_not_bind_a_name() { + // The initializer is resolved before `g` is declared, and the lambda has + // no name of its own. + assert_eq!(unknown_name("let g = fn(x) { g; };"), "g"); +} + +#[test] +fn named_function_expressions_bind_in_the_enclosing_scope() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let g = fn h(n) { n; }; h;").unwrap(); + + let [h, n, g] = declared(&resolver); + assert_eq!(uses(&resolver, h), 1); + assert_eq!(uses(&resolver, n), 1); + assert_eq!(uses(&resolver, g), 0); +} + +#[test] +fn function_bodies_see_enclosing_bindings() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let y = 1; fn f(x) { y; }").unwrap(); + + let [y, _, _] = declared(&resolver); + assert_eq!(uses(&resolver, y), 1); +} + +#[test] +fn duplicate_parameters_are_rejected() { + let mut resolver = NameResolver::new([]); + let source = "fn f(x, x) { 1; }"; + let error = resolve_source(&mut resolver, source).unwrap_err(); + assert_eq!(error.to_string(), "Illegal redefinition of parameter x"); + + let NameResolutionError::DuplicateParameter { name, span } = error else { + panic!("expected a duplicate parameter, got {error:?}"); + }; + assert_eq!(name, "x"); + assert_eq!(span.offset(), source.rfind('x').unwrap()); + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), None); +} + +#[test] +fn errors_in_function_bodies_restore_scope() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn f(x) { missing; }").unwrap_err(); + + assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), None); +} diff --git a/ndc_parser/src/expression.rs b/ndc_parser/src/expression.rs index 6eed9c88..1f2a1966 100644 --- a/ndc_parser/src/expression.rs +++ b/ndc_parser/src/expression.rs @@ -264,6 +264,15 @@ impl FunctionParameter { .collect(), ) } + + pub fn name(&self) -> &str { + match &self.lvalue.pattern { + BindingPattern::Identifier { identifier, .. } => identifier, + BindingPattern::Sequence(_) => { + unreachable!("a function parameter can never be a sequence") + } + } + } } #[derive(Debug, Eq, PartialEq, Clone)] From 54881093ab3a37991cb26571b4607de25928e604 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 13:10:46 +0200 Subject: [PATCH 09/19] =?UTF-8?q?fix(analyser):=20match=20the=20current=20?= =?UTF-8?q?unknown=20identifier=20message=20=F0=9F=94=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 2 +- ndc_analyser/src/name_resolution/resolver_tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index d603cb64..263f76db 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -38,7 +38,7 @@ pub(crate) struct NameResolver { #[derive(Debug, thiserror::Error)] pub(crate) enum NameResolutionError { - #[error("identifier '{name}' has not been declared")] + #[error("Identifier {name} has not previously been declared")] UnknownName { name: String, span: Span }, #[error("name resolution for {feature} is not implemented yet")] Unsupported { feature: &'static str, span: Span }, diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index d804b1e9..d03fe1d4 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -93,7 +93,7 @@ fn self_reference_reports_the_initializer_location() { let error = resolve_source(&mut resolver, source).unwrap_err(); assert_eq!( error.to_string(), - "identifier 'missing' has not been declared" + "Identifier missing has not previously been declared" ); let NameResolutionError::UnknownName { name, span } = error else { panic!("expected an unknown name"); From 6a69d6336594e92af8a55d5fb257768d1b46c823 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 13:21:31 +0200 Subject: [PATCH 10/19] =?UTF-8?q?feat(analyser):=20reject=20functions=20th?= =?UTF-8?q?at=20redefine=20variables=20in=20the=20same=20scope=20?= =?UTF-8?q?=F0=9F=9A=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scope holds a name either as plain bindings or as an overload set, so a function declared after a same-named variable in one scope is an error. Across scopes, ordinary lexical shadowing applies. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 21 ++++++- .../src/name_resolution/resolver_tests.rs | 57 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 263f76db..1443ad5f 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -40,10 +40,12 @@ pub(crate) struct NameResolver { pub(crate) enum NameResolutionError { #[error("Identifier {name} has not previously been declared")] UnknownName { name: String, span: Span }, - #[error("name resolution for {feature} is not implemented yet")] + #[error("Name resolution for {feature} is not implemented yet")] Unsupported { feature: &'static str, span: Span }, #[error("Illegal redefinition of parameter {name}")] DuplicateParameter { name: String, span: Span }, + #[error("Illegal redefinition of variable {name} as a function in the same scope")] + IllegalFunctionRedefinition { name: String, span: Span }, } /// Same-named symbols visible at a use, nearest scope first, newest first @@ -123,6 +125,12 @@ impl ScopeTree { .collect(), ) } + + /// Same-named symbols in the current scope only, newest first. Empty when + /// the name is unbound here, even if an enclosing scope binds it. + pub(crate) fn current_candidates(&self, name: &str, table: &SymbolTable) -> Vec { + self.scopes[self.current.0].candidates(name, table) + } } impl NameResolver { @@ -250,6 +258,17 @@ impl NameResolver { .. } => { if let Some(name) = name { + if self + .scopes + .current_candidates(name, &self.symbols) + .iter() + .any(|binding| self.symbols.kind(*binding) == BindingKind::Plain) + { + return Err(NameResolutionError::IllegalFunctionRedefinition { + name: name.clone(), + span: loc.span, + }); + } let symbol = self.symbols.declare(name.clone(), BindingKind::Function); self.scopes.bind(symbol); self.declarations.insert(loc.id, symbol); diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index d03fe1d4..061078bc 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -438,3 +438,60 @@ fn errors_in_function_bodies_restore_scope() { assert_eq!(resolver.scopes.lookup("x", &resolver.symbols), None); } + +#[test] +fn functions_cannot_redefine_variables_in_the_same_scope() { + let cases: [(&[&str], &str); 4] = [ + (&[], "let a = 1; fn a(x) { 1; }"), + (&[], "let a = fn(x) { 1; }; fn a(x) { 1; }"), + // The program's top level is one scope, separate from the builtins. + (&["a"], "let a = 99; fn a(x, y) { 1; }"), + // An arrow body shares the parameter scope; a block body would not. + (&[], "fn f(a) => fn a() { 1; };"), + ]; + for (builtins, source) in cases { + let mut resolver = NameResolver::new(builtins.iter().copied()); + let error = resolve_source(&mut resolver, source).expect_err(source); + assert_eq!( + error.to_string(), + "Illegal redefinition of variable a as a function in the same scope", + "{source}" + ); + let NameResolutionError::IllegalFunctionRedefinition { span, .. } = error else { + unreachable!("the message identifies the variant"); + }; + assert!(source[span.range()].starts_with("fn a"), "{source}"); + + // The rejected function binds neither its name nor its parameters. + assert!( + resolver + .scopes + .lookup("a", &resolver.symbols) + .is_none_or(|id| resolver.symbols.kind(id) == BindingKind::Plain), + "{source}" + ); + assert_eq!( + resolver.scopes.lookup("x", &resolver.symbols), + None, + "{source}" + ); + } +} + +#[test] +fn functions_may_share_names_with_variables_in_other_scopes() { + let cases: [(&[&str], &str); 6] = [ + (&[], "fn a(x) { 1; } let a = 1;"), + (&[], "let a = 1; { fn a(x) { 1; } }"), + (&[], "let a = 1; fn b() { fn a(x) { 1; } }"), + (&["len"], "fn len(x) { 1; }"), + // Rejecting identical overloads needs parameter types: the type pass's job. + (&[], "fn a(x) { 1; } fn a(x) { 1; }"), + // A block body is its own scope, separate from the parameters. + (&[], "fn f(a) { fn a() { 1; } }"), + ]; + for (builtins, source) in cases { + let mut resolver = NameResolver::new(builtins.iter().copied()); + resolve_source(&mut resolver, source).unwrap_or_else(|error| panic!("{source}: {error:?}")); + } +} From ecee3ad35b61b11499fb098b852f67d52134b63d Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 13:45:56 +0200 Subject: [PATCH 11/19] =?UTF-8?q?feat(analyser):=20record=20scope=20kinds?= =?UTF-8?q?=20=F0=9F=97=82=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scopes now know whether they hold builtins, the program's top level, a function's parameters, or a block, so later lookups can tell when they cross a function boundary. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 55 ++++++++++++------- .../src/name_resolution/resolver_tests.rs | 33 ++++++++++- 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 1443ad5f..ae1b11d7 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -9,10 +9,11 @@ use ndc_parser::{ #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] pub(crate) struct ScopeId(usize); -#[derive(Debug, Default)] +#[derive(Debug)] pub(crate) struct Scope { parent: Option, bindings: Vec, + kind: ScopeKind, } pub(crate) struct ScopeTree { @@ -53,13 +54,27 @@ pub(crate) enum NameResolutionError { #[derive(Debug)] pub(crate) struct Candidates(Vec>); +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ScopeKind { + /// Natives such as `len` and `+`. The VM keeps them as globals, so they're never captured. + Builtins, + /// The program's top level. In `let x = 1; fn f() { x }`, `x` lives here and `f` captures it. + Program, + /// The parameters of the function declared at this node, named or lambda. + Function(NodeId), + /// `{ … }` bodies and `for` iterations. Not a function boundary. + Block, +} + impl Scope { - pub(crate) fn new_with_parent(parent: ScopeId) -> Self { + pub(crate) fn new(kind: ScopeKind, parent: Option) -> Self { Self { - parent: Some(parent), + parent, bindings: Vec::default(), + kind, } } + pub(crate) fn bind(&mut self, symbol_id: SymbolId) { self.bindings.push(symbol_id); } @@ -87,7 +102,7 @@ impl ScopeTree { pub(crate) fn new() -> Self { Self { current: ScopeId(0), - scopes: vec![Scope::default()], + scopes: vec![Scope::new(ScopeKind::Builtins, None)], } } @@ -106,8 +121,8 @@ impl ScopeTree { self.chain().find_map(|scope| scope.lookup(name, table)) } - pub(crate) fn enter_scope(&mut self) { - self.scopes.push(Scope::new_with_parent(self.current)); + pub(crate) fn enter_scope(&mut self, kind: ScopeKind) { + self.scopes.push(Scope::new(kind, Some(self.current))); self.current = ScopeId(self.scopes.len() - 1); } @@ -146,7 +161,7 @@ impl NameResolver { scopes.bind(symbol); } - scopes.enter_scope(); + scopes.enter_scope(ScopeKind::Program); Self { symbols, scopes, @@ -186,7 +201,7 @@ impl NameResolver { self.resolve(r_value)?; } Expression::Block { statements } => { - self.in_scope(|resolver| { + self.in_scope(ScopeKind::Block, |resolver| { for statement in statements { resolver.resolve(statement)?; } @@ -276,7 +291,7 @@ impl NameResolver { // Parameter lists are short, so a linear scan beats hashing. let mut names = Vec::new(); - self.in_scope(|resolver| { + self.in_scope(ScopeKind::Function(loc.id), |resolver| { for parameter in parameters { let param_name = parameter.name(); if names.contains(¶m_name) { @@ -374,9 +389,10 @@ impl NameResolver { fn in_scope( &mut self, + kind: ScopeKind, resolve: impl FnOnce(&mut Self) -> Result<(), NameResolutionError>, ) -> Result<(), NameResolutionError> { - self.scopes.enter_scope(); + self.scopes.enter_scope(kind); let result = resolve(self); // Restore visibility on errors as well as successful traversal. self.scopes.leave_scope(); @@ -393,7 +409,7 @@ impl NameResolver { // A binder is not visible in its own iterable. Later // iterables and guards can see the earlier binders. self.resolve(sequence)?; - self.in_scope(|resolver| { + self.in_scope(ScopeKind::Block, |resolver| { resolver.declare_pattern(l_value); resolver.resolve_for(tail, body) }) @@ -428,8 +444,7 @@ mod resolver_tests; #[cfg(test)] mod test { - use crate::name_resolution::Scope; - use crate::name_resolution::ScopeTree; + use crate::name_resolution::{Scope, ScopeKind, ScopeTree}; use crate::symbols::{BindingKind, SymbolTable}; #[test] @@ -439,7 +454,7 @@ mod test { let second = table.declare("foo".to_string(), BindingKind::Plain); let other = table.declare("another".to_string(), BindingKind::Plain); - let mut scope = Scope::default(); + let mut scope = Scope::new(ScopeKind::Block, None); scope.bind(first); scope.bind(second); scope.bind(other); @@ -462,9 +477,9 @@ mod test { let mut tree = ScopeTree::new(); tree.bind(first); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); tree.bind(middle); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); assert_eq!(tree.lookup("foo", &table), Some(first)); assert_eq!(tree.lookup("bar", &table), Some(middle)); assert_eq!(tree.lookup("missing", &table), None); @@ -484,7 +499,7 @@ mod test { let mut tree = ScopeTree::new(); tree.bind(first); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); tree.bind(second); assert_eq!(tree.lookup("foo", &table), Some(second)); @@ -499,12 +514,12 @@ mod test { let second = table.declare("second".to_string(), BindingKind::Plain); let mut tree = ScopeTree::new(); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); tree.bind(first); tree.leave_scope(); assert_eq!(tree.lookup("first", &table), None); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); assert_eq!(tree.lookup("first", &table), None); tree.bind(second); assert_eq!(tree.lookup("second", &table), Some(second)); @@ -521,7 +536,7 @@ mod test { let mut tree = ScopeTree::new(); tree.bind(outer); - tree.enter_scope(); + tree.enter_scope(ScopeKind::Block); assert_eq!(tree.lookup("outer", &table), Some(outer)); assert_eq!(tree.lookup("missing", &table), None); diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 061078bc..945751dd 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,4 +1,4 @@ -use super::{Candidates, NameResolutionError, NameResolver}; +use super::{Candidates, NameResolutionError, NameResolver, ScopeKind}; use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; @@ -495,3 +495,34 @@ fn functions_may_share_names_with_variables_in_other_scopes() { resolve_source(&mut resolver, source).unwrap_or_else(|error| panic!("{source}: {error:?}")); } } + +#[test] +fn scopes_record_their_kind_in_creation_order() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn f(x) { 1; } for i in [1] { 1; }").unwrap(); + + let [f, _, _] = declared(&resolver); + let f_node = resolver + .declarations + .iter() + .find_map(|(&node, &symbol)| (symbol == f).then_some(node)) + .expect("f is declared"); + assert_eq!( + resolver + .scopes + .scopes + .iter() + .map(|scope| scope.kind) + .collect_vec(), + vec![ + ScopeKind::Builtins, + ScopeKind::Program, + ScopeKind::Function(f_node), + // The function body. + ScopeKind::Block, + // The `for` iteration, then the loop body. + ScopeKind::Block, + ScopeKind::Block, + ] + ); +} From 0cc5cecbc992747575c3e155efd49824af14c510 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 14:15:58 +0200 Subject: [PATCH 12/19] =?UTF-8?q?feat(analyser):=20record=20closure=20capt?= =?UTF-8?q?ures=20=F0=9F=AA=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each function records the outer symbols it captures, including ones it only passes through to an inner function, and the resolver collects every captured symbol so the compiler can keep them alive past their scope (#227). Every overload candidate is captured until the type pass narrows them. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 70 +++++++- .../src/name_resolution/resolver_tests.rs | 156 +++++++++++++++++- 2 files changed, 221 insertions(+), 5 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index ae1b11d7..e603e84a 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -1,5 +1,5 @@ use crate::symbols::{BindingKind, SymbolId, SymbolTable}; -use ahash::AHashMap; +use ahash::{AHashMap, AHashSet}; use ndc_lexer::Span; use ndc_parser::{ AssignmentTarget, AssignmentTargetLocation, BindingPattern, BindingPatternLocation, Expression, @@ -19,6 +19,7 @@ pub(crate) struct Scope { pub(crate) struct ScopeTree { current: ScopeId, scopes: Vec, + bound_in: AHashMap, } pub(crate) struct NameResolver { @@ -35,6 +36,11 @@ pub(crate) struct NameResolver { /// The symbol each identifier assignment target writes to, keyed by the /// target's node. In `x = 1`, the `x` node. assignments: AHashMap, + /// The outer symbols each function captures, keyed by its declaration node, in order of first use. + /// In `fn outer() { let x = 1; fn inner() { x } }`, `inner` captures `[x]`. + captures: AHashMap>, + /// Symbols some closure captures. They must outlive their scope (see #227). + captured: AHashSet, } #[derive(Debug, thiserror::Error)] @@ -103,18 +109,42 @@ impl ScopeTree { Self { current: ScopeId(0), scopes: vec![Scope::new(ScopeKind::Builtins, None)], + bound_in: Default::default(), } } pub(crate) fn bind(&mut self, symbol_id: SymbolId) { self.scopes[self.current.0].bind(symbol_id); + self.bound_in.insert(symbol_id, self.current); } /// The current scope followed by its ancestors, ending at the root. fn chain(&self) -> impl Iterator { - std::iter::successors(Some(&self.scopes[self.current.0]), |scope| { - scope.parent.map(|parent| &self.scopes[parent.0]) - }) + self.ancestors(self.current).map(|id| &self.scopes[id.0]) + } + + /// `from` and its ancestors, ending at the root. + fn ancestors(&self, from: ScopeId) -> impl Iterator { + std::iter::successors(Some(from), |id| self.scopes[id.0].parent) + } + + /// Functions a use of `symbol` from the current scope reaches through, innermost + /// first. In `fn f() { let y = 1; fn g() { y } }`, a use of `y` in `g` crosses `[g]`. + /// Natives are globals, so uses of them cross nothing. + pub(crate) fn crossed_functions(&self, symbol: SymbolId) -> Vec { + let bound_in = self.bound_in[&symbol]; + + if self.scopes[bound_in.0].kind == ScopeKind::Builtins { + return Vec::new(); + } + + self.ancestors(self.current) + .take_while(|&id| id != bound_in) + .filter_map(|id| match self.scopes[id.0].kind { + ScopeKind::Function(node) => Some(node), + _ => None, + }) + .collect() } pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { @@ -168,9 +198,30 @@ impl NameResolver { declarations: Default::default(), references: Default::default(), assignments: Default::default(), + captures: Default::default(), + captured: Default::default(), } } + /// Records a use of `symbol` from the current scope. Every function the use + /// reaches through captures it. In `fn outer() { let x = 1; fn inner() { x } }`, + /// the use of `x` adds `x` to `inner`'s captures. + fn record_use(&mut self, symbol: SymbolId) { + let crossed = self.scopes.crossed_functions(symbol); + if crossed.is_empty() { + return; + } + + for node in crossed { + let captures = self.captures.entry(node).or_default(); + if !captures.contains(&symbol) { + captures.push(symbol); + } + } + + self.captured.insert(symbol); + } + /// Resolve lexical value names. Type annotations and member selection /// belong to later passes; this does not validate types or control flow. pub(crate) fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { @@ -182,6 +233,10 @@ impl NameResolver { | Expression::Continue => {} Expression::Identifier { name, .. } => { let candidates = self.candidates(name, loc.span)?; + candidates + .iter() + .flatten() + .for_each(|symbol| self.record_use(*symbol)); self.references.insert(loc.id, candidates); } Expression::Statement(inner) @@ -371,6 +426,7 @@ impl NameResolver { identifier, span, .. } => { let symbol = self.lookup(identifier, *span)?; + self.record_use(symbol); self.assignments.insert(target.id, symbol); } AssignmentTarget::Sequence(items) => { @@ -439,6 +495,12 @@ impl NameResolver { } } +impl Candidates { + fn iter(&self) -> impl Iterator> { + self.0.iter() + } +} + #[cfg(test)] mod resolver_tests; diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 945751dd..6f8071ee 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -2,7 +2,7 @@ use super::{Candidates, NameResolutionError, NameResolver, ScopeKind}; use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; -use ndc_parser::Parser; +use ndc_parser::{NodeId, Parser}; fn resolve_source(resolver: &mut NameResolver, source: &str) -> Result<(), NameResolutionError> { let tokens = Lexer::new(source, SourceId::new(0)) @@ -65,6 +65,24 @@ fn unknown_name(source: &str) -> String { } } +/// Function scope nodes in creation order. Lambdas have no declaration entry, +/// so the scope arena is the one place every function's node appears. +fn functions(resolver: &NameResolver) -> Vec { + resolver + .scopes + .scopes + .iter() + .filter_map(|scope| match scope.kind { + ScopeKind::Function(node) => Some(node), + _ => None, + }) + .collect() +} + +fn captures_of(resolver: &NameResolver, function: NodeId) -> &[SymbolId] { + resolver.captures.get(&function).map_or(&[], Vec::as_slice) +} + fn only_reference(resolver: &NameResolver) -> &Candidates { resolver .references @@ -526,3 +544,139 @@ fn scopes_record_their_kind_in_creation_order() { ] ); } + +#[test] +fn functions_capture_outer_bindings() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let x = 1; fn f() { x; }").unwrap(); + + let [x, _] = declared(&resolver); + let [f] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + assert_eq!(captures_of(&resolver, f), [x]); + assert_eq!(resolver.captured, [x].into_iter().collect()); +} + +#[test] +fn blocks_are_not_function_boundaries() { + for source in ["let x = 1; { { x; } }", "fn f(a) { { a; } }"] { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, source).unwrap(); + + assert!(resolver.captures.is_empty(), "{source}"); + assert!(resolver.captured.is_empty(), "{source}"); + } +} + +#[test] +fn captures_pass_through_intermediate_functions() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "fn outer() { let x = 1; fn middle() { fn inner() { x; } } }", + ) + .unwrap(); + + let [_, x, _, _] = declared(&resolver); + let [outer, middle, inner] = functions(&resolver)[..] else { + panic!("expected three functions"); + }; + assert_eq!(captures_of(&resolver, outer), []); + assert_eq!(captures_of(&resolver, middle), [x]); + assert_eq!(captures_of(&resolver, inner), [x]); +} + +#[test] +fn lambdas_capture_under_their_own_node() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let x = 1; let g = fn() { x; };").unwrap(); + + let [x, _] = declared(&resolver); + let [lambda] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + assert_eq!(captures_of(&resolver, lambda), [x]); +} + +#[test] +fn builtins_are_never_captured() { + let mut resolver = NameResolver::new(["len"]); + resolve_source(&mut resolver, "fn f() { len; }").unwrap(); + + assert!(resolver.captures.is_empty()); + assert!(resolver.captured.is_empty()); +} + +#[test] +fn assignments_capture_their_target() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let x = 1; fn f() { x = 2; }").unwrap(); + + let [x, _] = declared(&resolver); + let [f] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + assert_eq!(captures_of(&resolver, f), [x]); + assert!(resolver.captured.contains(&x)); +} + +#[test] +fn every_candidate_is_captured() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "fn outer() { fn h(a) { 1; } fn h(a, b) { 1; } fn inner() { h; } }", + ) + .unwrap(); + + // The type pass picks the overload later, so the closure captures both. + let [_, h1, _, h2, _, _, _] = declared(&resolver); + let [_, _, _, inner] = functions(&resolver)[..] else { + panic!("expected four functions"); + }; + assert_eq!(captures_of(&resolver, inner), [h2, h1]); +} + +#[test] +fn recursive_functions_capture_themselves() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn fact(n) { fact(n); }").unwrap(); + + let [fact, _] = declared(&resolver); + let [fact_node] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + assert_eq!(captures_of(&resolver, fact_node), [fact]); +} + +#[test] +fn captures_are_listed_once_in_first_use_order() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let a = 1; let b = 2; fn f() { b; a; b; }").unwrap(); + + let [a, b, _] = declared(&resolver); + let [f] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + assert_eq!(captures_of(&resolver, f), [b, a]); +} + +#[test] +fn loop_variables_can_be_captured() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "for i in [1] { let g = fn() { i; }; }").unwrap(); + + let [i, _] = declared(&resolver); + assert!(resolver.captured.contains(&i)); +} + +#[test] +fn block_scoped_variables_can_be_captured() { + // The shape from #227: `y` must outlive its block for `g` to read it. + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let g = 0; { let y = 1; g = fn() { y; }; }").unwrap(); + + let [_, y] = declared(&resolver); + assert!(resolver.captured.contains(&y)); +} From 5df08466dab064a0b8cb4ac6dccd9aa996e8be85 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 14:45:28 +0200 Subject: [PATCH 13/19] =?UTF-8?q?feat(analyser):=20resolve=20augmented=20a?= =?UTF-8?q?ssignment=20operators=20=E2=9E=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `x op= y` records candidates for both the in-place `op=` and the plain `op` under the assignment's node, leaving the choice to the type pass. Sequence targets keep the current analyser's error message. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 61 +++++++++- .../src/name_resolution/resolver_tests.rs | 109 +++++++++++++++--- 2 files changed, 149 insertions(+), 21 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index e603e84a..9cb23749 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -41,6 +41,9 @@ pub(crate) struct NameResolver { captures: AHashMap>, /// Symbols some closure captures. They must outlive their scope (see #227). captured: AHashSet, + /// The operations each augmented assignment can use, keyed by its node. + /// In `x += 1`, the candidates for `+=` and `+`. + augmented: AHashMap, } #[derive(Debug, thiserror::Error)] @@ -53,13 +56,26 @@ pub(crate) enum NameResolutionError { DuplicateParameter { name: String, span: Span }, #[error("Illegal redefinition of variable {name} as a function in the same scope")] IllegalFunctionRedefinition { name: String, span: Span }, + #[error("This lvalue is required to be a single identifier")] + SequenceTargetNotAllowed { span: Span }, } /// Same-named symbols visible at a use, nearest scope first, newest first /// within a scope. For `x` in `let x = 1; { let x = 2; x }`: `[[x2], [x1]]`. +/// Identifiers always have at least one candidate, augmented operators may +/// not. #[derive(Debug)] pub(crate) struct Candidates(Vec>); +/// The two operations an augmented assignment can use. For `xs ++= ys`: +/// `in_place` holds `++=` (mutates `xs`), `operator` holds `++` (builds a new value +/// that is written back). Either may be empty; the type pass picks. +#[derive(Debug)] +pub(crate) struct AugmentedCandidates { + in_place: Candidates, + operator: Candidates, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum ScopeKind { /// Natives such as `len` and `+`. The VM keeps them as globals, so they're never captured. @@ -200,6 +216,7 @@ impl NameResolver { assignments: Default::default(), captures: Default::default(), captured: Default::default(), + augmented: Default::default(), } } @@ -368,11 +385,45 @@ impl NameResolver { span: loc.span, }); } - Expression::OpAssignment { .. } => { - return Err(NameResolutionError::Unsupported { - feature: "augmented assignment operator selection", - span: loc.span, - }); + Expression::OpAssignment { + l_value, + r_value, + operation, + .. + } => { + if let AssignmentTarget::Sequence(_) = l_value.target { + return Err(NameResolutionError::SequenceTargetNotAllowed { span: loc.span }); + } + + self.resolve_target(l_value)?; + self.resolve(r_value)?; + + let in_place_op_name = format!("{operation}="); + // Ask the scope tree directly: an empty list isn't an error here. + let in_place_candidates = self.scopes.candidates(&in_place_op_name, &self.symbols); + let candidates = self.scopes.candidates(operation, &self.symbols); + + if in_place_candidates.0.is_empty() && candidates.0.is_empty() { + return Err(NameResolutionError::UnknownName { + name: operation.clone(), + // The operator has no span of its own, so point at the whole assignment. + span: loc.span, + }); + } + + in_place_candidates + .iter() + .chain(candidates.iter()) + .flatten() + .for_each(|op| self.record_use(*op)); + + self.augmented.insert( + loc.id, + AugmentedCandidates { + in_place: in_place_candidates, + operator: candidates, + }, + ); } } Ok(()) diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 6f8071ee..1087486c 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,4 +1,4 @@ -use super::{Candidates, NameResolutionError, NameResolver, ScopeKind}; +use super::{AugmentedCandidates, Candidates, NameResolutionError, NameResolver, ScopeKind}; use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; @@ -83,6 +83,14 @@ fn captures_of(resolver: &NameResolver, function: NodeId) -> &[SymbolId] { resolver.captures.get(&function).map_or(&[], Vec::as_slice) } +fn only_augmented(resolver: &NameResolver) -> &AugmentedCandidates { + resolver + .augmented + .values() + .exactly_one() + .expect("exactly one augmented assignment") +} + fn only_reference(resolver: &NameResolver) -> &Candidates { resolver .references @@ -288,21 +296,13 @@ fn loop_body_forms_resolve_names_and_restore_scope_on_errors() { } #[test] -fn unfinished_language_features_report_errors_instead_of_panicking() { - for (source, feature) in [ - ("struct Point { x: Int }", "struct declarations"), - ( - "let x = 1; x += 2;", - "augmented assignment operator selection", - ), - ] { - let mut resolver = NameResolver::new([]); - let error = resolve_source(&mut resolver, source).unwrap_err(); - assert!( - matches!(error, NameResolutionError::Unsupported { feature: found, span } if found == feature && span.source_id() == SourceId::new(0)), - "{error:?}" - ); - } +fn struct_declarations_report_errors_instead_of_panicking() { + let mut resolver = NameResolver::new([]); + let error = resolve_source(&mut resolver, "struct Point { x: Int }").unwrap_err(); + assert!( + matches!(error, NameResolutionError::Unsupported { feature: "struct declarations", span } if span.source_id() == SourceId::new(0)), + "{error:?}" + ); } #[test] @@ -680,3 +680,80 @@ fn block_scoped_variables_can_be_captured() { let [_, y] = declared(&resolver); assert!(resolver.captured.contains(&y)); } + +#[test] +fn augmented_assignment_finds_the_plain_operator() { + let mut resolver = NameResolver::new(["+"]); + let plus = builtin(&resolver, "+"); + resolve_source(&mut resolver, "let x = 1; x += 2;").unwrap(); + + let [x] = declared(&resolver); + let augmented = only_augmented(&resolver); + assert!(augmented.in_place.0.is_empty()); + assert_eq!(augmented.operator.0, vec![vec![plus]]); + assert_eq!( + resolver.assignments.values().copied().collect_vec(), + vec![x] + ); +} + +#[test] +fn augmented_assignment_finds_both_operations() { + let mut resolver = NameResolver::new(["++", "++="]); + let concat = builtin(&resolver, "++"); + let append = builtin(&resolver, "++="); + resolve_source(&mut resolver, "let xs = []; xs ++= [1];").unwrap(); + + let augmented = only_augmented(&resolver); + assert_eq!(augmented.in_place.0, vec![vec![append]]); + assert_eq!(augmented.operator.0, vec![vec![concat]]); +} + +#[test] +fn augmented_assignment_resolves_index_and_member_targets() { + let mut resolver = NameResolver::new(["+"]); + resolve_source(&mut resolver, "let xs = [1]; let i = 0; xs[i] += 1;").unwrap(); + let [xs, i] = declared(&resolver); + assert_eq!(uses(&resolver, xs), 1); + assert_eq!(uses(&resolver, i), 1); + + // The member name itself is resolved in a later step; only the receiver is here. + let mut resolver = NameResolver::new(["+"]); + resolve_source(&mut resolver, "let p = (); p.x += 1;").unwrap(); + let [p] = declared(&resolver); + assert_eq!(uses(&resolver, p), 1); +} + +#[test] +fn augmented_assignment_rejects_sequence_targets() { + let mut resolver = NameResolver::new(["+"]); + let error = resolve_source(&mut resolver, "let a = 1; let b = 2; (a, b) += 1;").unwrap_err(); + + assert_eq!( + error.to_string(), + "This lvalue is required to be a single identifier" + ); + assert!( + matches!(error, NameResolutionError::SequenceTargetNotAllowed { .. }), + "{error:?}" + ); + assert!(resolver.augmented.is_empty()); +} + +#[test] +fn augmented_assignment_without_any_operator_is_unknown() { + assert_eq!(unknown_name("let x = 1; x += 2;"), "+"); +} + +#[test] +fn augmented_assignment_captures_its_target() { + let mut resolver = NameResolver::new(["+"]); + resolve_source(&mut resolver, "let x = 1; fn f() { x += 1; }").unwrap(); + + let [x, _] = declared(&resolver); + let [f] = functions(&resolver)[..] else { + panic!("expected one function"); + }; + // The native `+` is a global, so only `x` is captured. + assert_eq!(captures_of(&resolver, f), [x]); +} From cf57e635df7d57eeb6af6b0ec221cba89231c07a Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 15:30:40 +0200 Subject: [PATCH 14/19] =?UTF-8?q?feat(analyser):=20resolve=20getters=20and?= =?UTF-8?q?=20setters=20behind=20member=20and=20index=20access=20?= =?UTF-8?q?=F0=9F=94=91?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `p.x` records candidates for the getter `x`, member and index targets record their setter (`x=` or `[]=`), and augmented assignment to them also records the getter it reads through. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 68 ++++++++-- .../src/name_resolution/resolver_tests.rs | 127 +++++++++++++++++- 2 files changed, 179 insertions(+), 16 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 9cb23749..41f5c384 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -44,6 +44,12 @@ pub(crate) struct NameResolver { /// The operations each augmented assignment can use, keyed by its node. /// In `x += 1`, the candidates for `+=` and `+`. augmented: AHashMap, + /// Getter candidates for reads through an accessor, keyed by the reading node. + /// For `p.x`, the member access node's `x`. For `xs[i] += 1`, the target node's `[]`. + getters: AHashMap, + /// Setter candidates for writes through an accessor, keyed by the target node. + /// In `p.x = 1`, `x=`. In `xs[i] = 1`, `[]=`. + setters: AHashMap, } #[derive(Debug, thiserror::Error)] @@ -217,6 +223,8 @@ impl NameResolver { captures: Default::default(), captured: Default::default(), augmented: Default::default(), + getters: Default::default(), + setters: Default::default(), } } @@ -249,11 +257,7 @@ impl NameResolver { | Expression::Break | Expression::Continue => {} Expression::Identifier { name, .. } => { - let candidates = self.candidates(name, loc.span)?; - candidates - .iter() - .flatten() - .for_each(|symbol| self.record_use(*symbol)); + let candidates = self.use_name(name, loc.span)?; self.references.insert(loc.id, candidates); } Expression::Statement(inner) @@ -312,7 +316,16 @@ impl NameResolver { self.resolve(argument)?; } } - Expression::MemberAccess { receiver, .. } => self.resolve(receiver)?, + Expression::MemberAccess { + receiver, + member, + member_span, + .. + } => { + self.resolve(receiver)?; + let candidates = self.use_name(member, *member_span)?; + self.getters.insert(loc.id, candidates); + } Expression::Tuple { values } | Expression::List { values } => { for value in values { self.resolve(value)?; @@ -396,6 +409,20 @@ impl NameResolver { } self.resolve_target(l_value)?; + // Reading the old value goes through the target's getter. + // Identifiers are read directly, and sequences were rejected above. + let getter = match &l_value.target { + AssignmentTarget::Index { .. } => Some(self.use_name("[]", l_value.span)?), + AssignmentTarget::Member { + member, + member_span, + .. + } => Some(self.use_name(member, *member_span)?), + AssignmentTarget::Identifier { .. } | AssignmentTarget::Sequence(_) => None, + }; + if let Some(getter) = getter { + self.getters.insert(l_value.id, getter); + } self.resolve(r_value)?; let in_place_op_name = format!("{operation}="); @@ -429,17 +456,25 @@ impl NameResolver { Ok(()) } - fn candidates(&self, name: &str, span: Span) -> Result { - let out = self.scopes.candidates(name, &self.symbols); + /// Resolves a use of `name` from the current scope: every candidate it could + /// mean, each recorded as a use so closures capture it. Any candidate may be + /// the one the type pass picks, so all of them are captured. + fn use_name(&mut self, name: &str, span: Span) -> Result { + let candidates = self.scopes.candidates(name, &self.symbols); - if out.0.is_empty() { + if candidates.0.is_empty() { return Err(NameResolutionError::UnknownName { name: name.to_string(), span, }); } - Ok(out) + candidates + .iter() + .flatten() + .for_each(|symbol| self.record_use(*symbol)); + + Ok(candidates) } fn lookup(&self, name: &str, span: Span) -> Result { @@ -488,8 +523,19 @@ impl NameResolver { AssignmentTarget::Index { value, index, .. } => { self.resolve(value)?; self.resolve(index)?; + let candidates = self.use_name("[]=", target.span)?; + self.setters.insert(target.id, candidates); + } + AssignmentTarget::Member { + receiver, + member, + member_span, + .. + } => { + self.resolve(receiver)?; + let candidates = self.use_name(&format!("{member}="), *member_span)?; + self.setters.insert(target.id, candidates); } - AssignmentTarget::Member { receiver, .. } => self.resolve(receiver)?, } Ok(()) } diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 1087486c..bb9d4652 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -91,6 +91,22 @@ fn only_augmented(resolver: &NameResolver) -> &AugmentedCandidates { .expect("exactly one augmented assignment") } +fn only_getter(resolver: &NameResolver) -> &Candidates { + resolver + .getters + .values() + .exactly_one() + .expect("exactly one getter lookup") +} + +fn only_setter(resolver: &NameResolver) -> &Candidates { + resolver + .setters + .values() + .exactly_one() + .expect("exactly one setter lookup") +} + fn only_reference(resolver: &NameResolver) -> &Candidates { resolver .references @@ -166,7 +182,8 @@ fn assignment_reuses_declared_symbols() { #[test] fn member_and_index_targets_resolve_receivers_without_declaring_names() { - let mut resolver = NameResolver::new([]); + let mut resolver = NameResolver::new(["field", "field=", "[]="]); + let field = builtin(&resolver, "field"); resolve_source( &mut resolver, "let object = (); let i = 0; object.field = i; object[i] = i; object.field;", @@ -176,7 +193,11 @@ fn member_and_index_targets_resolve_receivers_without_declaring_names() { let [object, i] = declared(&resolver); assert_eq!(uses(&resolver, i), 3); assert_eq!(uses(&resolver, object), 3); - assert_eq!(resolver.scopes.lookup("field", &resolver.symbols), None); + // `field` still means the native getter: the targets declared nothing new. + assert_eq!( + resolver.scopes.lookup("field", &resolver.symbols), + Some(field) + ); assert_eq!( resolver.scopes.lookup("object", &resolver.symbols), Some(object) @@ -711,14 +732,13 @@ fn augmented_assignment_finds_both_operations() { #[test] fn augmented_assignment_resolves_index_and_member_targets() { - let mut resolver = NameResolver::new(["+"]); + let mut resolver = NameResolver::new(["+", "[]", "[]="]); resolve_source(&mut resolver, "let xs = [1]; let i = 0; xs[i] += 1;").unwrap(); let [xs, i] = declared(&resolver); assert_eq!(uses(&resolver, xs), 1); assert_eq!(uses(&resolver, i), 1); - // The member name itself is resolved in a later step; only the receiver is here. - let mut resolver = NameResolver::new(["+"]); + let mut resolver = NameResolver::new(["+", "x", "x="]); resolve_source(&mut resolver, "let p = (); p.x += 1;").unwrap(); let [p] = declared(&resolver); assert_eq!(uses(&resolver, p), 1); @@ -757,3 +777,100 @@ fn augmented_assignment_captures_its_target() { // The native `+` is a global, so only `x` is captured. assert_eq!(captures_of(&resolver, f), [x]); } + +#[test] +fn natives_resolve_as_properties() { + let mut resolver = NameResolver::new(["len"]); + let len = builtin(&resolver, "len"); + resolve_source(&mut resolver, "[1].len;").unwrap(); + + assert_eq!(only_getter(&resolver).0, vec![vec![len]]); +} + +#[test] +fn user_functions_resolve_as_properties() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn double(x) { x; } 5.double;").unwrap(); + + let [double, _] = declared(&resolver); + assert_eq!(only_getter(&resolver).0, vec![vec![double]]); +} + +#[test] +fn getters_keep_same_named_variables_as_candidates() { + // The type pass skips the variable because it can't be called. + let mut resolver = NameResolver::new(["x"]); + let native = builtin(&resolver, "x"); + resolve_source(&mut resolver, "let p = 1; let x = 99; p.x;").unwrap(); + + let [_, local] = declared(&resolver); + assert_eq!(only_getter(&resolver).0, vec![vec![local], vec![native]]); +} + +#[test] +fn getters_are_keyed_by_the_member_access_node() { + let mut resolver = NameResolver::new(["b", "c"]); + resolve_source(&mut resolver, "let a = 1; a.b.c;").unwrap(); + + // One lookup per member access; none of them under the receiver `a`. + assert_eq!(resolver.getters.len(), 2); + assert!( + resolver + .getters + .keys() + .all(|node| !resolver.references.contains_key(node)) + ); +} + +#[test] +fn member_writes_resolve_the_setter_only() { + let mut resolver = NameResolver::new(["x="]); + let setter = builtin(&resolver, "x="); + resolve_source(&mut resolver, "let p = 1; p.x = 5;").unwrap(); + + assert_eq!(only_setter(&resolver).0, vec![vec![setter]]); + assert!(resolver.getters.is_empty()); +} + +#[test] +fn index_writes_resolve_the_setter_only() { + let mut resolver = NameResolver::new(["[]="]); + let setter = builtin(&resolver, "[]="); + resolve_source(&mut resolver, "let xs = [1]; xs[0] = 2;").unwrap(); + + assert_eq!(only_setter(&resolver).0, vec![vec![setter]]); + assert!(resolver.getters.is_empty()); +} + +#[test] +fn augmented_member_assignment_resolves_getter_and_setter() { + let mut resolver = NameResolver::new(["x", "x=", "+"]); + let getter = builtin(&resolver, "x"); + let setter = builtin(&resolver, "x="); + resolve_source(&mut resolver, "let p = 1; p.x += 1;").unwrap(); + + assert_eq!(only_getter(&resolver).0, vec![vec![getter]]); + assert_eq!(only_setter(&resolver).0, vec![vec![setter]]); + // Both belong to the same target node. + assert_eq!( + resolver.getters.keys().collect_vec(), + resolver.setters.keys().collect_vec() + ); +} + +#[test] +fn augmented_index_assignment_resolves_getter_and_setter() { + let mut resolver = NameResolver::new(["[]", "[]=", "+"]); + let getter = builtin(&resolver, "[]"); + let setter = builtin(&resolver, "[]="); + resolve_source(&mut resolver, "let xs = [1]; xs[0] += 1;").unwrap(); + + assert_eq!(only_getter(&resolver).0, vec![vec![getter]]); + assert_eq!(only_setter(&resolver).0, vec![vec![setter]]); +} + +#[test] +fn unknown_accessors_are_unknown_names() { + assert_eq!(unknown_name("let p = 1; p.nope;"), "nope"); + assert_eq!(unknown_name("let p = 1; p.x = 5;"), "x="); +} From 14ce1aa8c46c5c7a7784e7630315c14f44134bbe Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 15:48:48 +0200 Subject: [PATCH 15/19] =?UTF-8?q?feat(analyser):=20resolve=20type=20names?= =?UTF-8?q?=20in=20annotations=20=F0=9F=8F=B7=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type names live in their own namespace beside values, so `let Int = 5` doesn't hide the type `Int`. Every name in a `let`, parameter, return or cast annotation resolves through lexical scopes to a type symbol, or reports the current analyser's unknown type message. Built-in type names are declared in the builtins scope. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 115 ++++++++++++++++-- .../src/name_resolution/resolver_tests.rs | 96 +++++++++++++++ ndc_analyser/src/symbols.rs | 2 + 3 files changed, 202 insertions(+), 11 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 41f5c384..c1967a0c 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -1,9 +1,10 @@ use crate::symbols::{BindingKind, SymbolId, SymbolTable}; use ahash::{AHashMap, AHashSet}; +use ndc_core::StaticType; use ndc_lexer::Span; use ndc_parser::{ AssignmentTarget, AssignmentTargetLocation, BindingPattern, BindingPatternLocation, Expression, - ExpressionLocation, ForBody, ForIteration, NodeId, + ExpressionLocation, ForBody, ForIteration, NodeId, TypeExpr, }; #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] @@ -13,6 +14,8 @@ pub(crate) struct ScopeId(usize); pub(crate) struct Scope { parent: Option, bindings: Vec, + /// Type names, kept apart from `bindings`: `let Int = 5` doesn't hide the type `Int`. + types: Vec, kind: ScopeKind, } @@ -50,6 +53,9 @@ pub(crate) struct NameResolver { /// Setter candidates for writes through an accessor, keyed by the target node. /// In `p.x = 1`, `x=`. In `xs[i] = 1`, `[]=`. setters: AHashMap, + /// The type each name in an annotation refers to, keyed by the name's node. + /// In `let m: Map`, the `Map`, `String` and `P` nodes. + type_names: AHashMap, } #[derive(Debug, thiserror::Error)] @@ -64,6 +70,8 @@ pub(crate) enum NameResolutionError { IllegalFunctionRedefinition { name: String, span: Span }, #[error("This lvalue is required to be a single identifier")] SequenceTargetNotAllowed { span: Span }, + #[error("unknown type `{name}`")] + UnknownType { name: String, span: Span }, } /// Same-named symbols visible at a use, nearest scope first, newest first @@ -99,6 +107,7 @@ impl Scope { Self { parent, bindings: Vec::default(), + types: Vec::default(), kind, } } @@ -107,13 +116,24 @@ impl Scope { self.bindings.push(symbol_id); } + pub(crate) fn bind_type(&mut self, symbol_id: SymbolId) { + self.types.push(symbol_id); + } + pub(crate) fn lookup(&self, name: &str, table: &SymbolTable) -> Option { - for id in self.bindings.iter().rev() { - if table.name(*id) == name { - return Some(*id); - } - } - None + Self::newest(&self.bindings, name, table) + } + + pub(crate) fn lookup_type(&self, name: &str, table: &SymbolTable) -> Option { + Self::newest(&self.types, name, table) + } + + fn newest(symbols: &[SymbolId], name: &str, table: &SymbolTable) -> Option { + symbols + .iter() + .rev() + .copied() + .find(|&id| table.name(id) == name) } fn candidates(&self, name: &str, table: &SymbolTable) -> Vec { @@ -140,6 +160,12 @@ impl ScopeTree { self.bound_in.insert(symbol_id, self.current); } + /// Binds a type name in the current scope. Types aren't runtime values, so + /// they're never captured and need no `bound_in` entry. + pub(crate) fn bind_type(&mut self, symbol_id: SymbolId) { + self.scopes[self.current.0].bind_type(symbol_id); + } + /// The current scope followed by its ancestors, ending at the root. fn chain(&self) -> impl Iterator { self.ancestors(self.current).map(|id| &self.scopes[id.0]) @@ -173,6 +199,12 @@ impl ScopeTree { self.chain().find_map(|scope| scope.lookup(name, table)) } + /// The innermost type named `name`. Types never overload, so there's at most one. + pub(crate) fn lookup_type(&self, name: &str, table: &SymbolTable) -> Option { + self.chain() + .find_map(|scope| scope.lookup_type(name, table)) + } + pub(crate) fn enter_scope(&mut self, kind: ScopeKind) { self.scopes.push(Scope::new(kind, Some(self.current))); self.current = ScopeId(self.scopes.len() - 1); @@ -204,6 +236,7 @@ impl NameResolver { /// Creates a new name resolver with the supplied `builtins` declared in a scope outside /// the program. With `new(["len"])`, a top-level `let len = 99` shadows the native /// instead of sharing its scope, so a later `len` resolves to `[[let len], [native len]]`. + /// The built-in type names (`Int`, `List`, …) are declared there too. pub(crate) fn new<'a>(builtins: impl IntoIterator) -> Self { let mut scopes = ScopeTree::new(); let mut symbols = SymbolTable::default(); @@ -212,6 +245,10 @@ impl NameResolver { let symbol = symbols.declare(builtin.to_string(), BindingKind::Function); scopes.bind(symbol); } + for name in StaticType::BUILTIN_TYPE_NAMES { + let symbol = symbols.declare((*name).to_string(), BindingKind::Type); + scopes.bind_type(symbol); + } scopes.enter_scope(ScopeKind::Program); Self { @@ -225,6 +262,7 @@ impl NameResolver { augmented: Default::default(), getters: Default::default(), setters: Default::default(), + type_names: Default::default(), } } @@ -247,8 +285,9 @@ impl NameResolver { self.captured.insert(symbol); } - /// Resolve lexical value names. Type annotations and member selection - /// belong to later passes; this does not validate types or control flow. + /// Resolve lexical names: values, accessors, and the type names in annotations. + /// Choosing overloads, checking types and validating control flow belong to + /// later passes. pub(crate) fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { match &loc.expression { Expression::BoolLiteral(_) @@ -262,13 +301,25 @@ impl NameResolver { } Expression::Statement(inner) | Expression::Grouping(inner) - | Expression::Cast { value: inner, .. } | Expression::Return { value: inner } => self.resolve(inner)?, + Expression::Cast { + value, annotation, .. + } => { + self.resolve_type(annotation)?; + self.resolve(value)?; + } Expression::Logical { left, right, .. } => { self.resolve(left)?; self.resolve(right)?; } - Expression::VariableDeclaration { l_value, value, .. } => { + Expression::VariableDeclaration { + l_value, + annotated_type, + value, + } => { + if let Some(annotation) = annotated_type { + self.resolve_type(annotation)?; + } self.resolve(value)?; self.declare_pattern(l_value); } @@ -355,6 +406,7 @@ impl NameResolver { name, parameters, body, + return_annotation, .. } => { if let Some(name) = name { @@ -386,8 +438,17 @@ impl NameResolver { }); } names.push(param_name); + // Only struct declarations add types, and none can appear among + // the parameters, so this scope sees the same types as the + // enclosing one. + if let Some(annotation) = ¶meter.annotation { + resolver.resolve_type(annotation)?; + } resolver.declare_pattern(¶meter.lvalue); } + if let Some(annotation) = return_annotation { + resolver.resolve_type(annotation)?; + } resolver.resolve(body) })?; @@ -477,6 +538,38 @@ impl NameResolver { Ok(candidates) } + /// Resolves every type name in `annotation`. In `Map`, that's + /// `Map`, `String`, `Int` and `P`. Checking how many type arguments a name takes + /// belongs to the type pass. + fn resolve_type(&mut self, annotation: &TypeExpr) -> Result<(), NameResolutionError> { + match annotation { + TypeExpr::Name { + id, + name, + args, + span, + } => { + let symbol = self + .scopes + .lookup_type(name, &self.symbols) + .ok_or_else(|| NameResolutionError::UnknownType { + name: name.clone(), + span: *span, + })?; + self.type_names.insert(*id, symbol); + for arg in args { + self.resolve_type(arg)?; + } + } + TypeExpr::Tuple { elements, .. } => { + for element in elements { + self.resolve_type(element)?; + } + } + } + Ok(()) + } + fn lookup(&self, name: &str, span: Span) -> Result { self.scopes .lookup(name, &self.symbols) diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index bb9d4652..e829364b 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -874,3 +874,99 @@ fn unknown_accessors_are_unknown_names() { assert_eq!(unknown_name("let p = 1; p.nope;"), "nope"); assert_eq!(unknown_name("let p = 1; p.x = 5;"), "x="); } + +/// The names of every resolved type, sorted for stable comparisons. +fn type_names(resolver: &NameResolver) -> Vec<&str> { + resolver + .type_names + .values() + .map(|&id| resolver.symbols.name(id)) + .sorted() + .collect() +} + +#[test] +fn builtin_type_names_resolve_including_nested_ones() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let m: Map)> = 1;").unwrap(); + + assert_eq!( + type_names(&resolver), + ["Bool", "Int", "List", "Map", "String"] + ); + assert!( + resolver + .type_names + .values() + .all(|&id| resolver.symbols.kind(id) == BindingKind::Type) + ); +} + +#[test] +fn unknown_types_are_reported_in_every_annotation_position() { + for source in [ + "let x: Nope = 1;", + "let x: List = 1;", + "fn f(x: Nope) { 1; }", + "fn f() -> Nope { 1; }", + "1 as Nope;", + ] { + let mut resolver = NameResolver::new([]); + let error = resolve_source(&mut resolver, source).expect_err(source); + assert_eq!(error.to_string(), "unknown type `Nope`", "{source}"); + let NameResolutionError::UnknownType { span, .. } = error else { + unreachable!("the message identifies the variant"); + }; + assert_eq!(&source[span.range()], "Nope", "{source}"); + // A failed parameter annotation leaves the function's scope. + assert_eq!( + resolver.scopes.lookup("x", &resolver.symbols), + None, + "{source}" + ); + } +} + +#[test] +fn types_and_values_are_separate_namespaces() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let Int = 5; let y: Int = Int;").unwrap(); + + // The annotation still means the built-in type; the value is the `let`. + let [value, _] = declared(&resolver); + assert_eq!(type_names(&resolver), ["Int"]); + let type_int = *resolver.type_names.values().next().unwrap(); + assert_ne!(type_int, value); + assert_eq!(resolver.symbols.kind(type_int), BindingKind::Type); + assert_eq!(uses(&resolver, value), 1); +} + +#[test] +fn annotations_resolve_before_values() { + for source in ["let x: Nope = missing;", "missing as Nope;"] { + let mut resolver = NameResolver::new([]); + let error = resolve_source(&mut resolver, source).expect_err(source); + assert!( + matches!(error, NameResolutionError::UnknownType { .. }), + "{source}: {error:?}" + ); + } +} + +#[test] +fn type_names_are_never_captured() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "fn f() { let x: Int = 1; 1 as Int; }").unwrap(); + + assert_eq!(type_names(&resolver), ["Int", "Int"]); + assert!(resolver.captures.is_empty()); + assert!(resolver.captured.is_empty()); +} + +#[test] +fn unannotated_code_records_no_type_names() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "let x = 1; fn f(a) { a; }").unwrap(); + + assert!(resolver.type_names.is_empty()); +} diff --git a/ndc_analyser/src/symbols.rs b/ndc_analyser/src/symbols.rs index 7a70f737..ab0f4494 100644 --- a/ndc_analyser/src/symbols.rs +++ b/ndc_analyser/src/symbols.rs @@ -33,6 +33,8 @@ pub(crate) enum BindingKind { Function, /// Example: bindings, parameters, and loop variables: these don't participate in overloading Plain, + /// Example: `struct Foo { }`: these live in a separate namespace + Type, } #[cfg(test)] From 0087093b0f87d57ceeb43c7beafaee156dd40665 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 15:50:45 +0200 Subject: [PATCH 16/19] =?UTF-8?q?feat(analyser):=20resolve=20struct=20decl?= =?UTF-8?q?arations=20=F0=9F=A7=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A struct binds its name as a type, a constructor, and a getter and setter per field. Field types resolve before the name is bound, so self and forward references stay unknown types. Builtin shadowing, same-scope redefinitions and duplicate fields keep the current analyser's messages. The constructor follows the same-scope rule for functions, while field accessors are exempt so field names can match local variables. With structs done, the resolver has no unsupported expressions left. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 136 ++++++++++-- .../src/name_resolution/resolver_tests.rs | 204 +++++++++++++++++- ndc_parser/src/lib.rs | 2 +- 3 files changed, 313 insertions(+), 29 deletions(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index c1967a0c..2deb2f2d 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -4,7 +4,7 @@ use ndc_core::StaticType; use ndc_lexer::Span; use ndc_parser::{ AssignmentTarget, AssignmentTargetLocation, BindingPattern, BindingPatternLocation, Expression, - ExpressionLocation, ForBody, ForIteration, NodeId, TypeExpr, + ExpressionLocation, ForBody, ForIteration, NodeId, StructField, TypeExpr, }; #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] @@ -14,7 +14,6 @@ pub(crate) struct ScopeId(usize); pub(crate) struct Scope { parent: Option, bindings: Vec, - /// Type names, kept apart from `bindings`: `let Int = 5` doesn't hide the type `Int`. types: Vec, kind: ScopeKind, } @@ -56,14 +55,14 @@ pub(crate) struct NameResolver { /// The type each name in an annotation refers to, keyed by the name's node. /// In `let m: Map`, the `Map`, `String` and `P` nodes. type_names: AHashMap, + /// What each struct declaration binds, keyed by the declaration's node. + structs: AHashMap, } #[derive(Debug, thiserror::Error)] pub(crate) enum NameResolutionError { #[error("Identifier {name} has not previously been declared")] UnknownName { name: String, span: Span }, - #[error("Name resolution for {feature} is not implemented yet")] - Unsupported { feature: &'static str, span: Span }, #[error("Illegal redefinition of parameter {name}")] DuplicateParameter { name: String, span: Span }, #[error("Illegal redefinition of variable {name} as a function in the same scope")] @@ -72,6 +71,18 @@ pub(crate) enum NameResolutionError { SequenceTargetNotAllowed { span: Span }, #[error("unknown type `{name}`")] UnknownType { name: String, span: Span }, + #[error("Struct '{name}' is not allowed to shadow the built-in type '{name}'")] + StructShadowsBuiltinType { name: String, span: Span }, + #[error("Illegal redefinition of struct '{name}'")] + StructRedefinition { name: String, span: Span }, + #[error("Illegal redefinition of field '{field}' in struct '{struct_name}'")] + DuplicateField { + field: String, + struct_name: String, + span: Span, + }, + #[error("Illegal redefinition of variable {name} as a struct in the same scope")] + IllegalStructRedefinition { name: String, span: Span }, } /// Same-named symbols visible at a use, nearest scope first, newest first @@ -90,6 +101,18 @@ pub(crate) struct AugmentedCandidates { operator: Candidates, } +/// The symbols a struct declaration binds. For `struct Point { x: Int }`: the +/// type `Point`, the constructor `Point`, and the getter `x` with the setter `x=`. +#[derive(Debug)] +pub(crate) struct StructSymbols { + /// `Point` in annotations such as `let p: Point`. + type_symbol: SymbolId, + /// `Point` in calls such as `Point(1)`. + constructor: SymbolId, + /// The getter and setter of each field, in declaration order. + accessors: Vec<(SymbolId, SymbolId)>, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum ScopeKind { /// Natives such as `len` and `+`. The VM keeps them as globals, so they're never captured. @@ -199,6 +222,11 @@ impl ScopeTree { self.chain().find_map(|scope| scope.lookup(name, table)) } + /// The type named `name` in the current scope only, ignoring enclosing scopes. + pub(crate) fn current_type(&self, name: &str, table: &SymbolTable) -> Option { + self.scopes[self.current.0].lookup_type(name, table) + } + /// The innermost type named `name`. Types never overload, so there's at most one. pub(crate) fn lookup_type(&self, name: &str, table: &SymbolTable) -> Option { self.chain() @@ -263,6 +291,7 @@ impl NameResolver { getters: Default::default(), setters: Default::default(), type_names: Default::default(), + structs: Default::default(), } } @@ -410,19 +439,13 @@ impl NameResolver { .. } => { if let Some(name) = name { - if self - .scopes - .current_candidates(name, &self.symbols) - .iter() - .any(|binding| self.symbols.kind(*binding) == BindingKind::Plain) - { + if self.has_plain_binding_here(name) { return Err(NameResolutionError::IllegalFunctionRedefinition { name: name.clone(), span: loc.span, }); } - let symbol = self.symbols.declare(name.clone(), BindingKind::Function); - self.scopes.bind(symbol); + let symbol = self.declare_function(name.clone()); self.declarations.insert(loc.id, symbol); } @@ -453,11 +476,9 @@ impl NameResolver { resolver.resolve(body) })?; } - Expression::StructDeclaration { .. } => { - return Err(NameResolutionError::Unsupported { - feature: "struct declarations", - span: loc.span, - }); + Expression::StructDeclaration { name, fields, .. } => { + let symbols = self.declare_struct(name, fields, loc.span)?; + self.structs.insert(loc.id, symbols); } Expression::OpAssignment { l_value, @@ -538,6 +559,87 @@ impl NameResolver { Ok(candidates) } + /// Whether the current scope has a `let`, parameter or loop variable named + /// `name`. A scope holds a name either as plain bindings or as functions. + fn has_plain_binding_here(&self, name: &str) -> bool { + self.scopes + .current_candidates(name, &self.symbols) + .iter() + .any(|&symbol| self.symbols.kind(symbol) == BindingKind::Plain) + } + + fn declare_function(&mut self, name: String) -> SymbolId { + let symbol = self.symbols.declare(name, BindingKind::Function); + self.scopes.bind(symbol); + symbol + } + + /// Declares `struct name { fields }` in the current scope: its name as a type, + /// its constructor, and a getter and setter per field. + fn declare_struct( + &mut self, + name: &str, + fields: &[StructField], + span: Span, + ) -> Result { + // Field types resolve before the struct's own name is bound, so a struct + // can't refer to itself or to structs declared after it. + for field in fields { + self.resolve_type(&field.annotation)?; + } + if StaticType::BUILTIN_TYPE_NAMES.contains(&name) { + return Err(NameResolutionError::StructShadowsBuiltinType { + name: name.to_string(), + span, + }); + } + if self.scopes.current_type(name, &self.symbols).is_some() { + return Err(NameResolutionError::StructRedefinition { + name: name.to_string(), + span, + }); + } + // Field lists are short, so a linear scan beats hashing. + let mut field_names = Vec::new(); + for field in fields { + if field_names.contains(&&field.identifier) { + return Err(NameResolutionError::DuplicateField { + field: field.identifier.clone(), + struct_name: name.to_string(), + span: field.span, + }); + } + field_names.push(&field.identifier); + } + // The constructor is a declared name, so it follows the same rule as `fn`. + if self.has_plain_binding_here(name) { + return Err(NameResolutionError::IllegalStructRedefinition { + name: name.to_string(), + span, + }); + } + + let type_symbol = self.symbols.declare(name.to_string(), BindingKind::Type); + self.scopes.bind_type(type_symbol); + let constructor = self.declare_function(name.to_string()); + // Accessors are exempt from that rule: a field name belongs to the struct, + // so `let x = 5; struct Point { x: Int }` is fine. + let accessors = fields + .iter() + .map(|field| { + let getter = self.declare_function(field.identifier.clone()); + let setter = self.declare_function(format!("{}=", field.identifier)); + (getter, setter) + }) + .collect(); + + Ok(StructSymbols { + type_symbol, + constructor, + accessors, + }) + } + /// Resolves every type name in `annotation`. In `Map`, that's /// `Map`, `String`, `Int` and `P`. Checking how many type arguments a name takes /// belongs to the type pass. diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index e829364b..113a1510 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1,4 +1,6 @@ -use super::{AugmentedCandidates, Candidates, NameResolutionError, NameResolver, ScopeKind}; +use super::{ + AugmentedCandidates, Candidates, NameResolutionError, NameResolver, ScopeKind, StructSymbols, +}; use crate::symbols::{BindingKind, SymbolId}; use itertools::Itertools; use ndc_lexer::{Lexer, SourceId}; @@ -316,16 +318,6 @@ fn loop_body_forms_resolve_names_and_restore_scope_on_errors() { } } -#[test] -fn struct_declarations_report_errors_instead_of_panicking() { - let mut resolver = NameResolver::new([]); - let error = resolve_source(&mut resolver, "struct Point { x: Int }").unwrap_err(); - assert!( - matches!(error, NameResolutionError::Unsupported { feature: "struct declarations", span } if span.source_id() == SourceId::new(0)), - "{error:?}" - ); -} - #[test] fn candidates_in_one_scope_are_newest_first() { let mut resolver = NameResolver::new([]); @@ -970,3 +962,193 @@ fn unannotated_code_records_no_type_names() { assert!(resolver.type_names.is_empty()); } + +fn only_struct(resolver: &NameResolver) -> &StructSymbols { + resolver + .structs + .values() + .exactly_one() + .expect("exactly one struct declaration") +} + +/// The message of the error that resolving `source` reports. +fn error_message(source: &str) -> String { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, source) + .expect_err(source) + .to_string() +} + +#[test] +fn structs_bind_a_type_a_constructor_and_accessors() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "struct Point { x: Int, y: Int } let p: Point = Point(1, 2); p.x; p.y = 3;", + ) + .unwrap(); + + let point = only_struct(&resolver); + let [(x, x_set), (y, y_set)] = point.accessors[..] else { + panic!("expected two fields"); + }; + let symbols = &resolver.symbols; + assert_eq!(symbols.name(point.type_symbol), "Point"); + assert_eq!(symbols.kind(point.type_symbol), BindingKind::Type); + assert_eq!(symbols.name(point.constructor), "Point"); + assert_eq!(symbols.kind(point.constructor), BindingKind::Function); + assert_eq!( + [x, x_set, y, y_set].map(|id| (symbols.name(id), symbols.kind(id))), + [ + ("x", BindingKind::Function), + ("x=", BindingKind::Function), + ("y", BindingKind::Function), + ("y=", BindingKind::Function), + ] + ); + + // Each binding is what its use resolves to. + assert!( + resolver + .type_names + .values() + .any(|&id| id == point.type_symbol) + ); + assert_eq!(uses(&resolver, point.constructor), 1); + assert_eq!(only_getter(&resolver).0, vec![vec![x]]); + assert_eq!(only_setter(&resolver).0, vec![vec![y_set]]); +} + +#[test] +fn structs_cannot_refer_to_themselves_or_later_structs() { + assert_eq!( + error_message("struct Node { value: Int, next: Option }"), + "unknown type `Node`" + ); + assert_eq!( + error_message("struct A { b: B } struct B { x: Int }"), + "unknown type `B`" + ); +} + +#[test] +fn structs_are_scoped_like_other_declarations() { + assert_eq!( + error_message("fn make() { struct S { v: Int } S(1); } fn check(x: S) { x.v; }"), + "unknown type `S`" + ); + + // An inner struct shadows an outer one of the same name. + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "struct P { x: Int } { struct P { y: Int } let p: P = P(2); p.y; }", + ) + .unwrap(); + let inner = resolver + .structs + .values() + .map(|symbols| symbols.type_symbol) + .max() + .unwrap(); + // Field annotations add `Int`s; the `let p: P` annotation is the only `P`. + let annotated = resolver + .type_names + .values() + .filter(|&&id| resolver.symbols.name(id) == "P") + .exactly_one() + .unwrap(); + assert_eq!(*annotated, inner); +} + +#[test] +fn struct_redefinition_errors_keep_the_current_messages() { + assert_eq!( + error_message("struct Int { x: Int }"), + "Struct 'Int' is not allowed to shadow the built-in type 'Int'" + ); + assert_eq!( + error_message("struct P { x: Int } struct P { y: Int }"), + "Illegal redefinition of struct 'P'" + ); + + let source = "struct Dup { a: Int, a: Int }"; + let mut resolver = NameResolver::new([]); + let error = resolve_source(&mut resolver, source).unwrap_err(); + assert_eq!( + error.to_string(), + "Illegal redefinition of field 'a' in struct 'Dup'" + ); + let NameResolutionError::DuplicateField { span, .. } = error else { + unreachable!("the message identifies the variant"); + }; + assert_eq!(span.offset(), source.rfind("a: Int").unwrap()); +} + +#[test] +fn constructors_cannot_redefine_variables_in_the_same_scope() { + assert_eq!( + error_message("let P = 5; struct P { x: Int }"), + "Illegal redefinition of variable P as a struct in the same scope" + ); + for source in [ + "struct P { x: Int } let P = 5;", + "let P = 5; { struct P { x: Int } }", + ] { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, source).unwrap_or_else(|error| panic!("{source}: {error:?}")); + } +} + +#[test] +fn field_accessors_may_share_names_with_variables() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "let x = 5; struct Point { x: Int, y: Int } Point(1, 2).x;", + ) + .unwrap(); + + // The getter is a candidate beside the variable; the type pass skips the variable. + let [local] = declared(&resolver); + let (getter, _) = only_struct(&resolver).accessors[0]; + assert_eq!(only_getter(&resolver).0, vec![vec![getter, local]]); +} + +#[test] +fn structs_sharing_a_field_name_overload_its_accessors() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "struct A { x: Int } struct B { x: String } A(1).x;", + ) + .unwrap(); + + let [a_x, b_x] = resolver + .structs + .values() + .map(|symbols| symbols.accessors[0].0) + .sorted() + .collect_vec()[..] + else { + panic!("expected two structs"); + }; + assert_eq!(only_getter(&resolver).0, vec![vec![b_x, a_x]]); +} + +#[test] +fn struct_bindings_are_captured_like_functions() { + let mut resolver = NameResolver::new([]); + resolve_source( + &mut resolver, + "fn outer() { struct P { x: Int } fn inner() { P(1).x; } }", + ) + .unwrap(); + + let p = only_struct(&resolver); + let inner = functions(&resolver)[1]; + assert_eq!( + captures_of(&resolver, inner), + [p.constructor, p.accessors[0].0] + ); +} diff --git a/ndc_parser/src/lib.rs b/ndc_parser/src/lib.rs index 0d6dfeb4..cfe230c9 100644 --- a/ndc_parser/src/lib.rs +++ b/ndc_parser/src/lib.rs @@ -6,7 +6,7 @@ mod type_expr; pub use expression::{ AssignmentTarget, AssignmentTargetLocation, AugmentedAssignmentPlan, Binding, BindingPattern, BindingPatternLocation, Candidate, CaptureSource, Expression, ExpressionLocation, ForBody, - ForIteration, FunctionParameter, NodeId, ResolvedVar, + ForIteration, FunctionParameter, NodeId, ResolvedVar, StructField, }; pub use operator::{BinaryOperator, LogicalOperator, UnaryOperator}; pub use parser::Error; From 5f7036b55bb071328b7c40b153d28390d8b11f02 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 16:15:11 +0200 Subject: [PATCH 17/19] =?UTF-8?q?fix(analyser):=20resolve=20the=20None=20l?= =?UTF-8?q?iteral=20=F0=9F=AB=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `None` is a literal since #229, so the resolver treats it like `true`. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 2deb2f2d..aa91b3bd 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -320,6 +320,7 @@ impl NameResolver { pub(crate) fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { match &loc.expression { Expression::BoolLiteral(_) + | Expression::NoneLiteral | Expression::StringLiteral(_) | Expression::NumericLiteral(_) | Expression::Break From 6174e5526c05f35445ae2ca28de3653c1791b478 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 16:31:22 +0200 Subject: [PATCH 18/19] =?UTF-8?q?feat(analyser):=20put=20the=20name=20reso?= =?UTF-8?q?lver=20in=20front=20of=20the=20analyser=20=F0=9F=94=97=20(#230)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #226 so these changes can be reviewed apart from the resolver work itself. The resolver on `feat/new-analyser` produced its results but nothing used them. This PR runs it in front of the existing analyser, so it handles every program and test, as phase 1 of replacing the old analyser. Phase 2 will turn the old analyser into the type pass. - **Keep going after errors:** the resolver records one error per failing statement and carries on, including inside blocks, so `a; b;` reports both names like the current analyser does. - **Resolve names before analysis:** `Analyser::analyse` runs the resolver on each top-level statement first, with the registered natives as builtins. A statement with name errors isn't analysed further, so the same error isn't reported twice. The resolver's state is part of the analyser checkpoint, so a failed REPL line rolls it back. The compiler is unchanged. **Worth a look:** - **Behaviour change:** a `fn` after a `let` of the same name in one scope is now an error, as decided for the resolver. `005_functions/034` became an error test, and the function page in the manual documents the rule. - **Test fixtures:** four analyser unit tests now declare the `[]`/`[]=` natives their index assignments use, because the resolver rejects index access without them. - **Unused fields:** the `AugmentedCandidates` and `StructSymbols` fields only the type pass will read are marked `expect(dead_code)`, so the marker becomes an error once they're used. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- manual/src/reference/types/function.md | 12 ++++ ndc_analyser/src/analyser.rs | 68 +++++++++++++++++-- ndc_analyser/src/name_resolution.rs | 57 ++++++++++++++-- .../src/name_resolution/resolver_tests.rs | 61 ++++++++++++++++- ndc_analyser/src/scope.rs | 8 +++ ndc_analyser/src/symbols.rs | 4 +- .../034_fn_after_let_adds_overload.ndc | 7 -- .../034_fn_after_let_is_an_error.ndc | 6 ++ 8 files changed, 199 insertions(+), 24 deletions(-) delete mode 100644 tests/functional/programs/005_functions/034_fn_after_let_adds_overload.ndc create mode 100644 tests/functional/programs/005_functions/034_fn_after_let_is_an_error.ndc diff --git a/manual/src/reference/types/function.md b/manual/src/reference/types/function.md index 5e429033..2b5964f0 100644 --- a/manual/src/reference/types/function.md +++ b/manual/src/reference/types/function.md @@ -164,6 +164,18 @@ let foo = fn(a) => "let"; foo("x"); // "let": both fn overloads are shadowed ``` +The reverse order is an error within one scope: a scope holds a name either as `let` bindings or as functions. In a nested scope, the `fn` shadows the outer `let` as usual. The same rule applies to a struct's constructor. + +```ndc +let foo = fn(a) => "let"; +fn foo(a, b) { "fn" } // ERROR: Illegal redefinition of variable foo as a function in the same scope + +let bar = 1; +{ + fn bar(a) { a } // fine: a different scope +} +``` + A non-function `let` binding shadows the name for value access, but function calls still resolve to the underlying function: ```ndc diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs index 60b74680..d2dd536a 100644 --- a/ndc_analyser/src/analyser.rs +++ b/ndc_analyser/src/analyser.rs @@ -1,3 +1,4 @@ +use crate::name_resolution::{NameResolutionError, NameResolver}; use crate::scope::{CallKind, ResolvedCall, ScopeTree, TypeBinding, TypeDef}; use itertools::{Itertools, izip}; use ndc_core::static_type::StaticTypeConstructionError; @@ -33,12 +34,16 @@ pub struct AnalysisResult { /// discards every binding and struct declared since. #[derive(Debug, Clone)] pub struct Checkpoint { + resolver: NameResolver, scope_tree: ScopeTree, struct_count: usize, } #[derive(Debug)] pub struct Analyser { + /// Resolves names before analysis. It runs alongside the scope tree below + /// until the type pass reads names from it instead. + resolver: NameResolver, struct_registry: Rc>, scope_tree: ScopeTree, /// Stack of explicit `return` types for each enclosing function scope. @@ -57,6 +62,7 @@ impl Analyser { struct_registry: Rc>, ) -> Self { Self { + resolver: NameResolver::new(scope_tree.native_names()), scope_tree, struct_registry, return_type_stack: Vec::new(), @@ -67,12 +73,14 @@ impl Analyser { pub fn checkpoint(&self) -> Checkpoint { Checkpoint { + resolver: self.resolver.clone(), scope_tree: self.scope_tree.clone(), struct_count: self.struct_registry.borrow().len(), } } pub fn restore(&mut self, checkpoint: Checkpoint) { + self.resolver = checkpoint.resolver; self.scope_tree = checkpoint.scope_tree; self.struct_registry .borrow_mut() @@ -152,9 +160,25 @@ impl Analyser { !self.errors.is_empty() } + /// Analyses a top-level statement. Names resolve first; a statement with + /// unresolved names isn't analysed further, which would only repeat its errors. pub fn analyse( &mut self, expr_loc: &mut ExpressionLocation, + ) -> Result { + self.resolver.resolve_statement(expr_loc); + let name_errors = self.resolver.take_errors(); + if !name_errors.is_empty() { + self.errors + .extend(name_errors.into_iter().map(AnalysisError::from)); + return Ok(StaticType::Any); + } + self.analyse_expr(expr_loc) + } + + fn analyse_expr( + &mut self, + expr_loc: &mut ExpressionLocation, ) -> Result { let typ = self.analyse_inner(expr_loc)?; self.result.expr_types.insert(expr_loc.id, typ.clone()); @@ -164,7 +188,7 @@ impl Analyser { /// Like [`analyse`], but on error emits the error and returns `Any` /// so that analysis can continue. fn analyse_or_any(&mut self, expr_loc: &mut ExpressionLocation) -> StaticType { - match self.analyse(expr_loc) { + match self.analyse_expr(expr_loc) { Ok(t) => t, Err(e) => { self.emit(e); @@ -298,7 +322,7 @@ impl Analyser { self.analyse_or_any(right); Ok(StaticType::Bool) } - Expression::Grouping(expr) => self.analyse(expr), + Expression::Grouping(expr) => self.analyse_expr(expr), Expression::Cast { value, annotation, @@ -1273,6 +1297,16 @@ pub struct AnalysisError { help_text: Option, } +impl From for AnalysisError { + fn from(error: NameResolutionError) -> Self { + Self { + text: error.to_string(), + span: error.span(), + help_text: None, + } + } +} + impl AnalysisError { pub fn span(&self) -> Span { self.span @@ -1476,6 +1510,19 @@ mod tests { analyse_last_type_with_globals(source, vec![]) } + /// `[]` and `[]=`, which index reads and writes go through. Name resolution + /// rejects index access without them, as it would any other unknown function. + fn index_natives() -> Vec<(String, StaticType)> { + let any_function = StaticType::Function { + parameters: None, + return_type: Box::new(StaticType::Any), + }; + vec![ + ("[]".to_string(), any_function.clone()), + ("[]=".to_string(), any_function), + ] + } + fn assert_analysis_error(source: &str, globals: Vec<(String, StaticType)>, expected: &str) { let (_, result) = analyse_with_globals(source, globals); assert!( @@ -1491,7 +1538,10 @@ mod tests { #[test] fn inferred_list_index_assignment_widens_element_type() { assert_eq!( - analyse_last_type("let values = [1]; values[0] = \"two\"; values"), + analyse_last_type_with_globals( + "let values = [1]; values[0] = \"two\"; values", + index_natives() + ), StaticType::List(Box::new(StaticType::Any)), ); } @@ -1499,7 +1549,10 @@ mod tests { #[test] fn inferred_list_slice_assignment_widens_element_type() { assert_eq!( - analyse_last_type("let values = [1]; values[0..1] = [\"two\"]; values"), + analyse_last_type_with_globals( + "let values = [1]; values[0..1] = [\"two\"]; values", + index_natives() + ), StaticType::List(Box::new(StaticType::Any)), ); } @@ -1507,7 +1560,10 @@ mod tests { #[test] fn inferred_map_index_assignment_widens_value_and_preserves_key_type() { assert_eq!( - analyse_last_type("let values = %{\"one\": 1}; values[\"two\"] = \"two\"; values"), + analyse_last_type_with_globals( + "let values = %{\"one\": 1}; values[\"two\"] = \"two\"; values", + index_natives() + ), StaticType::Map { key: Box::new(StaticType::String), value: Box::new(StaticType::Any), @@ -1524,7 +1580,7 @@ mod tests { assert_eq!( analyse_last_type_with_globals( "let values = [1]; values[0] += 0.5; values", - vec![("+".to_string(), add)], + [index_natives(), vec![("+".to_string(), add)]].concat(), ), StaticType::List(Box::new(StaticType::Any)), ); diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index aa91b3bd..16421e1f 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -10,7 +10,7 @@ use ndc_parser::{ #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)] pub(crate) struct ScopeId(usize); -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct Scope { parent: Option, bindings: Vec, @@ -18,12 +18,14 @@ pub(crate) struct Scope { kind: ScopeKind, } +#[derive(Debug, Clone)] pub(crate) struct ScopeTree { current: ScopeId, scopes: Vec, bound_in: AHashMap, } +#[derive(Debug, Clone)] pub(crate) struct NameResolver { symbols: SymbolTable, scopes: ScopeTree, @@ -57,9 +59,12 @@ pub(crate) struct NameResolver { type_names: AHashMap, /// What each struct declaration binds, keyed by the declaration's node. structs: AHashMap, + /// Errors from statements that failed to resolve. Resolution carries on with + /// the next statement, so `a; b;` reports both unknown names. + errors: Vec, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, Clone, thiserror::Error)] pub(crate) enum NameResolutionError { #[error("Identifier {name} has not previously been declared")] UnknownName { name: String, span: Span }, @@ -85,17 +90,37 @@ pub(crate) enum NameResolutionError { IllegalStructRedefinition { name: String, span: Span }, } +impl NameResolutionError { + pub(crate) fn span(&self) -> Span { + match self { + Self::UnknownName { span, .. } + | Self::DuplicateParameter { span, .. } + | Self::IllegalFunctionRedefinition { span, .. } + | Self::SequenceTargetNotAllowed { span } + | Self::UnknownType { span, .. } + | Self::StructShadowsBuiltinType { span, .. } + | Self::StructRedefinition { span, .. } + | Self::DuplicateField { span, .. } + | Self::IllegalStructRedefinition { span, .. } => *span, + } + } +} + /// Same-named symbols visible at a use, nearest scope first, newest first /// within a scope. For `x` in `let x = 1; { let x = 2; x }`: `[[x2], [x1]]`. /// Identifiers always have at least one candidate, augmented operators may /// not. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct Candidates(Vec>); /// The two operations an augmented assignment can use. For `xs ++= ys`: /// `in_place` holds `++=` (mutates `xs`), `operator` holds `++` (builds a new value /// that is written back). Either may be empty; the type pass picks. -#[derive(Debug)] +#[derive(Debug, Clone)] +#[cfg_attr( + not(test), + expect(dead_code, reason = "read by the type pass, which is not written yet") +)] pub(crate) struct AugmentedCandidates { in_place: Candidates, operator: Candidates, @@ -103,7 +128,11 @@ pub(crate) struct AugmentedCandidates { /// The symbols a struct declaration binds. For `struct Point { x: Int }`: the /// type `Point`, the constructor `Point`, and the getter `x` with the setter `x=`. -#[derive(Debug)] +#[derive(Debug, Clone)] +#[cfg_attr( + not(test), + expect(dead_code, reason = "read by the type pass, which is not written yet") +)] pub(crate) struct StructSymbols { /// `Point` in annotations such as `let p: Point`. type_symbol: SymbolId, @@ -292,6 +321,7 @@ impl NameResolver { setters: Default::default(), type_names: Default::default(), structs: Default::default(), + errors: Default::default(), } } @@ -314,10 +344,23 @@ impl NameResolver { self.captured.insert(symbol); } + /// Resolves `statement`, recording its error instead of returning it so the + /// statements after it still resolve. A failed statement stops at its first error. + pub(crate) fn resolve_statement(&mut self, statement: &ExpressionLocation) { + if let Err(error) = self.resolve(statement) { + self.errors.push(error); + } + } + + /// The errors recorded so far, leaving none behind. + pub(crate) fn take_errors(&mut self) -> Vec { + std::mem::take(&mut self.errors) + } + /// Resolve lexical names: values, accessors, and the type names in annotations. /// Choosing overloads, checking types and validating control flow belong to /// later passes. - pub(crate) fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { + fn resolve(&mut self, loc: &ExpressionLocation) -> Result<(), NameResolutionError> { match &loc.expression { Expression::BoolLiteral(_) | Expression::NoneLiteral @@ -360,7 +403,7 @@ impl NameResolver { Expression::Block { statements } => { self.in_scope(ScopeKind::Block, |resolver| { for statement in statements { - resolver.resolve(statement)?; + resolver.resolve_statement(statement); } Ok(()) })?; diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 113a1510..627af79a 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -12,9 +12,13 @@ fn resolve_source(resolver: &mut NameResolver, source: &str) -> Result<(), NameR .expect("source lexes"); let expressions = Parser::from_tokens(tokens).parse().expect("source parses"); for expression in &expressions { - resolver.resolve(expression)?; + resolver.resolve_statement(expression); + } + // Most tests care about the first error; the recovery tests read them all. + match resolver.take_errors().into_iter().next() { + Some(error) => Err(error), + None => Ok(()), } - Ok(()) } /// Declared symbols in declaration order; the symbol table hands out ids sequentially. @@ -1152,3 +1156,56 @@ fn struct_bindings_are_captured_like_functions() { [p.constructor, p.accessors[0].0] ); } + +/// Every error resolving `source` reports, in order. +fn all_errors(source: &str) -> Vec { + let mut resolver = NameResolver::new([]); + let tokens = Lexer::new(source, SourceId::new(0)) + .collect::, _>>() + .expect("source lexes"); + for expression in &Parser::from_tokens(tokens).parse().expect("source parses") { + resolver.resolve_statement(expression); + } + resolver + .take_errors() + .iter() + .map(ToString::to_string) + .collect() +} + +#[test] +fn errors_are_reported_for_every_failing_statement() { + let expected = [ + "Identifier a has not previously been declared", + "Identifier b has not previously been declared", + ]; + // At the top level, in a block and in a function body. + for source in ["a; b;", "{ a; b; }", "fn f() { a; b; }"] { + assert_eq!(all_errors(source), expected, "{source}"); + } + assert_eq!( + all_errors("let x: Unknwn = 1; undefined_variable;"), + [ + "unknown type `Unknwn`", + "Identifier undefined_variable has not previously been declared" + ] + ); +} + +#[test] +fn a_failing_statement_stops_at_its_first_error() { + assert_eq!( + all_errors("(a, b);"), + ["Identifier a has not previously been declared"] + ); +} + +#[test] +fn statements_after_an_error_still_resolve() { + let mut resolver = NameResolver::new([]); + resolve_source(&mut resolver, "missing; let x = 1; x;").unwrap_err(); + + let [x] = declared(&resolver); + assert_eq!(uses(&resolver, x), 1); + assert!(resolver.take_errors().is_empty()); +} diff --git a/ndc_analyser/src/scope.rs b/ndc_analyser/src/scope.rs index 4e57e299..87edf3c0 100644 --- a/ndc_analyser/src/scope.rs +++ b/ndc_analyser/src/scope.rs @@ -422,6 +422,14 @@ impl ScopeTree { } } + /// The names of the native bindings, in registration order. + pub(crate) fn native_names(&self) -> impl Iterator { + self.global_scope + .identifiers + .iter() + .map(|binding| binding.name.as_str()) + } + pub(crate) fn get_type(&self, res: ResolvedVar) -> &StaticType { match res { ResolvedVar::Local { slot } => self.find_type_by_slot(self.current_scope_idx, slot), diff --git a/ndc_analyser/src/symbols.rs b/ndc_analyser/src/symbols.rs index ab0f4494..15471156 100644 --- a/ndc_analyser/src/symbols.rs +++ b/ndc_analyser/src/symbols.rs @@ -1,13 +1,13 @@ #[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] pub(crate) struct SymbolId(usize); -#[derive(Debug)] +#[derive(Debug, Clone)] struct Symbol { name: String, kind: BindingKind, } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub(crate) struct SymbolTable { symbols: Vec, } diff --git a/tests/functional/programs/005_functions/034_fn_after_let_adds_overload.ndc b/tests/functional/programs/005_functions/034_fn_after_let_adds_overload.ndc deleted file mode 100644 index c0d0996f..00000000 --- a/tests/functional/programs/005_functions/034_fn_after_let_adds_overload.ndc +++ /dev/null @@ -1,7 +0,0 @@ -// A fn declaration after a let binding should add to the scope as a new -// overload entry (different arity), not replace the let binding. -let foo = fn(a) => "let:" ++ a; -fn foo(a, b) { "fn:" ++ a ++ "," ++ b } - -assert_eq(foo("x"), "let:x"); -assert_eq(foo("x", "y"), "fn:x,y"); diff --git a/tests/functional/programs/005_functions/034_fn_after_let_is_an_error.ndc b/tests/functional/programs/005_functions/034_fn_after_let_is_an_error.ndc new file mode 100644 index 00000000..70fe8a3c --- /dev/null +++ b/tests/functional/programs/005_functions/034_fn_after_let_is_an_error.ndc @@ -0,0 +1,6 @@ +// A scope holds a name either as `let` bindings or as functions, never both. +// A `fn` after a `let` of the same name in the same scope is an error rather +// than a new overload. In a nested scope it would shadow the `let` instead. +// expect-error: Illegal redefinition of variable foo as a function in the same scope +let foo = fn(a) => "let:" ++ a; +fn foo(a, b) { "fn:" ++ a ++ "," ++ b } From 98391ca927919aa5361171571cf2263f8035529e Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 16:44:49 +0200 Subject: [PATCH 19/19] =?UTF-8?q?feat(analyser):=20record=20each=20native'?= =?UTF-8?q?s=20registry=20slot=20=F0=9F=94=A2=20(#231)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2, step 1, stacked on #226. Later steps will stack on this branch. The layout pass in phase 2 will turn native symbols into the VM's global slots, so it needs each native's position in the function registry. That position happened to equal the native's `SymbolId`, because natives are declared first, but nothing guaranteed it. This makes the mapping explicit. - `NameResolver` records `native_slots`, from each native symbol to its registry index. Overloads of the same name keep separate slots. - Nothing reads it yet, so the field is marked `expect(dead_code)` outside tests, like the other fields waiting for later passes. No behaviour change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 --- ndc_analyser/src/name_resolution.rs | 15 ++++++++++++++- .../src/name_resolution/resolver_tests.rs | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 16421e1f..262f405f 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -59,6 +59,16 @@ pub(crate) struct NameResolver { type_names: AHashMap, /// What each struct declaration binds, keyed by the declaration's node. structs: AHashMap, + /// Each native's position in the function registry, which is also its + /// global slot in the VM. With `new(["len", "+"])`, `len` is 0 and `+` is 1. + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "read by the layout pass, which is not written yet" + ) + )] + native_slots: AHashMap, /// Errors from statements that failed to resolve. Resolution carries on with /// the next statement, so `a; b;` reports both unknown names. errors: Vec, @@ -298,9 +308,11 @@ impl NameResolver { let mut scopes = ScopeTree::new(); let mut symbols = SymbolTable::default(); - for builtin in builtins { + let mut native_slots = AHashMap::default(); + for (slot, builtin) in builtins.into_iter().enumerate() { let symbol = symbols.declare(builtin.to_string(), BindingKind::Function); scopes.bind(symbol); + native_slots.insert(symbol, slot); } for name in StaticType::BUILTIN_TYPE_NAMES { let symbol = symbols.declare((*name).to_string(), BindingKind::Type); @@ -321,6 +333,7 @@ impl NameResolver { setters: Default::default(), type_names: Default::default(), structs: Default::default(), + native_slots, errors: Default::default(), } } diff --git a/ndc_analyser/src/name_resolution/resolver_tests.rs b/ndc_analyser/src/name_resolution/resolver_tests.rs index 627af79a..e361fb54 100644 --- a/ndc_analyser/src/name_resolution/resolver_tests.rs +++ b/ndc_analyser/src/name_resolution/resolver_tests.rs @@ -1209,3 +1209,21 @@ fn statements_after_an_error_still_resolve() { assert_eq!(uses(&resolver, x), 1); assert!(resolver.take_errors().is_empty()); } + +#[test] +fn natives_record_their_registry_slot() { + let mut resolver = NameResolver::new(["len", "+", "len"]); + resolve_source(&mut resolver, "let x = 1; len;").unwrap(); + + // Overloads of one name keep their own slots, in registration order. + let [newest_len, oldest_len] = only_reference(&resolver).0[0][..] else { + panic!("expected both `len` overloads"); + }; + let plus = builtin(&resolver, "+"); + assert_eq!(resolver.native_slots[&oldest_len], 0); + assert_eq!(resolver.native_slots[&plus], 1); + assert_eq!(resolver.native_slots[&newest_len], 2); + // Program symbols aren't natives. + let [x] = declared(&resolver); + assert!(!resolver.native_slots.contains_key(&x)); +}