Skip to content

Commit 00a8b54

Browse files
timfennisclaude
timfennis
and
claude
committed
🐛 Fix REPL error attribution by tracking source per span (#29)
Add SourceId to Span and a SourceDb registry so each span knows which source string it belongs to. Diagnostics now use codespan-reporting's multi-file support to render errors against the correct source, even when a function defined on a previous REPL line triggers a runtime error. Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 parent 4cdadc7 commit 00a8b54

13 files changed

Lines changed: 239 additions & 84 deletions

File tree

‎compiler_tests/tests/compiler.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
use ndc_lexer::Lexer;
1+
use ndc_lexer::{Lexer, SourceId};
22
use ndc_parser::Parser;
33
use ndc_vm::chunk::OpCode;
44
use ndc_vm::chunk::OpCode::*;
55
use ndc_vm::compiler::Compiler;
66

77
fn compile(input: &str) -> Vec<OpCode> {
8-
let tokens = Lexer::new(input)
8+
let tokens = Lexer::new(input, SourceId::SYNTHETIC)
99
.collect::<Result<Vec<_>, _>>()
1010
.expect("lex failed");
1111
let expressions = Parser::from_tokens(tokens).parse().expect("parse failed");

‎ndc_bin/src/diagnostic.rs‎

Lines changed: 80 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,128 @@
11
use codespan_reporting::diagnostic::{Diagnostic, Label};
2-
use codespan_reporting::files::SimpleFile;
2+
use codespan_reporting::files;
33
use codespan_reporting::term;
44
use codespan_reporting::term::termcolor::{ColorChoice, StandardStream};
55
use ndc_interpreter::InterpreterError;
6-
use ndc_lexer::Span;
6+
use ndc_lexer::{SourceDb, SourceId, Span};
7+
use std::ops::Range;
78

8-
fn span_to_range(span: Span) -> std::ops::Range<usize> {
9+
fn span_to_range(span: Span) -> Range<usize> {
910
span.offset()..span.end()
1011
}
1112

12-
fn into_diagnostic(err: InterpreterError) -> Diagnostic<()> {
13+
struct DiagnosticFiles<'a>(&'a SourceDb);
14+
15+
impl<'a> files::Files<'a> for DiagnosticFiles<'a> {
16+
type FileId = SourceId;
17+
type Name = &'a str;
18+
type Source = &'a str;
19+
20+
fn name(&'a self, id: SourceId) -> Result<&'a str, files::Error> {
21+
if id == SourceId::SYNTHETIC {
22+
return Ok("<synthetic>");
23+
}
24+
Ok(self.0.name(id))
25+
}
26+
27+
fn source(&'a self, id: SourceId) -> Result<&'a str, files::Error> {
28+
if id == SourceId::SYNTHETIC {
29+
return Ok("");
30+
}
31+
Ok(self.0.source(id))
32+
}
33+
34+
fn line_index(&'a self, id: SourceId, byte_index: usize) -> Result<usize, files::Error> {
35+
let source = self.source(id)?;
36+
Ok(files::line_starts(source)
37+
.take_while(|&start| start <= byte_index)
38+
.count()
39+
.saturating_sub(1))
40+
}
41+
42+
fn line_range(&'a self, id: SourceId, line_index: usize) -> Result<Range<usize>, files::Error> {
43+
let source = self.source(id)?;
44+
let line_starts: Vec<usize> = files::line_starts(source).collect();
45+
let start = *line_starts
46+
.get(line_index)
47+
.ok_or(files::Error::LineTooLarge {
48+
given: line_index,
49+
max: line_starts.len().saturating_sub(1),
50+
})?;
51+
let end = line_starts
52+
.get(line_index + 1)
53+
.copied()
54+
.unwrap_or(source.len());
55+
Ok(start..end)
56+
}
57+
}
58+
59+
fn into_diagnostic(err: InterpreterError) -> Diagnostic<SourceId> {
1360
match err {
1461
InterpreterError::Lexer { cause } => {
62+
let span = cause.span();
1563
let mut d = Diagnostic::error()
1664
.with_code("lexer")
1765
.with_message(cause.to_string())
1866
.with_labels(vec![
19-
Label::primary((), span_to_range(cause.span())).with_message("here"),
67+
Label::primary(span.source_id(), span_to_range(span)).with_message("here"),
2068
]);
2169
if let Some(help) = cause.help_text() {
2270
d = d.with_notes(vec![help.to_owned()]);
2371
}
2472
d
2573
}
2674
InterpreterError::Parser { cause } => {
75+
let span = cause.span();
2776
let mut d = Diagnostic::error()
2877
.with_code("parser")
2978
.with_message(cause.to_string())
3079
.with_labels(vec![
31-
Label::primary((), span_to_range(cause.span())).with_message("here"),
80+
Label::primary(span.source_id(), span_to_range(span)).with_message("here"),
3281
]);
3382
if let Some(help) = cause.help_text() {
3483
d = d.with_notes(vec![help.to_owned()]);
3584
}
3685
d
3786
}
38-
InterpreterError::Resolver { cause } => Diagnostic::error()
39-
.with_code("resolver")
40-
.with_message(cause.to_string())
41-
.with_labels(vec![
42-
Label::primary((), span_to_range(cause.span())).with_message("related to this"),
43-
]),
44-
InterpreterError::Compiler { cause } => Diagnostic::error()
45-
.with_code("compiler")
46-
.with_message(cause.to_string())
47-
.with_labels(vec![
48-
Label::primary((), span_to_range(cause.span())).with_message("related to this"),
49-
]),
87+
InterpreterError::Resolver { cause } => {
88+
let span = cause.span();
89+
Diagnostic::error()
90+
.with_code("resolver")
91+
.with_message(cause.to_string())
92+
.with_labels(vec![
93+
Label::primary(span.source_id(), span_to_range(span))
94+
.with_message("related to this"),
95+
])
96+
}
97+
InterpreterError::Compiler { cause } => {
98+
let span = cause.span();
99+
Diagnostic::error()
100+
.with_code("compiler")
101+
.with_message(cause.to_string())
102+
.with_labels(vec![
103+
Label::primary(span.source_id(), span_to_range(span))
104+
.with_message("related to this"),
105+
])
106+
}
50107
InterpreterError::Vm(err) => {
51108
let mut d = Diagnostic::error()
52109
.with_code("vm")
53110
.with_message(&err.message);
54111
if let Some(span) = err.span {
55112
d = d.with_labels(vec![
56-
Label::primary((), span_to_range(span)).with_message("related to this"),
113+
Label::primary(span.source_id(), span_to_range(span))
114+
.with_message("related to this"),
57115
]);
58116
}
59117
d
60118
}
61119
}
62120
}
63121

64-
pub fn emit_error(filename: &str, source: &str, err: InterpreterError) {
122+
pub fn emit_error(source_db: &SourceDb, err: InterpreterError) {
65123
let diagnostic = into_diagnostic(err);
66-
let file = SimpleFile::new(filename, source);
124+
let files = DiagnosticFiles(source_db);
67125
let writer = StandardStream::stderr(ColorChoice::Auto);
68126
let config = term::Config::default();
69-
let _ = term::emit(&mut writer.lock(), &config, &file, &diagnostic);
127+
let _ = term::emit(&mut writer.lock(), &config, &files, &diagnostic);
70128
}

‎ndc_bin/src/highlighter.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use ahash::AHashSet;
22
use itertools::Itertools;
3-
use ndc_lexer::{Lexer, Token, TokenLocation};
3+
use ndc_lexer::{Lexer, SourceId, Token, TokenLocation};
44
use ndc_parser::{Expression, ExpressionLocation, ForBody, ForIteration};
55
use yansi::{Paint, Painted};
66

@@ -12,7 +12,7 @@ impl AndycppHighlighter {
1212
pub fn highlight_parsed(line: &str) -> Vec<Painted<&str>> {
1313
let mut function_spans = AHashSet::new();
1414

15-
let expressions = Lexer::new(line)
15+
let expressions = Lexer::new(line, SourceId::SYNTHETIC)
1616
.collect::<Result<Vec<TokenLocation>, _>>()
1717
.ok()
1818
.and_then(|tokens| ndc_parser::Parser::from_tokens(tokens).parse().ok());
@@ -30,7 +30,8 @@ impl AndycppHighlighter {
3030
line: &'a str,
3131
function_spans: &AHashSet<usize>,
3232
) -> Vec<Painted<&'a str>> {
33-
let Ok(tokens) = Lexer::new(line).collect::<Result<Vec<_>, _>>() else {
33+
let Ok(tokens) = Lexer::new(line, SourceId::SYNTHETIC).collect::<Result<Vec<_>, _>>()
34+
else {
3435
return vec![line.red()];
3536
};
3637

‎ndc_bin/src/main.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,24 +118,20 @@ fn main() -> anyhow::Result<()> {
118118

119119
let mut interpreter = Interpreter::new();
120120
interpreter.configure(ndc_stdlib::register);
121-
if let Err(err) = interpreter.eval(&string) {
122-
diagnostic::emit_error(&filename.expect("filename must exist"), &string, err);
121+
let name = filename.as_deref().unwrap_or("<input>");
122+
if let Err(err) = interpreter.eval_named(name, &string) {
123+
diagnostic::emit_error(interpreter.source_db(), err);
123124
process::exit(1);
124125
}
125126
}
126127
Action::DisassembleFile(path) => {
127-
let filename = path
128-
.file_name()
129-
.and_then(|name| name.to_str())
130-
.unwrap_or("<input>")
131-
.to_string();
132128
let string = fs::read_to_string(path)?;
133129
let mut interpreter = Interpreter::new();
134130
interpreter.configure(ndc_stdlib::register);
135131
match interpreter.disassemble_str(&string) {
136132
Ok(output) => print!("{output}"),
137133
Err(e) => {
138-
diagnostic::emit_error(&filename, &string, e);
134+
diagnostic::emit_error(interpreter.source_db(), e);
139135
process::exit(1);
140136
}
141137
}

‎ndc_bin/src/repl.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ pub fn run() -> anyhow::Result<()> {
4141
let _ = rl.add_history_entry(line.as_str());
4242

4343
// Run the line we just read through the interpreter
44-
match interpreter.eval(line.as_str()) {
44+
match interpreter.eval_named("<repl>", line.as_str()) {
4545
Ok(value) => {
4646
let output = value.to_string();
4747
if !output.is_empty() {
4848
println!("{output}")
4949
}
5050
}
5151
Err(err) => {
52-
crate::diagnostic::emit_error("<repl>", &line, err);
52+
crate::diagnostic::emit_error(interpreter.source_db(), err);
5353
}
5454
}
5555
}

‎ndc_interpreter/src/lib.rs‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use ndc_analyser::{Analyser, ScopeTree};
22
use ndc_core::FunctionRegistry;
3-
use ndc_lexer::{Lexer, TokenLocation};
3+
use ndc_lexer::{Lexer, SourceDb, SourceId, TokenLocation};
44
use ndc_parser::ExpressionLocation;
55
use ndc_vm::compiler::Compiler;
66
use ndc_vm::value::CompiledFunction;
@@ -13,6 +13,7 @@ pub struct Interpreter {
1313
registry: FunctionRegistry<Rc<NativeFunction>>,
1414
capturing: bool,
1515
analyser: Analyser,
16+
source_db: SourceDb,
1617
/// Persistent REPL VM and the compiler checkpoint from the last run.
1718
/// `None` until the first `eval` call; kept alive afterwards so that
1819
/// variables declared on one line are visible on subsequent lines.
@@ -38,6 +39,7 @@ impl Interpreter {
3839
registry: FunctionRegistry::default(),
3940
capturing,
4041
analyser: Analyser::from_scope_tree(ScopeTree::from_global_scope(vec![])),
42+
source_db: SourceDb::new(),
4143
repl_state: None,
4244
}
4345
}
@@ -72,15 +74,21 @@ impl Interpreter {
7274
}
7375
}
7476

77+
pub fn source_db(&self) -> &SourceDb {
78+
&self.source_db
79+
}
80+
7581
pub fn analyse_str(
7682
&mut self,
7783
input: &str,
7884
) -> Result<Vec<ExpressionLocation>, InterpreterError> {
79-
self.parse_and_analyse(input)
85+
let source_id = self.source_db.add("<input>", input);
86+
self.parse_and_analyse(input, source_id)
8087
}
8188

8289
pub fn compile_str(&mut self, input: &str) -> Result<CompiledFunction, InterpreterError> {
83-
let expressions = self.parse_and_analyse(input)?;
90+
let source_id = self.source_db.add("<input>", input);
91+
let expressions = self.parse_and_analyse(input, source_id)?;
8492
Ok(Compiler::compile(expressions.into_iter())?)
8593
}
8694

@@ -95,15 +103,22 @@ impl Interpreter {
95103
///
96104
/// Statements (semicolon-terminated) produce [`Value::unit()`].
97105
pub fn eval(&mut self, input: &str) -> Result<Value, InterpreterError> {
98-
let expressions = self.parse_and_analyse(input)?;
106+
self.eval_named("<input>", input)
107+
}
108+
109+
/// Execute source code with a custom source name for diagnostics.
110+
pub fn eval_named(&mut self, name: &str, input: &str) -> Result<Value, InterpreterError> {
111+
let source_id = self.source_db.add(name, input);
112+
let expressions = self.parse_and_analyse(input, source_id)?;
99113
self.interpret_vm(input, expressions.into_iter())
100114
}
101115

102116
fn parse_and_analyse(
103117
&mut self,
104118
input: &str,
119+
source_id: SourceId,
105120
) -> Result<Vec<ExpressionLocation>, InterpreterError> {
106-
let tokens = Lexer::new(input).collect::<Result<Vec<TokenLocation>, _>>()?;
121+
let tokens = Lexer::new(input, source_id).collect::<Result<Vec<TokenLocation>, _>>()?;
107122
let mut expressions = ndc_parser::Parser::from_tokens(tokens).parse()?;
108123

109124
let checkpoint = self.analyser.checkpoint();

‎ndc_lexer/src/lib.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
mod number;
2+
mod source_db;
23
mod span;
34
mod string;
45
mod token;
@@ -8,7 +9,8 @@ use std::collections::VecDeque;
89
use std::str::Chars;
910
use string::StringLexer;
1011

11-
pub use span::Span;
12+
pub use source_db::SourceDb;
13+
pub use span::{SourceId, Span};
1214
pub use token::{Token, TokenLocation};
1315

1416
pub struct Lexer<'a> {
@@ -41,12 +43,13 @@ impl<'a> Lexer<'a> {
4143
}
4244

4345
#[must_use]
44-
pub fn new(source: &'a str) -> Self {
46+
pub fn new(source: &'a str, source_id: SourceId) -> Self {
4547
Self {
4648
source: SourceIterator {
4749
inner: source.chars(),
4850
buffer: VecDeque::default(),
4951
offset: 0,
52+
source_id,
5053
},
5154
}
5255
}
@@ -182,6 +185,7 @@ struct SourceIterator<'a> {
182185
inner: Chars<'a>,
183186
buffer: VecDeque<char>,
184187
offset: usize,
188+
source_id: SourceId,
185189
}
186190

187191
impl SourceIterator<'_> {
@@ -190,11 +194,11 @@ impl SourceIterator<'_> {
190194
}
191195

192196
pub fn create_span(&self, start: usize) -> Span {
193-
Span::new(start, (self.current_offset()) - start)
197+
Span::new(self.source_id, start, self.current_offset() - start)
194198
}
195199

196200
pub fn span(&self) -> Span {
197-
Span::new(self.current_offset(), 1)
201+
Span::new(self.source_id, self.current_offset(), 1)
198202
}
199203

200204
pub fn consume(&mut self, count: usize) {

0 commit comments

Comments
 (0)