diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs index 37f62942..0bd54eb7 100644 --- a/ndc_analyser/src/analyser.rs +++ b/ndc_analyser/src/analyser.rs @@ -5,8 +5,9 @@ use ndc_core::r#struct::StructRegistry; use ndc_core::{StaticType, TypeSignature}; use ndc_lexer::{NumericLiteral, Span}; use ndc_parser::{ - AugmentedAssignmentPlan, Binding, Candidate, Expression, ExpressionLocation, ForBody, - ForIteration, FunctionParameter, Lvalue, NodeId, TypeExpr, + AssignmentTarget, AssignmentTargetLocation, AugmentedAssignmentPlan, Binding, BindingPattern, + BindingPatternLocation, Candidate, Expression, ExpressionLocation, ForBody, ForIteration, + FunctionParameter, NodeId, TypeExpr, }; use std::cell::RefCell; use std::collections::HashMap; @@ -266,6 +267,7 @@ impl Analyser { Expression::Identifier { name: ident, resolved, + .. } => { if ident == "None" { return Ok(StaticType::Option(Box::new(StaticType::Any))); @@ -324,7 +326,7 @@ impl Analyser { None => self.analyse_or_any(value), }; - self.resolve_lvalue_declarative( + self.resolve_binding_pattern( l_value, annotated_type, found_type.clone(), @@ -333,9 +335,9 @@ impl Analyser { Ok(StaticType::unit()) } Expression::Assignment { l_value, r_value } => { - let old_type = self.resolve_lvalue_or_any(l_value, *span); + let old_type = self.resolve_assignment_target_or_any(l_value, *span); let new_type = self.analyse_with_expected(r_value, &old_type); - self.validate_lvalue_write(l_value, &old_type, &new_type, *span); + self.validate_assignment_write(l_value, &old_type, &new_type, *span); Ok(StaticType::unit()) } @@ -345,7 +347,7 @@ impl Analyser { operation, plan, } => { - let left_type = self.resolve_single_lvalue(l_value, *span)?; + let left_type = self.resolve_single_assignment_target(l_value, *span)?; let right_type = self.analyse_or_any(r_value); let arg_types = vec![left_type.clone(), right_type.clone()]; @@ -424,7 +426,7 @@ impl Analyser { }; if let Some(result_type) = writeback_type { - self.validate_lvalue_write(l_value, &left_type, &result_type, *span); + self.validate_assignment_write(l_value, &left_type, &result_type, *span); } Ok(StaticType::unit()) @@ -476,9 +478,10 @@ impl Analyser { self.return_type_stack.push(None); let param_types = self.resolve_parameters_declarative(&type_signature, *span); - // Fill inferred_type on parameter Lvalues for LSP hints. + // Fill inferred_type on parameter binding patterns for LSP hints. for (p, typ) in parameters.iter_mut().zip(¶m_types) { - if let Lvalue::Identifier { inferred_type, .. } = &mut p.lvalue { + if let BindingPattern::Identifier { inferred_type, .. } = &mut p.lvalue.pattern + { *inferred_type = Some(typ.clone()); } } @@ -747,7 +750,7 @@ impl Analyser { // Higher-order call shapes like `get_function()()` have a non-identifier // function position; in that case we just analyse the callee as a value // and trust the runtime to dispatch. - let Expression::Identifier { name, resolved } = &mut function.expression else { + let Expression::Identifier { name, resolved, .. } = &mut function.expression else { let callee_type = self.analyse_or_any(function); return Ok(match callee_type { StaticType::Function { return_type, .. } => *return_type, @@ -803,7 +806,7 @@ impl Analyser { // TOOD: get this from the AST when the parser adds it let expected_type = None; - self.resolve_lvalue_declarative(l_value, expected_type, found_type, sequence_span); + self.resolve_binding_pattern(l_value, expected_type, found_type, sequence_span); do_destroy = true; } ForIteration::Guard(expr) => { @@ -851,24 +854,24 @@ impl Analyser { out_type } - fn resolve_single_lvalue( + fn resolve_single_assignment_target( &mut self, - lvalue: &mut Lvalue, + lvalue: &mut AssignmentTargetLocation, span: Span, ) -> Result { - if matches!(lvalue, Lvalue::Sequence(_)) { + if matches!(lvalue.target, AssignmentTarget::Sequence(_)) { return Err(AnalysisError::lvalue_required_to_be_single_identifier(span)); } - self.resolve_lvalue(lvalue, span) + self.resolve_assignment_target(lvalue, span) } - fn resolve_lvalue( + fn resolve_assignment_target( &mut self, - lvalue: &mut Lvalue, + lvalue: &mut AssignmentTargetLocation, span: Span, ) -> Result { - match lvalue { - Lvalue::Identifier { + match &mut lvalue.target { + AssignmentTarget::Identifier { identifier, resolved, .. @@ -882,7 +885,7 @@ impl Analyser { *resolved = Some(target); Ok(self.scope_tree.get_type(target).clone()) } - Lvalue::Index { + AssignmentTarget::Index { index, value, resolved_set, @@ -927,13 +930,13 @@ impl Analyser { Ok(StaticType::Any) } } - Lvalue::Sequence(seq) => { + AssignmentTarget::Sequence(seq) => { for sub_lvalue in seq { - self.resolve_lvalue_or_any(sub_lvalue, span); + self.resolve_assignment_target_or_any(sub_lvalue, span); } Ok(StaticType::unit()) } - Lvalue::Member { + AssignmentTarget::Member { receiver, member, member_span, @@ -976,8 +979,12 @@ impl Analyser { } } - fn resolve_lvalue_or_any(&mut self, lvalue: &mut Lvalue, span: Span) -> StaticType { - match self.resolve_lvalue(lvalue, span) { + fn resolve_assignment_target_or_any( + &mut self, + lvalue: &mut AssignmentTargetLocation, + span: Span, + ) -> StaticType { + match self.resolve_assignment_target(lvalue, span) { Ok(t) => t, Err(e) => { self.emit(e); @@ -1016,15 +1023,15 @@ impl Analyser { /// Validate a value that will be stored through an lvalue, widening an /// inferred binding when the location has a stable variable to update. - fn validate_lvalue_write( + fn validate_assignment_write( &mut self, - lvalue: &Lvalue, + lvalue: &AssignmentTargetLocation, stored_type: &StaticType, value_type: &StaticType, span: Span, ) { - match lvalue { - Lvalue::Identifier { + match &lvalue.target { + AssignmentTarget::Identifier { resolved: Some(target), .. } => { @@ -1041,7 +1048,7 @@ impl Analyser { )); } } - Lvalue::Member { .. } => { + AssignmentTarget::Member { .. } => { if !value_type.is_subtype(stored_type) { self.emit(AnalysisError::mismatched_types( value_type, @@ -1050,7 +1057,7 @@ impl Analyser { )); } } - Lvalue::Index { value, index, .. } => { + AssignmentTarget::Index { value, index, .. } => { if value_type.is_subtype(stored_type) { return; } @@ -1106,7 +1113,8 @@ impl Analyser { span, )); } - Lvalue::Identifier { resolved: None, .. } | Lvalue::Sequence(_) => {} + AssignmentTarget::Identifier { resolved: None, .. } | AssignmentTarget::Sequence(_) => { + } } } @@ -1141,15 +1149,15 @@ impl Analyser { .map(|param| param.type_name.clone()) .collect() } - fn resolve_lvalue_declarative( + fn resolve_binding_pattern( &mut self, - lvalue: &mut Lvalue, + lvalue: &mut BindingPatternLocation, expected_type: Option, found_type: StaticType, span: Span, ) { - match lvalue { - Lvalue::Identifier { + match &mut lvalue.pattern { + BindingPattern::Identifier { identifier, resolved, inferred_type, @@ -1178,13 +1186,9 @@ impl Analyser { *inferred_type = Some(type_binding.typ().clone()) } - Lvalue::Index { index, value, .. } => { - self.analyse_or_any(index); - self.analyse_or_any(value); - } - Lvalue::Sequence(seq) => { + BindingPattern::Sequence(seq) => { // If the type is a fixed-length Tuple whose arity doesn't match - // the number of lvalues, fall back to Any for each element. This + // the number of bindings, fall back to Any for each element. This // can happen when a variable is declared with one type (e.g. ()) // and later reassigned to a tuple of a different arity — the // analyser doesn't track reassignment types. @@ -1225,7 +1229,7 @@ impl Analyser { } else { None }; - self.resolve_lvalue_declarative( + self.resolve_binding_pattern( sub_lvalue, sub_expected, found_type.clone(), @@ -1239,9 +1243,6 @@ impl Analyser { self.emit(AnalysisError::unable_to_unpack_type(&found_type, span)); } } - Lvalue::Member { receiver, .. } => { - self.analyse_or_any(receiver); - } } } fn analyse_multiple_expression_with_same_type( diff --git a/ndc_lsp/src/features/definition.rs b/ndc_lsp/src/features/definition.rs index 9aaff897..c16f104f 100644 --- a/ndc_lsp/src/features/definition.rs +++ b/ndc_lsp/src/features/definition.rs @@ -38,9 +38,9 @@ pub fn goto_definition(state: &DocumentState, position: Position, uri: Url) -> O /// The identifier name the cursor is on, plus the document's `SourceId`. /// /// Handles both expression uses (`x`) and assignment targets (`x = 2`). A -/// reassignment target is an [`ndc_parser::Lvalue`], not an expression node, so +/// reassignment target is an [`ndc_parser::AssignmentTargetLocation`], so /// `node_at_offset` returns the enclosing assignment for it; we then look up the -/// lvalue identifier directly. (Declaration/parameter/loop-variable lvalues are +/// target identifier directly. (Declaration/parameter/loop-variable patterns are /// found too, but they aren't visible to themselves, so they resolve to nothing.) fn identifier_at( ast: &[ndc_parser::ExpressionLocation], @@ -51,7 +51,7 @@ fn identifier_at( { return Some((name.clone(), node.span.source_id())); } - let mut finder = LvalueIdentFinder { + let mut finder = BindingIdentFinder { offset, found: None, }; @@ -59,14 +59,14 @@ fn identifier_at( finder.found } -/// Finds the lvalue identifier whose span contains the cursor. Lvalue -/// identifiers don't overlap, so the one containing the offset is unambiguous. -struct LvalueIdentFinder { +/// Finds the pattern or target identifier whose span contains the cursor. +/// Identifier spans do not overlap, so the match is unambiguous. +struct BindingIdentFinder { offset: usize, found: Option<(String, SourceId)>, } -impl AstVisitor for LvalueIdentFinder { +impl AstVisitor for BindingIdentFinder { fn on_declaration( &mut self, identifier: &str, @@ -193,7 +193,7 @@ mod tests { #[test] fn jump_from_reassignment_target_to_declaration() { - // The `x` in `x = 2` is a use in write position (an lvalue, not an + // The `x` in `x = 2` is a use in write position (an assignment target, not an // expression node), but should still jump to its declaration. let src = "let x = 1;\nx = 2;"; let state = analyse(src); @@ -201,4 +201,28 @@ mod tests { let loc = def_at(&state, target).expect("definition found"); assert_eq!(start_offset(&state, &loc), 4); // `let x` at byte 4 } + + #[test] + fn grouped_destructuring_target_resolves_to_precise_binding_span() { + let src = "let ((left), [right]) = (1, [2]);\n((left), [right]) = (3, [4]);"; + let state = analyse(src); + for name in ["left", "right"] { + let loc = def_at(&state, src.rfind(name).unwrap()).expect("definition found"); + assert_eq!(start_offset(&state, &loc), src.find(name).unwrap()); + assert_eq!( + state.line_index.offset(&state.source, loc.range.end), + Some(src.find(name).unwrap() + name.len()), + ); + } + } + + #[test] + fn indexed_assignment_visits_receiver_and_index_expressions() { + let src = "let values = [1]; let index = 0; values[index] = 2;"; + let state = analyse(src); + for name in ["values", "index"] { + let loc = def_at(&state, src.rfind(name).unwrap()).expect("definition found"); + assert_eq!(start_offset(&state, &loc), src.find(name).unwrap()); + } + } } diff --git a/ndc_lsp/src/features/hover.rs b/ndc_lsp/src/features/hover.rs index c922ae96..dc2b8296 100644 --- a/ndc_lsp/src/features/hover.rs +++ b/ndc_lsp/src/features/hover.rs @@ -29,7 +29,7 @@ pub fn hover( let node = node_at_offset(&state.ast, offset)?; let markdown = match &node.expression { - Expression::Identifier { name, resolved } if resolves_to_global(resolved) => { + Expression::Identifier { name, resolved, .. } if resolves_to_global(resolved) => { function_hover(name, functions).or_else(|| type_hover(state, node.id)) } _ => type_hover(state, node.id), diff --git a/ndc_lsp/src/features/symbols.rs b/ndc_lsp/src/features/symbols.rs index 08df7e6f..25463b76 100644 --- a/ndc_lsp/src/features/symbols.rs +++ b/ndc_lsp/src/features/symbols.rs @@ -1,6 +1,8 @@ use ndc_core::StaticType; use ndc_lexer::Span; -use ndc_parser::{Expression, ExpressionLocation, FunctionParameter, Lvalue}; +use ndc_parser::{ + BindingPattern, BindingPatternLocation, Expression, ExpressionLocation, FunctionParameter, +}; use tower_lsp::lsp_types::{DocumentSymbol, SymbolKind}; use crate::util::LineIndex; @@ -53,7 +55,7 @@ fn collect_symbol( )); } Expression::VariableDeclaration { l_value, value, .. } => { - push_lvalue_symbols(l_value, expr.span, text, line_index, out); + push_pattern_symbols(l_value, expr.span, text, line_index, out); // A lambda bound to a variable should still appear in the outline. collect_symbol(value, text, line_index, out); } @@ -110,15 +112,15 @@ fn collect_children( } } -fn push_lvalue_symbols( - lvalue: &Lvalue, +fn push_pattern_symbols( + pattern: &BindingPatternLocation, decl_span: Span, text: &str, line_index: &LineIndex, out: &mut Vec, ) { - match lvalue { - Lvalue::Identifier { + match &pattern.pattern { + BindingPattern::Identifier { identifier, span, inferred_type, @@ -135,12 +137,11 @@ fn push_lvalue_symbols( Vec::new(), )); } - Lvalue::Sequence(lvalues) => { - for lv in lvalues { - push_lvalue_symbols(lv, decl_span, text, line_index, out); + BindingPattern::Sequence(patterns) => { + for pattern in patterns { + push_pattern_symbols(pattern, decl_span, text, line_index, out); } } - Lvalue::Index { .. } | Lvalue::Member { .. } => {} } } @@ -148,8 +149,8 @@ fn push_lvalue_symbols( fn signature(parameters: &[FunctionParameter], return_type: Option<&StaticType>) -> String { let params = parameters .iter() - .map(|p| match &p.lvalue { - Lvalue::Identifier { identifier, .. } => identifier.clone(), + .map(|p| match &p.lvalue.pattern { + BindingPattern::Identifier { identifier, .. } => identifier.clone(), _ => "_".to_string(), }) .collect::>() diff --git a/ndc_lsp/src/scope_resolve.rs b/ndc_lsp/src/scope_resolve.rs index 106803be..ec22a058 100644 --- a/ndc_lsp/src/scope_resolve.rs +++ b/ndc_lsp/src/scope_resolve.rs @@ -13,7 +13,9 @@ //! stays. use ndc_lexer::{SourceId, Span}; -use ndc_parser::{Expression, ExpressionLocation, ForBody, ForIteration, Lvalue}; +use ndc_parser::{ + BindingPattern, BindingPatternLocation, Expression, ExpressionLocation, ForBody, ForIteration, +}; /// A declaration discovered while walking the AST. pub struct Decl { @@ -68,7 +70,7 @@ fn collect(expr: &ExpressionLocation, scope: Span, out: &mut Vec) { Expression::VariableDeclaration { l_value, value, .. } => { // Visible only after the initializer, so `let x = x` resolves the RHS // to an outer binding rather than itself. - push_lvalue(l_value, scope, value.span.end(), out); + push_pattern(l_value, scope, value.span.end(), out); collect(value, scope, out); } Expression::FunctionDeclaration { @@ -92,7 +94,7 @@ fn collect(expr: &ExpressionLocation, scope: Span, out: &mut Vec) { // visible throughout it (the body follows the parameter list). let body_scope = body.span; for p in parameters { - push_lvalue(&p.lvalue, body_scope, p.span.offset(), out); + push_pattern(&p.lvalue, body_scope, p.span.offset(), out); } collect(body, body_scope, out); } @@ -130,7 +132,7 @@ fn collect(expr: &ExpressionLocation, scope: Span, out: &mut Vec) { for iteration in iterations { match iteration { ForIteration::Iteration { l_value, sequence } => { - push_lvalue(l_value, loop_scope, sequence.span.end(), out); + push_pattern(l_value, loop_scope, sequence.span.end(), out); collect(sequence, scope, out); } ForIteration::Guard(e) => collect(e, loop_scope, out), @@ -223,9 +225,14 @@ fn collect(expr: &ExpressionLocation, scope: Span, out: &mut Vec) { } } -fn push_lvalue(lvalue: &Lvalue, scope: Span, visible_from: usize, out: &mut Vec) { - match lvalue { - Lvalue::Identifier { +fn push_pattern( + pattern: &BindingPatternLocation, + scope: Span, + visible_from: usize, + out: &mut Vec, +) { + match &pattern.pattern { + BindingPattern::Identifier { identifier, span, .. } => out.push(Decl { name: identifier.clone(), @@ -234,11 +241,10 @@ fn push_lvalue(lvalue: &Lvalue, scope: Span, visible_from: usize, out: &mut Vec< visible_from, is_function: false, }), - Lvalue::Sequence(lvalues) => { - for lv in lvalues { - push_lvalue(lv, scope, visible_from, out); + BindingPattern::Sequence(patterns) => { + for pattern in patterns { + push_pattern(pattern, scope, visible_from, out); } } - Lvalue::Index { .. } | Lvalue::Member { .. } => {} } } diff --git a/ndc_lsp/src/visitor.rs b/ndc_lsp/src/visitor.rs index cd0cf37b..8b9e3afe 100644 --- a/ndc_lsp/src/visitor.rs +++ b/ndc_lsp/src/visitor.rs @@ -1,6 +1,9 @@ use ndc_core::StaticType; 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, +}; /// Trait for visiting interesting nodes during an AST walk. /// @@ -78,14 +81,13 @@ fn find_node_at<'a>( /// The expression-typed children of a node. Mirrors the structure walked by /// [`walk_expression`], but returns references so callers can search for a node -/// rather than visiting via the [`AstVisitor`] trait. Lvalue (declaration) -/// positions are not expression children and are intentionally omitted, except -/// for the expression operands inside an `Lvalue::Index`. +/// rather than visiting via the [`AstVisitor`] trait. Binding patterns are not +/// expression children. Assignment targets contribute their receiver and index +/// expressions. fn child_expressions(expr: &ExpressionLocation) -> Vec<&ExpressionLocation> { let mut out: Vec<&ExpressionLocation> = Vec::new(); match &expr.expression { - Expression::VariableDeclaration { l_value, value, .. } => { - push_lvalue_index(l_value, &mut out); + Expression::VariableDeclaration { value, .. } => { out.push(value); } Expression::FunctionDeclaration { body, .. } => out.push(body), @@ -114,8 +116,7 @@ fn child_expressions(expr: &ExpressionLocation) -> Vec<&ExpressionLocation> { Expression::For { iterations, body } => { for iteration in iterations { match iteration { - ForIteration::Iteration { l_value, sequence } => { - push_lvalue_index(l_value, &mut out); + ForIteration::Iteration { sequence, .. } => { out.push(sequence); } ForIteration::Guard(e) => out.push(e), @@ -148,7 +149,7 @@ fn child_expressions(expr: &ExpressionLocation) -> Vec<&ExpressionLocation> { | Expression::OpAssignment { l_value, r_value, .. } => { - push_lvalue_index(l_value, &mut out); + push_target_expressions(l_value, &mut out); out.push(r_value); } Expression::Call { @@ -196,21 +197,23 @@ fn child_expressions(expr: &ExpressionLocation) -> Vec<&ExpressionLocation> { out } -/// Push the expression operands of an `Lvalue::Index` (the indexed value and -/// the index expression) so they participate in position lookup. -fn push_lvalue_index<'a>(lvalue: &'a Lvalue, out: &mut Vec<&'a ExpressionLocation>) { - match lvalue { - Lvalue::Index { value, index, .. } => { +/// Push receiver and index expressions so they participate in position lookup. +fn push_target_expressions<'a>( + target: &'a AssignmentTargetLocation, + out: &mut Vec<&'a ExpressionLocation>, +) { + match &target.target { + AssignmentTarget::Index { value, index, .. } => { out.push(value); out.push(index); } - Lvalue::Member { receiver, .. } => out.push(receiver), - Lvalue::Sequence(lvalues) => { - for lv in lvalues { - push_lvalue_index(lv, out); + AssignmentTarget::Member { receiver, .. } => out.push(receiver), + AssignmentTarget::Sequence(targets) => { + for target in targets { + push_target_expressions(target, out); } } - Lvalue::Identifier { .. } => {} + AssignmentTarget::Identifier { .. } => {} } } @@ -222,7 +225,7 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { annotated_type, value, } => { - walk_lvalue(visitor, l_value, annotated_type.is_some()); + walk_pattern(visitor, l_value, annotated_type.is_some()); walk_expression(visitor, value); } Expression::FunctionDeclaration { @@ -233,7 +236,7 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { .. } => { for p in parameters { - walk_lvalue(visitor, &p.lvalue, p.annotation.is_some()); + walk_pattern(visitor, &p.lvalue, p.annotation.is_some()); } visitor.on_function_declaration( resolved_return_type.as_ref(), @@ -274,7 +277,7 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { for iteration in iterations { match iteration { ForIteration::Iteration { l_value, sequence } => { - walk_lvalue(visitor, l_value, false); + walk_pattern(visitor, l_value, false); walk_expression(visitor, sequence); } ForIteration::Guard(expr) => walk_expression(visitor, expr), @@ -309,7 +312,7 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { | Expression::OpAssignment { l_value, r_value, .. } => { - walk_lvalue(visitor, l_value, false); + walk_target(visitor, l_value); walk_expression(visitor, r_value); } Expression::Call { @@ -362,25 +365,45 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { } } -fn walk_lvalue(visitor: &mut impl AstVisitor, lvalue: &Lvalue, has_annotation: bool) { - match lvalue { - Lvalue::Identifier { +fn walk_target(visitor: &mut impl AstVisitor, target: &AssignmentTargetLocation) { + match &target.target { + AssignmentTarget::Identifier { identifier, inferred_type, span, .. } => { - visitor.on_declaration(identifier, inferred_type.as_ref(), has_annotation, *span); + visitor.on_declaration(identifier, inferred_type.as_ref(), false, *span); } - Lvalue::Sequence(lvalues) => { - for lv in lvalues { - walk_lvalue(visitor, lv, has_annotation); + AssignmentTarget::Sequence(targets) => { + for target in targets { + walk_target(visitor, target); } } - Lvalue::Index { value, index, .. } => { + AssignmentTarget::Index { value, index, .. } => { walk_expression(visitor, value); walk_expression(visitor, index); } - Lvalue::Member { receiver, .. } => walk_expression(visitor, receiver), + AssignmentTarget::Member { receiver, .. } => walk_expression(visitor, receiver), + } +} + +fn walk_pattern( + visitor: &mut impl AstVisitor, + pattern: &BindingPatternLocation, + has_annotation: bool, +) { + match &pattern.pattern { + BindingPattern::Identifier { + identifier, + inferred_type, + span, + .. + } => visitor.on_declaration(identifier, inferred_type.as_ref(), has_annotation, *span), + BindingPattern::Sequence(patterns) => { + for pattern in patterns { + walk_pattern(visitor, pattern, has_annotation); + } + } } } diff --git a/ndc_parser/src/expression.rs b/ndc_parser/src/expression.rs index 9c3c8670..22496511 100644 --- a/ndc_parser/src/expression.rs +++ b/ndc_parser/src/expression.rs @@ -97,6 +97,11 @@ pub enum Expression { Identifier { name: String, resolved: Binding, + /// The identifier token's range. When parsing `(foo)`, the parser reuses + /// the identifier expression and widens `ExpressionLocation.span` to + /// include the parentheses. This field still covers only `foo`, so a + /// binding pattern can use it as the go-to-definition destination. + identifier_span: Span, }, Statement(Box), Logical { @@ -114,16 +119,16 @@ pub enum Expression { requires_check: bool, }, VariableDeclaration { - l_value: Lvalue, + l_value: BindingPatternLocation, annotated_type: Option, value: Box, }, Assignment { - l_value: Lvalue, + l_value: AssignmentTargetLocation, r_value: Box, }, OpAssignment { - l_value: Lvalue, + l_value: AssignmentTargetLocation, r_value: Box, operation: String, plan: AugmentedAssignmentPlan, @@ -204,7 +209,7 @@ pub enum Expression { #[derive(Debug, Eq, PartialEq, Clone)] pub enum ForIteration { Iteration { - l_value: Lvalue, + l_value: BindingPatternLocation, sequence: ExpressionLocation, }, Guard(ExpressionLocation), @@ -234,7 +239,7 @@ pub struct StructField { #[derive(Debug, Eq, PartialEq, Clone)] pub struct FunctionParameter { - pub lvalue: Lvalue, + pub lvalue: BindingPatternLocation, pub annotation: Option, pub resolved_type: Option, pub span: Span, @@ -246,7 +251,7 @@ impl FunctionParameter { params .iter() .map(|p| { - let Lvalue::Identifier { identifier, .. } = &p.lvalue else { + let BindingPattern::Identifier { identifier, .. } = &p.lvalue.pattern else { unreachable!( "parameter list may only contain identifiers {:?} found.", p.lvalue @@ -260,8 +265,8 @@ impl FunctionParameter { } #[derive(Debug, Eq, PartialEq, Clone)] -pub enum Lvalue { - // Example: `let foo = ...` +pub enum AssignmentTarget { + // Example: `foo = ...` Identifier { identifier: String, resolved: Option, @@ -275,8 +280,8 @@ pub enum Lvalue { resolved_set: Option, resolved_get: Option, }, - // Example: `let a, b = ...` - Sequence(Vec), + // Example: `a, b = ...` + Sequence(Vec), // Example: `a.b = ...` Member { receiver: Box, @@ -320,14 +325,14 @@ impl ExpressionLocation { pub fn as_identifier(&self) -> &str { match &self.expression { - Expression::Identifier { name, resolved: _ } => name, + Expression::Identifier { name, .. } => name, _ => panic!("the parser should have guaranteed us the right type of expression"), } } pub fn to_identifier(self) -> String { match self.expression { - Expression::Identifier { name, resolved: _ } => name, + Expression::Identifier { name, .. } => name, _ => panic!("the parser should have guaranteed us the right type of expression"), } } @@ -356,7 +361,7 @@ impl ExpressionLocation { } } -impl Lvalue { +impl AssignmentTarget { #[must_use] pub fn can_build_from_expression(expression: &Expression) -> bool { match expression { @@ -368,7 +373,7 @@ impl Lvalue { Expression::List { values } | Expression::Tuple { values } => values .iter() .all(|el| Self::can_build_destructure_from_expression(&el.expression)), - // Parentheses around an lvalue are transparent: `(s.x) = 5` writes + // Parentheses around a target are transparent: `(s.x) = 5` writes // the same location as `s.x = 5`. Expression::Grouping(inner) => Self::can_build_from_expression(&inner.expression), _ => false, @@ -387,69 +392,130 @@ impl Lvalue { expression => Self::can_build_from_expression(expression), } } +} - /// The first target in this lvalue that writes through an existing value - /// rather than binding a new name, if any. - #[must_use] - pub fn non_binding_target(&self) -> Option { - match self { - Self::Identifier { .. } => None, - Self::Index { .. } => Some(NonBindingTarget::Index), - Self::Member { .. } => Some(NonBindingTarget::Member), - Self::Sequence(items) => items.iter().find_map(Self::non_binding_target), - } - } +/// A declaration pattern, with identity independent of its source location. +#[derive(Debug, Eq, PartialEq, Clone)] +pub struct BindingPatternLocation { + pub id: NodeId, + pub pattern: BindingPattern, + pub span: Span, +} - pub fn new_identifier(identifier: String, span: Span) -> Self { - Self::Identifier { - identifier, - resolved: None, - span, - inferred_type: None, - } +/// Syntax that introduces names. Index and member writes are not declarations. +#[derive(Debug, Eq, PartialEq, Clone)] +pub enum BindingPattern { + Identifier { + identifier: String, + resolved: Option, + /// The identifier token, which may be narrower than the pattern's span. + span: Span, + inferred_type: Option, + }, + Sequence(Vec), +} + +/// A destination for a write, with the complete target's source range. +#[derive(Debug, Eq, PartialEq, Clone)] +pub struct AssignmentTargetLocation { + pub id: NodeId, + pub target: AssignmentTarget, + pub span: Span, +} + +impl TryFrom for BindingPatternLocation { + type Error = NonBindingTarget; + + fn try_from(value: AssignmentTargetLocation) -> Result { + let pattern = match value.target { + AssignmentTarget::Identifier { + identifier, + resolved, + span, + inferred_type, + } => BindingPattern::Identifier { + identifier, + resolved, + span, + inferred_type, + }, + AssignmentTarget::Sequence(items) => BindingPattern::Sequence( + items + .into_iter() + .map(Self::try_from) + .collect::>()?, + ), + AssignmentTarget::Index { .. } => return Err(NonBindingTarget::Index), + AssignmentTarget::Member { .. } => return Err(NonBindingTarget::Member), + }; + Ok(Self { + id: value.id, + pattern, + span: value.span, + }) } } -impl TryFrom for Lvalue { +impl TryFrom for AssignmentTargetLocation { type Error = ParseError; + /// Convert a parsed expression into an assignment target, reusing its NodeId + /// and source span. For `(object.field)`, remove the Grouping wrapper and use + /// its ID and span for the resulting member target. Discard the inner member + /// node's ID; keep the receiver expression and its ID. fn try_from(value: ExpressionLocation) -> Result { - match value.expression { - Expression::Identifier { name, .. } => Ok(Self::new_identifier(name, value.span)), + let target = match value.expression { + Expression::Identifier { + name, + identifier_span, + .. + } => AssignmentTarget::Identifier { + identifier: name, + resolved: None, + span: identifier_span, + inferred_type: None, + }, Expression::Call { function, mut arguments, } if is_index_call(&function, &arguments) => { let index = arguments.remove(1); let container = arguments.remove(0); - Ok(Self::Index { + AssignmentTarget::Index { value: Box::new(container), index: Box::new(index), resolved_set: None, resolved_get: None, - }) + } } Expression::MemberAccess { receiver, member, member_span, .. - } => Ok(Self::Member { + } => AssignmentTarget::Member { receiver, member, member_span, resolved_getter: None, resolved_setter: None, - }), - Expression::List { values } | Expression::Tuple { values } => Ok(Self::Sequence( - values - .into_iter() - .map(Self::try_from) - .collect::, Self::Error>>()?, - )), - Expression::Grouping(value) => Self::try_from(*value), - _expr => Err(ParseError::text("invalid l-value".to_string(), value.span)), - } + }, + Expression::List { values } | Expression::Tuple { values } => { + AssignmentTarget::Sequence( + values + .into_iter() + .map(Self::try_from) + .collect::>()?, + ) + } + Expression::Grouping(inner) => Self::try_from(*inner)?.target, + _ => return Err(ParseError::text("invalid l-value".to_string(), value.span)), + }; + Ok(Self { + id: value.id, + target, + span: value.span, + }) } } diff --git a/ndc_parser/src/lib.rs b/ndc_parser/src/lib.rs index f5fc4f87..0d6dfeb4 100644 --- a/ndc_parser/src/lib.rs +++ b/ndc_parser/src/lib.rs @@ -4,8 +4,9 @@ mod parser; mod type_expr; pub use expression::{ - AugmentedAssignmentPlan, Binding, Candidate, CaptureSource, Expression, ExpressionLocation, - ForBody, ForIteration, FunctionParameter, Lvalue, NodeId, ResolvedVar, + AssignmentTarget, AssignmentTargetLocation, AugmentedAssignmentPlan, Binding, BindingPattern, + BindingPatternLocation, Candidate, CaptureSource, Expression, ExpressionLocation, ForBody, + ForIteration, FunctionParameter, NodeId, ResolvedVar, }; pub use operator::{BinaryOperator, LogicalOperator, UnaryOperator}; pub use parser::Error; diff --git a/ndc_parser/src/parser.rs b/ndc_parser/src/parser.rs index 095cc9ed..1d8070ee 100644 --- a/ndc_parser/src/parser.rs +++ b/ndc_parser/src/parser.rs @@ -1,8 +1,9 @@ use std::fmt::Write; use crate::expression::{ - AugmentedAssignmentPlan, Binding, ExpressionLocation, ForBody, ForIteration, FunctionParameter, - Lvalue, NodeId, NonBindingTarget, + AssignmentTarget, AssignmentTargetLocation, AugmentedAssignmentPlan, Binding, BindingPattern, + BindingPatternLocation, ExpressionLocation, ForBody, ForIteration, FunctionParameter, NodeId, + NonBindingTarget, }; use crate::expression::{Expression, StructField}; use crate::operator::{BinaryOperator, LogicalOperator, UnaryOperator}; @@ -210,6 +211,7 @@ impl Parser { Expression::Identifier { name: operator_token_loc.token.to_string(), resolved: Binding::None, + identifier_span: operator_token_loc.span, } .to_location(operator_token_loc.span), ), @@ -223,6 +225,7 @@ impl Parser { Expression::Identifier { name: not_token.token.to_string(), resolved: Binding::None, + identifier_span: not_token.span, } .to_location(not_token.span), ), @@ -255,6 +258,7 @@ impl Parser { Expression::Identifier { name: operator.to_string(), resolved: Binding::None, + identifier_span: operator_span, } .to_location(operator_span), ), @@ -356,7 +360,7 @@ impl Parser { let maybe_lvalue = self.tuple_expression(Self::single_expression, false)?; let start = maybe_lvalue.span; - if !Lvalue::can_build_from_expression(&maybe_lvalue.expression) { + if !AssignmentTarget::can_build_from_expression(&maybe_lvalue.expression) { // In this case we got some kind of expression that we can't assign to. We can just return the expression as is. // But to improve error handling and stuff it would be nice if we could check if the next token matches one // of the assignment operator and throw an appropriate error. @@ -371,14 +375,13 @@ impl Parser { } match self.peek_current_token() { - // NOTE: the parser supports every LValue but some might cause an error when declaring vars Some(Token::EqualsSign) => { self.advance(); let expression = self.tuple_expression(Self::single_expression, false)?; let end = expression.span; let assignment_expression = Expression::Assignment { - l_value: Lvalue::try_from(maybe_lvalue) - .expect("guaranteed to produce an lvalue"), + l_value: AssignmentTargetLocation::try_from(maybe_lvalue) + .expect("guaranteed to produce an assignment target"), r_value: Box::new(expression), }; @@ -391,8 +394,8 @@ impl Parser { let expression = self.tuple_expression(Self::single_expression, false)?; let end = expression.span; let op_assign = Expression::OpAssignment { - l_value: Lvalue::try_from(maybe_lvalue) - .expect("guaranteed to produce an lvalue"), + l_value: AssignmentTargetLocation::try_from(maybe_lvalue) + .expect("guaranteed to produce an assignment target"), r_value: Box::new(expression), operation: operation_identifier, plan: AugmentedAssignmentPlan::Unresolved, @@ -523,6 +526,7 @@ impl Parser { Expression::Identifier { name: operator_token_loc.token.to_string(), resolved: Binding::None, + identifier_span: operator_span, } .to_location(operator_span), ), @@ -670,6 +674,7 @@ impl Parser { Expression::Identifier { name: operator_token_loc.token.to_string(), resolved: Binding::None, + identifier_span: token_span, } .to_location(token_span), ), @@ -718,6 +723,7 @@ impl Parser { Expression::Identifier { name: member, resolved: Binding::None, + identifier_span: member_span, } .to_location(member_span), ), @@ -747,8 +753,8 @@ impl Parser { self.require_current_token_matches(&Token::Dot)?; // consume matched token let l_value = self.require_identifier()?; let identifier_span = l_value.span; - let identifier = Lvalue::try_from(l_value)?; - let Lvalue::Identifier { identifier, .. } = identifier else { + let identifier = AssignmentTargetLocation::try_from(l_value)?; + let AssignmentTarget::Identifier { identifier, .. } = identifier.target else { unreachable!("Guaranteed to match by previous call to require_identifier") }; @@ -816,6 +822,7 @@ impl Parser { Expression::Identifier { name: "[]".to_string(), resolved: Binding::None, + identifier_span: bracket_span, } .to_location(bracket_span), ), @@ -958,9 +965,9 @@ impl Parser { fn for_iteration(&mut self) -> Result { let maybe_lvalue = self.tuple_expression(Self::primary, false)?; let lvalue_span = maybe_lvalue.span; - let l_value = Lvalue::try_from(maybe_lvalue)?; + let l_value = AssignmentTargetLocation::try_from(maybe_lvalue)?; - if let Some(target) = l_value.non_binding_target() { + let l_value = BindingPatternLocation::try_from(l_value).map_err(|target| { let help = match target { NonBindingTarget::Member => { "A for loop introduces a new binding for each iteration; a struct field like `foo.bar` cannot be an iteration variable." @@ -969,12 +976,12 @@ impl Parser { "A for loop introduces a new binding for each iteration; an indexed element like `foo[index]` cannot be an iteration variable." } }; - return Err(Error::with_help( + Error::with_help( "Invalid iteration variable".to_string(), lvalue_span, help.to_string(), - )); - } + ) + })?; self.require_current_token_matches(&Token::In)?; @@ -1073,6 +1080,7 @@ impl Parser { Token::Identifier(identifier) => Expression::Identifier { name: identifier, resolved: Binding::None, + identifier_span: token_location.span, }, _ => { return Err(Error::text( @@ -1661,7 +1669,15 @@ impl Parser { let maybe_lvalue = self.single_expression()?; let lvalue_span = maybe_lvalue.span; - let Ok(lvalue @ Lvalue::Identifier { .. }) = Lvalue::try_from(maybe_lvalue) else { + let Some( + lvalue @ BindingPatternLocation { + pattern: BindingPattern::Identifier { .. }, + .. + }, + ) = AssignmentTargetLocation::try_from(maybe_lvalue) + .ok() + .and_then(|target| BindingPatternLocation::try_from(target).ok()) + else { return Err(Error::with_help( "Expected parameter name".to_string(), lvalue_span, @@ -1690,11 +1706,11 @@ impl Parser { }) } - pub fn named_binding(&mut self) -> Result<(Lvalue, Option), Error> { + pub fn named_binding(&mut self) -> Result<(BindingPatternLocation, Option), Error> { let maybe_lvalue = self.tuple_expression(Self::single_expression, false)?; let lvalue_span = maybe_lvalue.span; - let Ok(lvalue) = Lvalue::try_from(maybe_lvalue) else { + let Ok(lvalue) = AssignmentTargetLocation::try_from(maybe_lvalue) else { return Err(Error::with_help( "Invalid assignment target".to_string(), lvalue_span, @@ -1702,7 +1718,7 @@ impl Parser { )); }; - if let Some(target) = lvalue.non_binding_target() { + let lvalue = BindingPatternLocation::try_from(lvalue).map_err(|target| { let help = match target { NonBindingTarget::Member => { "`let` introduces a new binding; assign to a field with `foo.bar = value` instead." @@ -1711,12 +1727,12 @@ impl Parser { "`let` introduces a new binding; assign to an element with `foo[index] = value` instead." } }; - return Err(Error::with_help( + Error::with_help( "Invalid declaration target".to_string(), lvalue_span, help.to_string(), - )); - } + ) + })?; let annotated_type = if self.peek_current_token() == Some(&Token::Colon) { self.advance(); diff --git a/ndc_parser/tests/member_access.rs b/ndc_parser/tests/member_access.rs index f2540f9a..8dc3400c 100644 --- a/ndc_parser/tests/member_access.rs +++ b/ndc_parser/tests/member_access.rs @@ -1,7 +1,7 @@ #![allow(unused_crate_dependencies)] use ndc_lexer::{Lexer, SourceId}; -use ndc_parser::{Expression, ExpressionLocation, Lvalue, Parser}; +use ndc_parser::{AssignmentTarget, Expression, ExpressionLocation, Parser}; fn parse_one(source: &str) -> ExpressionLocation { let tokens = Lexer::new(source, SourceId::SYNTHETIC) @@ -58,7 +58,7 @@ fn invoked_dot_is_call_with_receiver_as_first_argument() { } #[test] -fn member_assignment_has_a_member_lvalue() { +fn member_assignment_has_a_member_target() { let expression = parse_one("foo.bar = value"); let Expression::Assignment { l_value, .. } = expression.expression else { @@ -66,8 +66,8 @@ fn member_assignment_has_a_member_lvalue() { }; assert!(matches!( - l_value, - Lvalue::Member { + l_value.target, + AssignmentTarget::Member { member, receiver, .. @@ -101,7 +101,7 @@ fn member_destructuring_is_not_assignable() { } #[test] -fn member_augmented_assignment_has_a_member_lvalue() { +fn member_augmented_assignment_has_a_member_target() { let expression = parse_one("foo.bar += value"); let Expression::OpAssignment { @@ -116,8 +116,8 @@ fn member_augmented_assignment_has_a_member_lvalue() { assert_eq!(operation, "+"); assert!(matches!( - l_value, - Lvalue::Member { member, .. } if member == "bar" + l_value.target, + AssignmentTarget::Member { member, .. } if member == "bar" )); } diff --git a/ndc_parser/tests/targets.rs b/ndc_parser/tests/targets.rs new file mode 100644 index 00000000..3d40bcad --- /dev/null +++ b/ndc_parser/tests/targets.rs @@ -0,0 +1,205 @@ +#![allow(unused_crate_dependencies)] + +use std::collections::HashSet; + +use ndc_lexer::{Lexer, SourceId, Span}; +use ndc_parser::{ + AssignmentTarget, AssignmentTargetLocation, BindingPattern, BindingPatternLocation, Expression, + ExpressionLocation, ForIteration, NodeId, Parser, +}; + +fn parse_one(source: &str) -> ExpressionLocation { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .unwrap(); + let mut expressions = Parser::from_tokens(tokens).parse().unwrap(); + assert_eq!(expressions.len(), 1); + expressions.pop().unwrap() +} + +fn text(source: &str, span: Span) -> &str { + assert_eq!(span.source_id(), SourceId::SYNTHETIC); + &source[span.range()] +} + +fn pattern_nodes(pattern: &BindingPatternLocation, nodes: &mut Vec<(NodeId, Span)>) { + nodes.push((pattern.id, pattern.span)); + if let BindingPattern::Sequence(items) = &pattern.pattern { + for item in items { + pattern_nodes(item, nodes); + } + } +} + +#[test] +fn nested_binding_patterns_have_distinct_ids_and_complete_spans() { + let source = "let [(a), (b, [c])] = input"; + let expression = parse_one(source); + let Expression::VariableDeclaration { l_value, value, .. } = expression.expression else { + panic!("expected declaration"); + }; + let mut nodes = vec![]; + pattern_nodes(&l_value, &mut nodes); + assert_eq!( + nodes + .iter() + .map(|(_, span)| text(source, *span)) + .collect::>(), + ["[(a), (b, [c])]", "(a)", "(b, [c])", "b", "[c]", "c"] + ); + let ids: HashSet<_> = nodes + .iter() + .map(|(id, _)| *id) + .chain([expression.id, value.id]) + .collect(); + assert_eq!(ids.len(), nodes.len() + 2); + let BindingPattern::Sequence(items) = l_value.pattern else { + unreachable!() + }; + let BindingPattern::Identifier { span, .. } = items[0].pattern else { + unreachable!() + }; + assert_eq!(text(source, span), "a"); +} + +#[test] +fn assignment_conversion_preserves_ids_and_index_expression_children() { + let source = "[(a), values[next()]]"; + let expression = parse_one(source); + let id = expression.id; + let Expression::List { values } = &expression.expression else { + panic!("expected list") + }; + let a_id = values[0].id; + let index_id = values[1].id; + let Expression::Call { arguments, .. } = &values[1].expression else { + panic!("expected index call") + }; + let receiver_id = arguments[0].id; + let subscript_id = arguments[1].id; + + let target = AssignmentTargetLocation::try_from(expression).unwrap(); + assert_eq!(target.id, id); + assert_eq!(text(source, target.span), source); + let AssignmentTarget::Sequence(items) = target.target else { + unreachable!() + }; + assert_eq!(items[0].id, a_id); + assert_eq!(text(source, items[0].span), "(a)"); + let AssignmentTarget::Identifier { span, .. } = items[0].target else { + unreachable!() + }; + assert_eq!(text(source, span), "a"); + assert_eq!(items[1].id, index_id); + assert_eq!(text(source, items[1].span), "values[next()]"); + let AssignmentTarget::Index { value, index, .. } = &items[1].target else { + unreachable!() + }; + assert_eq!(value.id, receiver_id); + assert_eq!(index.id, subscript_id); + assert_eq!(text(source, value.span), "values"); + assert_eq!(text(source, index.span), "next()"); + assert_eq!( + HashSet::from([id, a_id, index_id, receiver_id, subscript_id]).len(), + 5 + ); +} + +#[test] +fn grouped_member_target_keeps_outer_identity_and_precise_member_span() { + let source = "((make().field))"; + let expression = parse_one(source); + let id = expression.id; + let target = AssignmentTargetLocation::try_from(expression).unwrap(); + assert_eq!(target.id, id); + assert_eq!(text(source, target.span), source); + let AssignmentTarget::Member { + receiver, + member_span, + .. + } = target.target + else { + panic!("expected member target") + }; + assert_eq!(text(source, member_span), "field"); + assert_eq!(text(source, receiver.span), "make()"); + assert_ne!(receiver.id, id); +} + +#[test] +fn declaration_conversion_retains_each_pattern_identity() { + let expression = parse_one("[x, [y]]"); + let target = AssignmentTargetLocation::try_from(expression).unwrap(); + let root_id = target.id; + let AssignmentTarget::Sequence(items) = &target.target else { + unreachable!() + }; + let x_id = items[0].id; + let nested_id = items[1].id; + let AssignmentTarget::Sequence(nested) = &items[1].target else { + unreachable!() + }; + let y_id = nested[0].id; + let pattern = BindingPatternLocation::try_from(target).unwrap(); + let mut nodes = vec![]; + pattern_nodes(&pattern, &mut nodes); + assert_eq!( + nodes.iter().map(|(id, _)| *id).collect::>(), + [root_id, x_id, nested_id, y_id] + ); +} + +#[test] +fn function_parameters_and_loop_binders_carry_pattern_metadata() { + let source = "fn identity((x): Int) => x"; + let expression = parse_one(source); + let Expression::FunctionDeclaration { parameters, .. } = expression.expression else { + panic!("expected function") + }; + let parameter = ¶meters[0]; + assert_eq!(text(source, parameter.span), "(x): Int"); + assert_eq!(text(source, parameter.lvalue.span), "(x)"); + let BindingPattern::Identifier { span, .. } = parameter.lvalue.pattern else { + unreachable!() + }; + assert_eq!(text(source, span), "x"); + + let source = "for [(x), y] in input { x }"; + let expression = parse_one(source); + let Expression::For { iterations, .. } = expression.expression else { + panic!("expected loop") + }; + let ForIteration::Iteration { l_value, sequence } = &iterations[0] else { + unreachable!() + }; + let mut nodes = vec![]; + pattern_nodes(l_value, &mut nodes); + assert_eq!( + nodes + .iter() + .map(|(_, span)| text(source, *span)) + .collect::>(), + ["[(x), y]", "(x)", "y"] + ); + assert!(nodes.iter().all(|(id, _)| *id != sequence.id)); +} + +#[test] +fn split_targets_preserve_rejected_syntax() { + for source in [ + "let [xs[0]] = input", + "let [object.field] = input", + "for [xs[0]] in input { 0 }", + "for [object.field] in input { 0 }", + "fn destructure((x, y)) => x", + "[(object.field)] = input", + ] { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .unwrap(); + assert!( + Parser::from_tokens(tokens).parse().is_err(), + "must reject {source}" + ); + } +} diff --git a/ndc_vm/src/compiler.rs b/ndc_vm/src/compiler.rs index b31d7a1f..dc1c2120 100644 --- a/ndc_vm/src/compiler.rs +++ b/ndc_vm/src/compiler.rs @@ -5,8 +5,9 @@ use ndc_core::r#struct::StructRegistry; use ndc_core::{StaticType, TypeSignature}; use ndc_lexer::{NumericLiteral, Span}; use ndc_parser::{ - AugmentedAssignmentPlan, Binding, Candidate, CaptureSource, Expression, ExpressionLocation, - ForBody, ForIteration, FunctionParameter, LogicalOperator, Lvalue, ResolvedVar, + AssignmentTarget, AugmentedAssignmentPlan, Binding, BindingPattern, BindingPatternLocation, + Candidate, CaptureSource, Expression, ExpressionLocation, ForBody, ForIteration, + FunctionParameter, LogicalOperator, ResolvedVar, }; use std::cell::RefCell; use std::rc::Rc; @@ -227,7 +228,7 @@ impl Compiler { let idx = self.ir.add_constant(value); self.ir.write(OpCode::Constant(idx), span); } - Expression::Identifier { name, resolved } => { + Expression::Identifier { name, resolved, .. } => { if name == "None" { let idx = self.ir.add_constant(Value::None); self.ir.write(OpCode::Constant(idx), span); @@ -281,14 +282,14 @@ impl Compiler { } Expression::VariableDeclaration { value, l_value, .. } => { self.compile_expr(*value)?; - self.compile_declare_lvalue(l_value, span)?; + self.compile_binding_pattern(l_value, span)?; self.emit_unit(); } Expression::Assignment { l_value, r_value: value, - } => match l_value { - Lvalue::Index { + } => match l_value.target { + AssignmentTarget::Index { value: container, index, resolved_set, @@ -301,20 +302,20 @@ impl Compiler { self.compile_expr(*value)?; self.ir.write(OpCode::Call(3), span); } - l_value @ Lvalue::Identifier { .. } => { + l_value @ AssignmentTarget::Identifier { .. } => { self.compile_expr(*value)?; - self.compile_lvalue(l_value, span)?; + self.compile_assignment_target(l_value, span)?; self.emit_unit(); } - Lvalue::Sequence(seq) => { + AssignmentTarget::Sequence(seq) => { self.compile_expr(*value)?; self.ir.write(OpCode::Unpack(seq.len()), span); for l_value in seq { - self.compile_lvalue(l_value, span)?; + self.compile_assignment_target(l_value.target, span)?; } self.emit_unit(); } - Lvalue::Member { + AssignmentTarget::Member { receiver, member_span, resolved_setter, @@ -333,7 +334,7 @@ impl Compiler { plan, .. } => { - let target = PreparedAssignmentTarget::prepare(self, l_value, span)?; + let target = PreparedAssignmentTarget::prepare(self, l_value.target, span)?; let binding = match plan { AugmentedAssignmentPlan::Resolved(binding) => binding, AugmentedAssignmentPlan::Unresolved => { @@ -569,16 +570,20 @@ impl Compiler { Ok(()) } - fn compile_lvalue(&mut self, l_value: Lvalue, span: Span) -> Result<(), CompileError> { + fn compile_assignment_target( + &mut self, + l_value: AssignmentTarget, + span: Span, + ) -> Result<(), CompileError> { match l_value { - Lvalue::Identifier { + AssignmentTarget::Identifier { resolved, span: lv_span, .. } => { self.emit_set_var(resolved.expect("identifiers must be resolved"), lv_span); } - Lvalue::Index { + AssignmentTarget::Index { value, index, resolved_set, @@ -601,13 +606,13 @@ impl Compiler { self.ir.write(OpCode::Call(3), span); self.ir.write(OpCode::Pop, Span::synthetic()); } - Lvalue::Sequence(seq) => { + AssignmentTarget::Sequence(seq) => { self.ir.write(OpCode::Unpack(seq.len()), span); for lv in seq { - self.compile_lvalue(lv, span)?; + self.compile_assignment_target(lv.target, span)?; } } - Lvalue::Member { .. } => unreachable!( + AssignmentTarget::Member { .. } => unreachable!( "member assignment is lowered by Expression::Assignment; the parser rejects members inside destructuring patterns" ), } @@ -615,24 +620,26 @@ impl Compiler { Ok(()) } - fn compile_declare_lvalue(&mut self, l_value: Lvalue, span: Span) -> Result<(), CompileError> { - match l_value { - Lvalue::Identifier { resolved, .. } => { - let slot = match resolved.expect("declaration lvalue must be resolved") { + fn compile_binding_pattern( + &mut self, + pattern: BindingPatternLocation, + span: Span, + ) -> Result<(), CompileError> { + match pattern.pattern { + BindingPattern::Identifier { resolved, .. } => { + let slot = match resolved.expect("binding pattern must be resolved") { ResolvedVar::Local { slot } => slot, - _ => unreachable!("declaration lvalue must be a local"), + _ => unreachable!("binding pattern must be a local"), }; self.ir.write(OpCode::SetLocal(slot), span); self.source_locals = self.source_locals.max(slot + 1); } - Lvalue::Index { .. } => unreachable!("cannot declare into index"), - Lvalue::Sequence(seq) => { + BindingPattern::Sequence(seq) => { self.ir.write(OpCode::Unpack(seq.len()), span); for lv in seq { - self.compile_declare_lvalue(lv, span)?; + self.compile_binding_pattern(lv, span)?; } } - Lvalue::Member { .. } => unreachable!("cannot declare into a field"), } Ok(()) } @@ -982,13 +989,13 @@ impl Compiler { let iter_next = self .ir .write(OpCode::IterNext(JumpTarget::PLACEHOLDER), span); - self.compile_declare_lvalue(l_value.clone(), span)?; + self.compile_binding_pattern(l_value.clone(), span)?; self.compile_for_iterations(rest, span, compile_leaf)?; // Close upvalues for the loop variable so each iteration's closures // get their own frozen copy rather than sharing a mutable slot. - if let Some(slot) = min_lvalue_slot(l_value) { + if let Some(slot) = min_binding_slot(l_value) { self.ir.write(OpCode::CloseUpvalue(slot), span); } @@ -1094,13 +1101,17 @@ enum PreparedAssignmentTarget { } impl PreparedAssignmentTarget { - fn prepare(compiler: &mut Compiler, l_value: Lvalue, span: Span) -> Result { + fn prepare( + compiler: &mut Compiler, + l_value: AssignmentTarget, + span: Span, + ) -> Result { match l_value { - Lvalue::Identifier { resolved, span, .. } => Ok(Self::Variable { + AssignmentTarget::Identifier { resolved, span, .. } => Ok(Self::Variable { variable: resolved.expect("lvalue must be resolved"), span, }), - Lvalue::Index { + AssignmentTarget::Index { value, index, resolved_get, @@ -1125,7 +1136,7 @@ impl PreparedAssignmentTarget { setter: resolved_set.expect("[]= must be resolved"), }) } - Lvalue::Member { + AssignmentTarget::Member { receiver, member_span, resolved_getter, @@ -1146,7 +1157,9 @@ impl PreparedAssignmentTarget { setter: resolved_setter.expect("member setter must be resolved"), }) } - Lvalue::Sequence(_) => Err(CompileError::lvalue_required_to_be_single_identifier(span)), + AssignmentTarget::Sequence(_) => { + Err(CompileError::lvalue_required_to_be_single_identifier(span)) + } } } @@ -1226,15 +1239,15 @@ impl PreparedAssignmentTarget { } } -/// Returns the minimum local slot referenced by an lvalue, used to determine +/// Returns the minimum local slot referenced by a binding pattern, used to determine /// which upvalues to close at the end of a loop iteration. -fn min_lvalue_slot(lv: &Lvalue) -> Option { - match lv { - Lvalue::Identifier { +fn min_binding_slot(pattern: &BindingPatternLocation) -> Option { + match &pattern.pattern { + BindingPattern::Identifier { resolved: Some(ResolvedVar::Local { slot }), .. } => Some(*slot), - Lvalue::Sequence(seq) => seq.iter().filter_map(min_lvalue_slot).min(), + BindingPattern::Sequence(seq) => seq.iter().filter_map(min_binding_slot).min(), _ => None, } }