Skip to content

Commit 42812ff

Browse files
committed
Track data sequences as expressions, not values
This helps decouple the AST from Value types and, because Exprs carry a position that needs to be checked for in tests, highlights that there was a subtle bug in position reporting for negative data numeric values.
1 parent 7910171 commit 42812ff

3 files changed

Lines changed: 62 additions & 26 deletions

File tree

core/src/ast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ pub struct CallableSpan {
496496
#[derive(Debug, PartialEq)]
497497
pub struct DataSpan {
498498
/// Collection of optional literal values.
499-
pub values: Vec<Option<Value>>,
499+
pub values: Vec<Option<Expr>>,
500500
}
501501

502502
/// Components of a variable definition.

core/src/compiler/mod.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ struct Compiler {
354354
instrs: Vec<Instruction>,
355355

356356
/// Data discovered so far.
357-
data: Vec<Option<Value>>,
357+
data: Vec<Option<Expr>>,
358358

359359
/// Symbols table.
360360
symtable: SymbolsTable,
@@ -1239,12 +1239,27 @@ impl Compiler {
12391239
self.instrs[pc] = new_instr;
12401240
}
12411241

1242-
let image =
1243-
Image { upcalls: self.symtable.upcalls(), instrs: self.instrs, data: self.data };
1242+
let data = compile_data(self.data);
1243+
1244+
let image = Image { upcalls: self.symtable.upcalls(), instrs: self.instrs, data };
12441245
Ok((image, self.symtable))
12451246
}
12461247
}
12471248

1249+
/// Translates the reduced set of expressions that represent data into values.
1250+
fn compile_data(data: Vec<Option<Expr>>) -> Vec<Option<Value>> {
1251+
data.into_iter()
1252+
.map(|expr| match expr {
1253+
None => None,
1254+
Some(Expr::Boolean(span)) => Some(Value::Boolean(span.value)),
1255+
Some(Expr::Double(span)) => Some(Value::Double(span.value)),
1256+
Some(Expr::Integer(span)) => Some(Value::Integer(span.value)),
1257+
Some(Expr::Text(span)) => Some(Value::Text(span.value)),
1258+
_ => unreachable!("Valid data types enforced at parse time"),
1259+
})
1260+
.collect()
1261+
}
1262+
12481263
/// Compiles a collection of statements into an image ready for execution.
12491264
///
12501265
/// `symtable` is the symbols table as used by the compiler and should be prepopulated with any

core/src/parser.rs

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -436,16 +436,30 @@ impl<'a> Parser<'a> {
436436

437437
let token_span = self.lexer.read()?;
438438
match token_span.token {
439-
Token::Boolean(b) => values.push(Some(Value::Boolean(b))),
440-
Token::Double(d) => values.push(Some(Value::Double(d))),
441-
Token::Integer(i) => values.push(Some(Value::Integer(i))),
442-
Token::Text(t) => values.push(Some(Value::Text(t))),
439+
Token::Boolean(b) => {
440+
values.push(Some(Expr::Boolean(BooleanSpan { value: b, pos: token_span.pos })))
441+
}
442+
Token::Double(d) => {
443+
values.push(Some(Expr::Double(DoubleSpan { value: d, pos: token_span.pos })))
444+
}
445+
Token::Integer(i) => {
446+
values.push(Some(Expr::Integer(IntegerSpan { value: i, pos: token_span.pos })))
447+
}
448+
Token::Text(t) => {
449+
values.push(Some(Expr::Text(TextSpan { value: t, pos: token_span.pos })))
450+
}
443451

444452
Token::Minus => {
445-
let token_span = self.lexer.read()?;
446-
match token_span.token {
447-
Token::Double(d) => values.push(Some(Value::Double(-d))),
448-
Token::Integer(i) => values.push(Some(Value::Integer(-i))),
453+
let token_span2 = self.lexer.read()?;
454+
match token_span2.token {
455+
Token::Double(d) => values.push(Some(Expr::Double(DoubleSpan {
456+
value: -d,
457+
pos: token_span.pos,
458+
}))),
459+
Token::Integer(i) => values.push(Some(Expr::Integer(IntegerSpan {
460+
value: -i,
461+
pos: token_span.pos,
462+
}))),
449463
_ => {
450464
return Err(Error::Bad(
451465
token_span.pos,
@@ -2373,19 +2387,23 @@ mod tests {
23732387
do_ok_test(
23742388
"DATA 1: DATA 2",
23752389
&[
2376-
Statement::Data(DataSpan { values: vec![Some(Value::Integer(1))] }),
2377-
Statement::Data(DataSpan { values: vec![Some(Value::Integer(2))] }),
2390+
Statement::Data(DataSpan {
2391+
values: vec![Some(Expr::Integer(IntegerSpan { value: 1, pos: lc(1, 6) }))],
2392+
}),
2393+
Statement::Data(DataSpan {
2394+
values: vec![Some(Expr::Integer(IntegerSpan { value: 2, pos: lc(1, 14) }))],
2395+
}),
23782396
],
23792397
);
23802398

23812399
do_ok_test(
23822400
"DATA TRUE, -3, 5.1, \"foo\"",
23832401
&[Statement::Data(DataSpan {
23842402
values: vec![
2385-
Some(Value::Boolean(true)),
2386-
Some(Value::Integer(-3)),
2387-
Some(Value::Double(5.1)),
2388-
Some(Value::Text("foo".to_owned())),
2403+
Some(Expr::Boolean(BooleanSpan { value: true, pos: lc(1, 6) })),
2404+
Some(Expr::Integer(IntegerSpan { value: -3, pos: lc(1, 12) })),
2405+
Some(Expr::Double(DoubleSpan { value: 5.1, pos: lc(1, 16) })),
2406+
Some(Expr::Text(TextSpan { value: "foo".to_owned(), pos: lc(1, 21) })),
23892407
],
23902408
})],
23912409
);
@@ -2395,13 +2413,13 @@ mod tests {
23952413
&[Statement::Data(DataSpan {
23962414
values: vec![
23972415
None,
2398-
Some(Value::Boolean(true)),
2416+
Some(Expr::Boolean(BooleanSpan { value: true, pos: lc(1, 8) })),
23992417
None,
2400-
Some(Value::Integer(3)),
2418+
Some(Expr::Integer(IntegerSpan { value: 3, pos: lc(1, 16) })),
24012419
None,
2402-
Some(Value::Double(5.1)),
2420+
Some(Expr::Double(DoubleSpan { value: 5.1, pos: lc(1, 21) })),
24032421
None,
2404-
Some(Value::Text("foo".to_owned())),
2422+
Some(Expr::Text(TextSpan { value: "foo".to_owned(), pos: lc(1, 28) })),
24052423
None,
24062424
],
24072425
})],
@@ -2410,7 +2428,10 @@ mod tests {
24102428
do_ok_test(
24112429
"DATA -3, -5.1",
24122430
&[Statement::Data(DataSpan {
2413-
values: vec![Some(Value::Integer(-3)), Some(Value::Double(-5.1))],
2431+
values: vec![
2432+
Some(Expr::Integer(IntegerSpan { value: -3, pos: lc(1, 6) })),
2433+
Some(Expr::Double(DoubleSpan { value: -5.1, pos: lc(1, 10) })),
2434+
],
24142435
})],
24152436
);
24162437
}
@@ -2421,9 +2442,9 @@ mod tests {
24212442
do_error_test("DATA ;", "1:6: Unexpected ; in DATA statement");
24222443
do_error_test("DATA 5 + 1", "1:8: Expected comma after datum but found +");
24232444
do_error_test("DATA 5 ; 1", "1:8: Expected comma after datum but found ;");
2424-
do_error_test("DATA -FALSE", "1:7: Expected number after -");
2425-
do_error_test("DATA -\"abc\"", "1:7: Expected number after -");
2426-
do_error_test("DATA -foo", "1:7: Expected number after -");
2445+
do_error_test("DATA -FALSE", "1:6: Expected number after -");
2446+
do_error_test("DATA -\"abc\"", "1:6: Expected number after -");
2447+
do_error_test("DATA -foo", "1:6: Expected number after -");
24272448
}
24282449

24292450
#[test]

0 commit comments

Comments
 (0)