From a6e30498f8d4d0f391b752011b25843ca07b3ba4 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 16:44:33 +0200 Subject: [PATCH] =?UTF-8?q?refactor(analyser):=20give=20old=20scope=20bind?= =?UTF-8?q?ings=20their=20resolver=20symbol=20=F0=9F=AA=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each binding in the old scope tree records the name resolver's symbol for it: `let` and loop patterns, parameters, named functions, struct constructors and accessors, and natives. Debug assertions check that every binding's name matches its symbol's. Nothing reads the symbols yet; the next steps move lookups onto the resolver through them. Co-Authored-By: Claude Opus 5.5 --- ndc_analyser/src/analyser.rs | 107 +++++++++++++++++++--------- ndc_analyser/src/name_resolution.rs | 33 ++++++--- ndc_analyser/src/scope.rs | 95 +++++++++++++++++------- 3 files changed, 164 insertions(+), 71 deletions(-) diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs index d2dd536a..1b52bf8b 100644 --- a/ndc_analyser/src/analyser.rs +++ b/ndc_analyser/src/analyser.rs @@ -1,5 +1,6 @@ use crate::name_resolution::{NameResolutionError, NameResolver}; use crate::scope::{CallKind, ResolvedCall, ScopeTree, TypeBinding, TypeDef}; +use crate::symbols::SymbolId; use itertools::{Itertools, izip}; use ndc_core::static_type::StaticTypeConstructionError; use ndc_core::r#struct::StructRegistry; @@ -8,7 +9,7 @@ use ndc_lexer::{NumericLiteral, Span}; use ndc_parser::{ AssignmentTarget, AssignmentTargetLocation, AugmentedAssignmentPlan, Binding, BindingPattern, BindingPatternLocation, Candidate, Expression, ExpressionLocation, ForBody, ForIteration, - FunctionParameter, NodeId, TypeExpr, + FunctionParameter, NodeId, ResolvedVar, TypeExpr, }; use std::cell::RefCell; use std::collections::HashMap; @@ -61,8 +62,18 @@ impl Analyser { scope_tree: ScopeTree, struct_registry: Rc>, ) -> Self { + let resolver = NameResolver::new(scope_tree.native_names()); + let mut scope_tree = scope_tree; + let natives = resolver.native_symbols(); + debug_assert!( + scope_tree + .native_names() + .zip(&natives) + .all(|(name, &symbol)| resolver.symbol_name(symbol) == name) + ); + scope_tree.attach_native_symbols(natives); Self { - resolver: NameResolver::new(scope_tree.native_names()), + resolver, scope_tree, struct_registry, return_type_stack: Vec::new(), @@ -95,6 +106,17 @@ impl Analyser { result } + /// Binds `name` in the current scope as the resolver's `symbol` for it. + fn create_local_binding( + &mut self, + name: String, + binding: TypeBinding, + symbol: SymbolId, + ) -> ResolvedVar { + debug_assert_eq!(self.resolver.symbol_name(symbol), name); + self.scope_tree.create_local_binding(name, binding, symbol) + } + /// Record a non-fatal analysis error. The analyser continues with a /// fallback type (usually `Any`) so that subsequent code is still checked. fn emit(&mut self, err: AnalysisError) { @@ -475,32 +497,34 @@ impl Analyser { // Pre-register the function before analyzing its body so recursive calls can // resolve the name. The return type is unknown at this point so we use Any. - let pre_slot = - if let Some(name) = name { - let arity = type_signature.types().map(|t| t.len()); - if self.scope_tree.has_function_in_current_scope(name, arity) { - self.emit(AnalysisError::function_redefinition(name, arity, *span)); - // Skip re-registering but still analyse the body below. - None - } else { - let placeholder = StaticType::Function { - parameters: type_signature.types(), - return_type: Box::new( - return_type_slot.clone().unwrap_or(StaticType::Any), - ), - }; - Some(self.scope_tree.create_local_binding( - name.clone(), - TypeBinding::Inferred(placeholder), - )) - } - } else { + let pre_slot = if let Some(name) = name { + let arity = type_signature.types().map(|t| t.len()); + if self.scope_tree.has_function_in_current_scope(name, arity) { + self.emit(AnalysisError::function_redefinition(name, arity, *span)); + // Skip re-registering but still analyse the body below. None - }; + } else { + let placeholder = StaticType::Function { + parameters: type_signature.types(), + return_type: Box::new( + return_type_slot.clone().unwrap_or(StaticType::Any), + ), + }; + Some(self.create_local_binding( + name.clone(), + TypeBinding::Inferred(placeholder), + self.resolver.declared_symbol(*id), + )) + } + } else { + None + }; self.scope_tree.new_function_scope(); self.return_type_stack.push(None); - let param_types = self.resolve_parameters_declarative(&type_signature, *span); + let parameter_nodes = parameters.iter().map(|p| p.lvalue.id).collect_vec(); + let param_types = + self.resolve_parameters_declarative(&type_signature, ¶meter_nodes, *span); // Fill inferred_type on parameter binding patterns for LSP hints. for (p, typ) in parameters.iter_mut().zip(¶m_types) { @@ -735,20 +759,26 @@ impl Analyser { self.scope_tree .declare_type(name.clone(), TypeDef::Struct(Rc::clone(&info))); - *resolved_name = Some(self.scope_tree.create_local_binding( + let symbols = self.resolver.struct_symbols(*id).clone(); + *resolved_name = Some(self.create_local_binding( name.clone(), TypeBinding::Annotated(info.constructor_type()), + symbols.constructor, )); - for (index, field) in fields.iter_mut().enumerate() { - field.resolved_getter = Some(self.scope_tree.create_local_binding( + for (index, (field, &(getter, setter))) in + fields.iter_mut().zip(&symbols.accessors).enumerate() + { + field.resolved_getter = Some(self.create_local_binding( field.identifier.clone(), TypeBinding::Annotated(info.getter_type(index)), + getter, )); - field.resolved_setter = Some(self.scope_tree.create_local_binding( + field.resolved_setter = Some(self.create_local_binding( format!("{}=", field.identifier), TypeBinding::Annotated(info.setter_type(index)), + setter, )); } @@ -1146,6 +1176,7 @@ impl Analyser { fn resolve_parameters_declarative( &mut self, type_signature: &TypeSignature, + parameter_nodes: &[NodeId], span: Span, ) -> Vec { let TypeSignature::Exact(parameters) = type_signature else { @@ -1156,7 +1187,11 @@ impl Analyser { self.emit(AnalysisError::parameter_redefined(¶m.name, span)); } - for param in parameters.iter().unique_by(|param| ¶m.name) { + for (param, &node) in parameters + .iter() + .zip(parameter_nodes) + .unique_by(|(param, _)| ¶m.name) + { let has_annotation = param.type_name != StaticType::Any; let binding = if has_annotation { TypeBinding::Annotated(param.type_name.clone()) @@ -1164,8 +1199,8 @@ impl Analyser { TypeBinding::Inferred(StaticType::Any) }; - self.scope_tree - .create_local_binding(param.name.clone(), binding); + let symbol = self.resolver.declared_symbol(node); + self.create_local_binding(param.name.clone(), binding, symbol); } parameters @@ -1203,10 +1238,12 @@ impl Analyser { None => TypeBinding::Inferred(found_type), }; - *resolved = Some( - self.scope_tree - .create_local_binding(identifier.clone(), type_binding.clone()), - ); + let symbol = self.resolver.declared_symbol(lvalue.id); + *resolved = Some(self.create_local_binding( + identifier.clone(), + type_binding.clone(), + symbol, + )); *inferred_type = Some(type_binding.typ().clone()) } diff --git a/ndc_analyser/src/name_resolution.rs b/ndc_analyser/src/name_resolution.rs index 262f405f..247c619b 100644 --- a/ndc_analyser/src/name_resolution.rs +++ b/ndc_analyser/src/name_resolution.rs @@ -61,13 +61,6 @@ pub(crate) struct NameResolver { 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. @@ -147,9 +140,9 @@ pub(crate) struct StructSymbols { /// `Point` in annotations such as `let p: Point`. type_symbol: SymbolId, /// `Point` in calls such as `Point(1)`. - constructor: SymbolId, + pub(crate) constructor: SymbolId, /// The getter and setter of each field, in declaration order. - accessors: Vec<(SymbolId, SymbolId)>, + pub(crate) accessors: Vec<(SymbolId, SymbolId)>, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -365,6 +358,28 @@ impl NameResolver { } } + /// The native symbols in registry order: the first is the native in slot 0. + pub(crate) fn native_symbols(&self) -> Vec { + let mut natives = self.native_slots.iter().collect::>(); + natives.sort_by_key(|&(_, &slot)| slot); + natives.into_iter().map(|(&symbol, _)| symbol).collect() + } + + /// The symbol a resolved declaration introduced: a binding pattern identifier's + /// or a named function's, keyed by its node. + pub(crate) fn declared_symbol(&self, node: NodeId) -> SymbolId { + self.declarations[&node] + } + + pub(crate) fn symbol_name(&self, symbol: SymbolId) -> &str { + self.symbols.name(symbol) + } + + /// What the struct declared at `node` bound. + pub(crate) fn struct_symbols(&self, node: NodeId) -> &StructSymbols { + &self.structs[&node] + } + /// The errors recorded so far, leaving none behind. pub(crate) fn take_errors(&mut self) -> Vec { std::mem::take(&mut self.errors) diff --git a/ndc_analyser/src/scope.rs b/ndc_analyser/src/scope.rs index 87edf3c0..4722d0ab 100644 --- a/ndc_analyser/src/scope.rs +++ b/ndc_analyser/src/scope.rs @@ -1,3 +1,4 @@ +use crate::symbols::SymbolId; use ndc_core::StaticType; use ndc_core::r#struct::StructInfo; use ndc_parser::{Binding, Candidate, CaptureSource, ResolvedVar}; @@ -136,6 +137,9 @@ impl TypeBinding { pub(crate) struct ScopeBinding { pub name: String, pub binding: TypeBinding, + /// The name resolver's symbol for this binding. Natives get theirs when the + /// analyser is created, since this tree is built before the resolver. + pub symbol: Option, } #[derive(Debug, Clone)] @@ -350,10 +354,11 @@ impl Scope { }) } - fn allocate(&mut self, name: String, type_binding: TypeBinding) -> usize { + fn allocate(&mut self, name: String, type_binding: TypeBinding, symbol: SymbolId) -> usize { self.identifiers.push(ScopeBinding { name, binding: type_binding, + symbol: Some(symbol), }); self.base_offset + self.identifiers.len() - 1 } @@ -408,6 +413,7 @@ impl ScopeTree { .map(|(name, typ)| ScopeBinding { name, binding: TypeBinding::Inferred(typ), + symbol: None, }) .collect(); global_scope.types = StaticType::BUILTIN_TYPE_NAMES @@ -1000,9 +1006,17 @@ impl ScopeTree { &mut self, ident: String, binding: TypeBinding, + symbol: SymbolId, ) -> ResolvedVar { ResolvedVar::Local { - slot: self.scopes[self.current_scope_idx].allocate(ident, binding), + slot: self.scopes[self.current_scope_idx].allocate(ident, binding, symbol), + } + } + + /// Gives each native binding its resolver symbol, in registry order. + pub(crate) fn attach_native_symbols(&mut self, symbols: impl IntoIterator) { + for (binding, symbol) in self.global_scope.identifiers.iter_mut().zip(symbols) { + binding.symbol = Some(symbol); } } @@ -1171,8 +1185,14 @@ impl Debug for ScopeTree { #[cfg(test)] mod tests { use super::*; + use crate::symbols::{BindingKind, SymbolTable}; use ndc_parser::ResolvedVar; + /// A placeholder symbol: these tests exercise slots, not symbols. + fn symbol() -> SymbolId { + SymbolTable::default().declare(String::new(), BindingKind::Plain) + } + fn empty_scope_tree() -> ScopeTree { ScopeTree::from_global_scope(vec![]) } @@ -1180,7 +1200,8 @@ mod tests { #[test] fn single_local_in_function_scope() { let mut tree = empty_scope_tree(); - let var = tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + let var = + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(var, ResolvedVar::Local { slot: 0 }); assert_eq!( tree.get_binding_any("x"), @@ -1191,9 +1212,12 @@ mod tests { #[test] fn multiple_locals_get_ascending_slots() { let mut tree = empty_scope_tree(); - let x = tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); - let y = tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int)); - let z = tree.create_local_binding("z".into(), TypeBinding::Inferred(StaticType::Int)); + let x = + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); + let y = + tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int), symbol()); + let z = + tree.create_local_binding("z".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(x, ResolvedVar::Local { slot: 0 }); assert_eq!(y, ResolvedVar::Local { slot: 1 }); assert_eq!(z, ResolvedVar::Local { slot: 2 }); @@ -1202,11 +1226,13 @@ mod tests { #[test] fn block_scope_continues_flat_numbering() { let mut tree = empty_scope_tree(); - let x = tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + let x = + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(x, ResolvedVar::Local { slot: 0 }); tree.new_block_scope(); - let y = tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int)); + let y = + tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(y, ResolvedVar::Local { slot: 1 }); assert_eq!( @@ -1218,21 +1244,23 @@ mod tests { #[test] fn nested_block_scopes_continue_numbering() { let mut tree = empty_scope_tree(); - tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_block_scope(); - let b = tree.create_local_binding("b".into(), TypeBinding::Inferred(StaticType::Int)); + let b = + tree.create_local_binding("b".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(b, ResolvedVar::Local { slot: 1 }); tree.new_block_scope(); - let c = tree.create_local_binding("c".into(), TypeBinding::Inferred(StaticType::Int)); + let c = + tree.create_local_binding("c".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(c, ResolvedVar::Local { slot: 2 }); } #[test] fn block_scope_does_not_create_upvalue() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_block_scope(); assert_eq!( @@ -1244,10 +1272,11 @@ mod tests { #[test] fn function_scope_resets_slots_and_captures_as_upvalue() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_function_scope(); - let y = tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int)); + let y = + tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(y, ResolvedVar::Local { slot: 0 }); assert_eq!( @@ -1259,10 +1288,11 @@ mod tests { #[test] fn iteration_scope_continues_numbering_and_is_transparent() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_iteration_scope(); - let i = tree.create_local_binding("i".into(), TypeBinding::Inferred(StaticType::Int)); + let i = + tree.create_local_binding("i".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(i, ResolvedVar::Local { slot: 1 }); assert_eq!( @@ -1290,21 +1320,26 @@ mod tests { #[test] fn slot_reuse_after_scope_destroy() { let mut tree = empty_scope_tree(); - tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_block_scope(); - tree.create_local_binding("b".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("b".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.destroy_scope(); - let c = tree.create_local_binding("c".into(), TypeBinding::Inferred(StaticType::Int)); + let c = + tree.create_local_binding("c".into(), TypeBinding::Inferred(StaticType::Int), symbol()); assert_eq!(c, ResolvedVar::Local { slot: 1 }); } #[test] fn get_type_returns_correct_type() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); - tree.create_local_binding("y".into(), TypeBinding::Inferred(StaticType::String)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); + tree.create_local_binding( + "y".into(), + TypeBinding::Inferred(StaticType::String), + symbol(), + ); assert_eq!( tree.get_type(ResolvedVar::Local { slot: 0 }), @@ -1323,7 +1358,7 @@ mod tests { #[test] fn upvalue_hoisting_across_two_function_scopes() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_function_scope(); // outer tree.new_function_scope(); // inner @@ -1350,8 +1385,12 @@ mod tests { #[test] fn multiple_upvalues_get_distinct_indices() { let mut tree = empty_scope_tree(); - tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int)); - tree.create_local_binding("b".into(), TypeBinding::Inferred(StaticType::String)); + tree.create_local_binding("a".into(), TypeBinding::Inferred(StaticType::Int), symbol()); + tree.create_local_binding( + "b".into(), + TypeBinding::Inferred(StaticType::String), + symbol(), + ); tree.new_function_scope(); @@ -1366,7 +1405,7 @@ mod tests { #[test] fn duplicate_upvalue_resolution_reuses_index() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_function_scope(); @@ -1385,7 +1424,7 @@ mod tests { #[test] fn get_type_follows_upvalue_chain() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_function_scope(); // outer tree.new_function_scope(); // inner @@ -1401,7 +1440,7 @@ mod tests { #[test] fn sibling_closure_finds_existing_upvalue() { let mut tree = empty_scope_tree(); - tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int)); + tree.create_local_binding("x".into(), TypeBinding::Inferred(StaticType::Int), symbol()); tree.new_function_scope(); // middle @@ -1509,6 +1548,7 @@ mod tests { let inner_var = tree.create_local_binding( "foo".into(), TypeBinding::Inferred(fn_type(vec![StaticType::Any], StaticType::Any)), + symbol(), ); let ResolvedVar::Local { slot: inner_slot } = inner_var else { panic!("expected local binding"); @@ -1540,6 +1580,7 @@ mod tests { tree.create_local_binding( "foo".into(), TypeBinding::Inferred(fn_type(vec![StaticType::Int], StaticType::Int)), + symbol(), ); // Called with `String`: inner `foo(Int)` doesn't match, falls through