Skip to content

Commit 93b515e

Browse files
timfennisclaude
andcommitted
♻️ Adopt hybrid architecture: NodeId + side table for expression types
Replace `inferred_type: Option<StaticType>` on `ExpressionLocation` with a `NodeId` identity field and an `AnalysisResult` side table in the analyser. This follows the hybrid pattern: compilation-critical data (bindings, captures) stays on the AST; tooling-specific data (expression types) lives in a side table keyed by NodeId. - Add `NodeId` with atomic counter to ndc_parser - Add `AnalysisResult { expr_types: HashMap<NodeId, StaticType> }` to ndc_analyser, populated during analysis via `take_result()` - LSP reads expression types from the side table via NodeId lookup Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 parent 2b53bf4 commit 93b515e

8 files changed

Lines changed: 67 additions & 24 deletions

File tree

ndc_analyser/src/analyser.rs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
1+
use std::collections::HashMap;
2+
use std::fmt::Debug;
3+
14
use crate::scope::ScopeTree;
25
use itertools::Itertools;
36
use ndc_core::{StaticType, TypeSignature};
47
use ndc_lexer::Span;
5-
use ndc_parser::{Binding, Expression, ExpressionLocation, ForBody, ForIteration, Lvalue};
6-
use std::fmt::Debug;
8+
use ndc_parser::{Binding, Expression, ExpressionLocation, ForBody, ForIteration, Lvalue, NodeId};
9+
10+
/// Side table holding semantic information keyed by AST node identity.
11+
/// Keeps tooling-specific data (like per-expression types) out of the AST.
12+
#[derive(Debug, Default)]
13+
pub struct AnalysisResult {
14+
/// Maps each expression node to its inferred result type.
15+
pub expr_types: HashMap<NodeId, StaticType>,
16+
}
717

818
#[derive(Debug)]
919
pub struct Analyser {
@@ -12,13 +22,16 @@ pub struct Analyser {
1222
/// Pushed on function entry, popped on exit. The value accumulates the
1323
/// lub of all `return <expr>` types seen so far.
1424
return_type_stack: Vec<Option<StaticType>>,
25+
/// Side table populated during analysis.
26+
result: AnalysisResult,
1527
}
1628

1729
impl Analyser {
1830
pub fn from_scope_tree(scope_tree: ScopeTree) -> Self {
1931
Self {
2032
scope_tree,
2133
return_type_stack: Vec::new(),
34+
result: AnalysisResult::default(),
2235
}
2336
}
2437

@@ -30,13 +43,18 @@ impl Analyser {
3043
self.scope_tree = checkpoint;
3144
}
3245

46+
/// Take the accumulated analysis result, resetting it for the next analysis.
47+
pub fn take_result(&mut self) -> AnalysisResult {
48+
std::mem::take(&mut self.result)
49+
}
50+
3351
pub fn analyse(
3452
&mut self,
3553
expr_loc: &mut ExpressionLocation,
3654
) -> Result<StaticType, AnalysisError> {
37-
let result = self.analyse_inner(expr_loc)?;
38-
expr_loc.inferred_type = Some(result.clone());
39-
Ok(result)
55+
let typ = self.analyse_inner(expr_loc)?;
56+
self.result.expr_types.insert(expr_loc.id, typ.clone());
57+
Ok(typ)
4058
}
4159

4260
fn analyse_inner(

ndc_analyser/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
mod analyser;
22
mod scope;
33

4-
pub use analyser::{Analyser, AnalysisError};
4+
pub use analyser::{Analyser, AnalysisError, AnalysisResult};
55
pub use scope::ScopeTree;

ndc_interpreter/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use ndc_vm::value::CompiledFunction;
77
use ndc_vm::{OutputSink, Vm};
88
use std::rc::Rc;
99

10+
pub use ndc_analyser::AnalysisResult;
1011
#[cfg(feature = "trace")]
1112
pub use ndc_vm::tracer;
1213
pub use ndc_vm::{NativeFunction, Value};
@@ -92,9 +93,11 @@ impl Interpreter {
9293
pub fn analyse_str(
9394
&mut self,
9495
input: &str,
95-
) -> Result<Vec<ExpressionLocation>, InterpreterError> {
96+
) -> Result<(Vec<ExpressionLocation>, AnalysisResult), InterpreterError> {
9697
let source_id = self.source_db.add("<input>", input);
97-
self.parse_and_analyse(input, source_id)
98+
let expressions = self.parse_and_analyse(input, source_id)?;
99+
let result = self.analyser.take_result();
100+
Ok((expressions, result))
98101
}
99102

100103
pub fn compile_str(&mut self, input: &str) -> Result<CompiledFunction, InterpreterError> {

ndc_lsp/src/backend.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ impl Backend {
7575
interpreter
7676
.analyse_str(text)
7777
.ok()
78-
.map(|expressions| inlay_hints::collect(&expressions, text))
78+
.map(|(expressions, analysis_result)| {
79+
inlay_hints::collect(&expressions, &analysis_result, text)
80+
})
7981
};
8082

8183
// Only update document state when analysis succeeds. On failure (e.g.

ndc_lsp/src/features/inlay_hints.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::collections::HashMap;
22

33
use ndc_core::StaticType;
4+
use ndc_interpreter::AnalysisResult;
45
use ndc_lexer::Span;
56
use ndc_parser::ExpressionLocation;
67
use tower_lsp::lsp_types::{InlayHint, InlayHintKind, InlayHintLabel};
@@ -12,15 +13,20 @@ use crate::visitor::{AstVisitor, walk_ast};
1213
pub struct AnalysisInfo {
1314
pub hints: Vec<InlayHint>,
1415
pub variable_types: HashMap<String, StaticType>,
15-
/// Maps expression end offset inferred type for dot-completion on
16+
/// Maps expression end offset -> inferred type for dot-completion on
1617
/// arbitrary expressions (e.g. `read_file("foo").`).
1718
pub expression_types: HashMap<usize, StaticType>,
1819
}
1920

2021
/// Collect inlay hints, variable types, and expression types from an analysed AST.
21-
pub fn collect(expressions: &[ExpressionLocation], text: &str) -> AnalysisInfo {
22+
pub fn collect(
23+
expressions: &[ExpressionLocation],
24+
analysis_result: &AnalysisResult,
25+
text: &str,
26+
) -> AnalysisInfo {
2227
let mut collector = HintCollector {
2328
text,
29+
analysis_result,
2430
hints: Vec::new(),
2531
variable_types: HashMap::new(),
2632
expression_types: HashMap::new(),
@@ -35,14 +41,15 @@ pub fn collect(expressions: &[ExpressionLocation], text: &str) -> AnalysisInfo {
3541

3642
struct HintCollector<'a> {
3743
text: &'a str,
44+
analysis_result: &'a AnalysisResult,
3845
hints: Vec<InlayHint>,
3946
variable_types: HashMap<String, StaticType>,
4047
expression_types: HashMap<usize, StaticType>,
4148
}
4249

4350
impl AstVisitor for HintCollector<'_> {
4451
fn on_expression(&mut self, expr: &ExpressionLocation) {
45-
if let Some(typ) = &expr.inferred_type {
52+
if let Some(typ) = self.analysis_result.expr_types.get(&expr.id) {
4653
self.expression_types.insert(expr.span.end(), typ.clone());
4754
}
4855
}

ndc_parser/src/expression.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ use ndc_core::{StaticType, TypeSignature};
44
use ndc_lexer::Span;
55
use num::BigInt;
66
use num::complex::Complex64;
7+
use std::sync::atomic::{AtomicU32, Ordering};
8+
9+
/// Unique identity for an AST node. Used as a key in side tables (e.g. the
10+
/// analyser's expression type map) so that tooling data doesn't bloat the AST.
11+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
12+
pub struct NodeId(pub u32);
13+
14+
static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0);
15+
16+
impl NodeId {
17+
pub fn next() -> Self {
18+
Self(NEXT_NODE_ID.fetch_add(1, Ordering::Relaxed))
19+
}
20+
}
721

822
#[derive(Debug, Eq, PartialEq, Clone)]
923
pub enum Binding {
@@ -35,10 +49,9 @@ pub enum CaptureSource {
3549

3650
#[derive(Eq, PartialEq, Clone, Debug)]
3751
pub struct ExpressionLocation {
52+
pub id: NodeId,
3853
pub expression: Expression,
3954
pub span: Span,
40-
/// Filled by the semantic analyser with the inferred result type of this expression.
41-
pub inferred_type: Option<StaticType>,
4255
}
4356

4457
#[derive(Debug, PartialEq, Clone)]
@@ -183,9 +196,9 @@ impl Expression {
183196
#[must_use]
184197
pub fn to_location(self, span: Span) -> ExpressionLocation {
185198
ExpressionLocation {
199+
id: NodeId::next(),
186200
expression: self,
187201
span,
188-
inferred_type: None,
189202
}
190203
}
191204
}
@@ -194,9 +207,9 @@ impl ExpressionLocation {
194207
#[must_use]
195208
pub fn to_statement(self) -> Self {
196209
Self {
210+
id: NodeId::next(),
197211
span: self.span,
198212
expression: Expression::Statement(Box::new(self)),
199-
inferred_type: None,
200213
}
201214
}
202215

ndc_parser/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ mod operator;
33
mod parser;
44

55
pub use expression::{
6-
Binding, CaptureSource, Expression, ExpressionLocation, ForBody, ForIteration, Lvalue,
6+
Binding, CaptureSource, Expression, ExpressionLocation, ForBody, ForIteration, Lvalue, NodeId,
77
ResolvedVar,
88
};
99
pub use operator::{BinaryOperator, LogicalOperator, UnaryOperator};

ndc_parser/src/parser.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::fmt::Write;
22

33
use crate::expression::Expression;
4-
use crate::expression::{Binding, ExpressionLocation, ForBody, ForIteration, Lvalue};
4+
use crate::expression::{Binding, ExpressionLocation, ForBody, ForIteration, Lvalue, NodeId};
55
use crate::operator::{BinaryOperator, LogicalOperator, UnaryOperator};
66
use ndc_core::{Parameter, StaticType, TypeSignature};
77
use ndc_lexer::{Span, Token, TokenLocation};
@@ -421,7 +421,7 @@ impl Parser {
421421
values: expressions,
422422
},
423423
span: new_span,
424-
inferred_type: None,
424+
id: NodeId::next(),
425425
};
426426

427427
if must_be_tuple {
@@ -519,7 +519,7 @@ impl Parser {
519519
Ok(ExpressionLocation {
520520
expression,
521521
span,
522-
inferred_type: None,
522+
id: NodeId::next(),
523523
})
524524
} else {
525525
Ok(left)
@@ -653,7 +653,7 @@ impl Parser {
653653
arguments,
654654
},
655655
span: span.merge(arguments_span),
656-
inferred_type: None,
656+
id: NodeId::next(),
657657
};
658658
}
659659
Token::Dot => {
@@ -698,7 +698,7 @@ impl Parser {
698698
span: tuple_span
699699
.unwrap_or(identifier_span)
700700
.merge(first_argument_span),
701-
inferred_type: None,
701+
id: NodeId::next(),
702702
}
703703

704704
// for now, we require parentheses
@@ -763,7 +763,7 @@ impl Parser {
763763
arguments: vec![expr, index_expression],
764764
},
765765
span,
766-
inferred_type: None,
766+
id: NodeId::next(),
767767
};
768768
}
769769
_ => unreachable!("guaranteed to match"),
@@ -1194,7 +1194,7 @@ impl Parser {
11941194
pure: is_pure,
11951195
},
11961196
span,
1197-
inferred_type: None,
1197+
id: NodeId::next(),
11981198
})
11991199
}
12001200

0 commit comments

Comments
 (0)