Skip to content

Commit e8861d4

Browse files
ghaithclaude
ghaith
and
claude
authored
fix(parser): recover from unexpected tokens in expression position (#1709)
## Summary `parse_atomic_leaf_expression`'s fallback emitted an `E007 unexpected_token_found` diagnostic but **did not advance the lexer**, leaving the bad token in the stream for binary-op parsers higher in the cascade to re-consume. For `prog(p := &g)` from #1306 this meant the AST became `BinaryExpression { And, EmptyStatement, g }`; the empty LHS reached codegen and produced a confusing `no type hint available for EmptyStatement` while compilation reported success. The same shape bit any other binary-only operator misused as a unary prefix (`MOD x`, `OR x`, …). Recovery now: - emits a clearer `"expected expression"` diagnostic (was `"Literal"`); - when the bad token is itself an operator, advances past it and retries the leaf so the operand the user wrote is captured (`&g` → `g`); - otherwise leaves the token in place so outer parsers can use it for synchronization (e.g. `END_CASE`, `END_VAR`). The missing-parameter-assignment carve-out (`foo(p := )`) is narrowed to fire only when the next token actually closes the parameter list (`)` or `,`); previously any unrecognised token after `:=` was silently treated as an empty parameter, masking real errors like the one in #1306. Resolver tests that exercised pointer arithmetic via the dead-code `&x` path are migrated to `REF(x)`. Fixes #1306 ## Behaviour change for users Before: ``` $ plc bug.st --check $ echo $? 0 ``` After: ``` $ plc bug.st --check error[E007]: Unexpected token: expected expression but found & ┌─ bug.st:13:14 │ 13 │ foo(p := &g); │ ^ Unexpected token: expected expression but found & Compilation aborted due to critical parse errors. $ echo $? 1 ``` ## Test plan - [x] New inline-snapshot tests in `expressions_parser_tests.rs` covering `prog(p := &g)` (the #1306 regression), bare `&y`, and `MOD y` (generality witness). - [x] Defensive `amp_as_and_test` (binary `b & c`) stays green. - [x] 16 existing parser-error snapshots updated to reflect the `"Literal"` → `"expression"` wording. Locations and column markers are unchanged across all of them. - [x] `qualified_reference_location_test` no longer covers `&aaa.bbb.ccc` since that case was anchoring the buggy `EmptyStatement AND expr` shape. - [x] Two resolver tests migrated from `&x` to `REF(x)` (the `&` path was dead code that fell through the buggy parser recovery). - [x] `cargo test --workspace` clean (2466/2466). - [x] `cargo xtask lit` clean (347/347). - [x] `cargo fmt --all` + `cargo clippy --workspace -- -Dwarnings` clean. - [x] Manual repro: `plc <file> --check` on the issue's input now exits 1 with E007; full compile aborts before codegen with the same diagnostic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d57b735 commit e8861d4

19 files changed

Lines changed: 299 additions & 50 deletions

File tree

‎src/parser/expressions_parser.rs‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,17 +327,41 @@ fn parse_atomic_leaf_expression(lexer: &mut ParseSession<'_>) -> Option<AstNode>
327327
_ => {
328328
if lexer.closing_keywords.contains(&vec![KeywordParensClose])
329329
&& matches!(lexer.last_token, KeywordOutputAssignment | KeywordAssignment)
330+
&& matches!(lexer.token, KeywordParensClose | KeywordComma)
330331
{
331332
// due to closing keyword ')' and last_token '=>' / ':='
332333
// we are probably in a call statement missing a parameter assignment 'foo(param := );
333334
// optional parameter assignments are allowed, validation should handle any unwanted cases
334335
Some(AstFactory::create_empty_statement(lexer.location(), lexer.next_id()))
335336
} else {
336337
lexer.accept_diagnostic(Diagnostic::unexpected_token_found(
337-
"Literal",
338+
"expression",
338339
lexer.slice(),
339340
lexer.location(),
340341
));
342+
// If the bad token has any operator interpretation, drain
343+
// any consecutive operator tokens and retry the leaf so the
344+
// recovered AST captures the operand the user wrote. In
345+
// practice this fires for *binary-only* operators misused
346+
// as a prefix (e.g. `&y`, `MOD y`); unary-eligible
347+
// operators (`-`, `+`, `NOT`) also match `to_operator()`
348+
// but normally never reach this fallback because the
349+
// prefix-operator parser higher in the cascade handles
350+
// them first. Looping (rather than tail-recursing per
351+
// token) emits one diagnostic per run instead of N, and
352+
// keeps the recovery cost flat on adversarial input like
353+
// `& & & g`. For non-operator tokens (e.g. `END_CASE`,
354+
// `END_VAR`), leave the token in the stream so outer
355+
// parsers can synchronise on it — the `advanced` guard
356+
// prevents an infinite retry loop in that case.
357+
let mut advanced = false;
358+
while to_operator(&lexer.token).is_some() {
359+
lexer.advance();
360+
advanced = true;
361+
}
362+
if advanced {
363+
return parse_atomic_leaf_expression(lexer);
364+
}
341365
None
342366
}
343367
}

‎src/parser/tests/expressions_parser_tests.rs‎

Lines changed: 230 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright (c) 2020 Ghaith Hachem and Mathias Rieder
22
use crate::parser::tests::ref_to;
3-
use crate::test_utils::tests::parse;
3+
use crate::test_utils::tests::{parse, parse_buffered};
44
use insta::{assert_debug_snapshot, assert_snapshot};
55
use plc_ast::ast::{Assignment, AstFactory, AstNode, AstStatement, Operator};
66
use plc_ast::literals::AstLiteral;
@@ -1014,6 +1014,234 @@ fn amp_as_and_test() {
10141014
assert_debug_snapshot!(statement);
10151015
}
10161016

1017+
// Recovery for unexpected tokens in expression position.
1018+
// `&` (and other binary-only operators) used as a prefix used to leave the
1019+
// bad token in the stream; binary-AND parsing then re-consumed it and built
1020+
// `EmptyStatement & rhs`, which crashed codegen with E071. The leaf-fallback
1021+
// now advances past the bad token after diagnosing it, so the cascade above
1022+
// cannot misinterpret it.
1023+
1024+
#[test]
1025+
fn amp_as_address_of_in_call_arg_recovers() {
1026+
let src = "
1027+
VAR_GLOBAL g : DINT; END_VAR
1028+
FUNCTION foo : DINT VAR_INPUT p : DINT; END_VAR foo := p; END_FUNCTION
1029+
PROGRAM main foo(p := &g); END_PROGRAM
1030+
";
1031+
let (unit, diagnostics) = parse_buffered(src);
1032+
assert_snapshot!(diagnostics, @r"
1033+
error[E007]: Unexpected token: expected expression but found &
1034+
┌─ <internal>:4:31
1035+
│
1036+
4 │ PROGRAM main foo(p := &g); END_PROGRAM
1037+
│ ^ Unexpected token: expected expression but found &
1038+
");
1039+
let main = unit.implementations.iter().find(|i| i.name == "main").unwrap();
1040+
assert_debug_snapshot!(main.statements, @r#"
1041+
[
1042+
CallStatement {
1043+
operator: ReferenceExpr {
1044+
kind: Member(
1045+
Identifier {
1046+
name: "foo",
1047+
},
1048+
),
1049+
base: None,
1050+
},
1051+
parameters: Some(
1052+
Assignment {
1053+
left: ReferenceExpr {
1054+
kind: Member(
1055+
Identifier {
1056+
name: "p",
1057+
},
1058+
),
1059+
base: None,
1060+
},
1061+
right: ReferenceExpr {
1062+
kind: Member(
1063+
Identifier {
1064+
name: "g",
1065+
},
1066+
),
1067+
base: None,
1068+
},
1069+
},
1070+
),
1071+
},
1072+
]
1073+
"#);
1074+
}
1075+
1076+
#[test]
1077+
fn amp_as_unary_prefix_in_assignment_recovers() {
1078+
let src = "
1079+
PROGRAM prg
1080+
VAR x : DINT; y : DINT; END_VAR
1081+
x := &y;
1082+
END_PROGRAM
1083+
";
1084+
let (unit, diagnostics) = parse_buffered(src);
1085+
assert_snapshot!(diagnostics, @r"
1086+
error[E007]: Unexpected token: expected expression but found &
1087+
┌─ <internal>:4:18
1088+
│
1089+
4 │ x := &y;
1090+
│ ^ Unexpected token: expected expression but found &
1091+
");
1092+
assert_debug_snapshot!(unit.implementations[0].statements, @r#"
1093+
[
1094+
Assignment {
1095+
left: ReferenceExpr {
1096+
kind: Member(
1097+
Identifier {
1098+
name: "x",
1099+
},
1100+
),
1101+
base: None,
1102+
},
1103+
right: ReferenceExpr {
1104+
kind: Member(
1105+
Identifier {
1106+
name: "y",
1107+
},
1108+
),
1109+
base: None,
1110+
},
1111+
},
1112+
]
1113+
"#);
1114+
}
1115+
1116+
#[test]
1117+
fn consecutive_bad_operators_emit_a_single_diagnostic() {
1118+
// `& & & g` previously emitted one diagnostic per `&` and recursed N
1119+
// times. The loop drains the run and retries once, so adversarial input
1120+
// produces exactly one diagnostic and one recovered identifier.
1121+
let src = "
1122+
PROGRAM prg
1123+
VAR x : DINT; g : DINT; END_VAR
1124+
x := & & & g;
1125+
END_PROGRAM
1126+
";
1127+
let (unit, diagnostics) = parse_buffered(src);
1128+
assert_snapshot!(diagnostics, @r"
1129+
error[E007]: Unexpected token: expected expression but found &
1130+
┌─ <internal>:4:18
1131+
│
1132+
4 │ x := & & & g;
1133+
│ ^ Unexpected token: expected expression but found &
1134+
");
1135+
assert_debug_snapshot!(unit.implementations[0].statements, @r#"
1136+
[
1137+
Assignment {
1138+
left: ReferenceExpr {
1139+
kind: Member(
1140+
Identifier {
1141+
name: "x",
1142+
},
1143+
),
1144+
base: None,
1145+
},
1146+
right: ReferenceExpr {
1147+
kind: Member(
1148+
Identifier {
1149+
name: "g",
1150+
},
1151+
),
1152+
base: None,
1153+
},
1154+
},
1155+
]
1156+
"#);
1157+
}
1158+
1159+
#[test]
1160+
fn empty_parameter_assignment_carve_out_still_fires() {
1161+
// A named parameter with no RHS — `p := )` or `p := ,` — is explicitly
1162+
// allowed by the parser: it lands as `Assignment { right: EmptyStatement }`
1163+
// and emits no diagnostic. This test locks in that the new "advance past
1164+
// unexpected tokens" recovery in `parse_atomic_leaf_expression` did NOT
1165+
// regress that carve-out — i.e. it must not consume the trailing `)` or
1166+
// `,` while looking for an expression. Both call shapes therefore land
1167+
// cleanly with an empty RHS and no parse diagnostics.
1168+
let src = "
1169+
FUNCTION foo : DINT VAR_INPUT p : DINT; q : DINT; END_VAR foo := p; END_FUNCTION
1170+
PROGRAM main
1171+
foo(p := );
1172+
foo(p := , q := 1);
1173+
END_PROGRAM
1174+
";
1175+
let (unit, diagnostics) = parse_buffered(src);
1176+
assert!(diagnostics.is_empty(), "expected no parse diagnostics, got:\n{diagnostics}");
1177+
1178+
let main = unit.implementations.iter().find(|i| i.name == "main").unwrap();
1179+
// Both calls should land an Assignment with right == EmptyStatement.
1180+
for stmt in &main.statements {
1181+
let AstStatement::CallStatement(call) = &stmt.stmt else {
1182+
panic!("expected CallStatement, got {:?}", stmt);
1183+
};
1184+
let Some(params) = call.parameters.as_deref() else {
1185+
panic!("expected parameters on call, got {:?}", call);
1186+
};
1187+
// walk into the first Assignment we find and check its RHS
1188+
let first_assignment = match &params.stmt {
1189+
AstStatement::Assignment(a) => a,
1190+
AstStatement::ExpressionList(list) => match &list[0].stmt {
1191+
AstStatement::Assignment(a) => a,
1192+
other => panic!("expected Assignment in list head, got {:?}", other),
1193+
},
1194+
other => panic!("expected Assignment or ExpressionList, got {:?}", other),
1195+
};
1196+
assert!(
1197+
matches!(first_assignment.right.stmt, AstStatement::EmptyStatement(..)),
1198+
"expected RHS to be EmptyStatement, got {:?}",
1199+
first_assignment.right.stmt
1200+
);
1201+
}
1202+
}
1203+
1204+
#[test]
1205+
fn binary_only_operator_as_prefix_recovers() {
1206+
// Generality witness — recovery is not specific to `&`.
1207+
let src = "
1208+
PROGRAM prg
1209+
VAR x : DINT; y : DINT; END_VAR
1210+
x := MOD y;
1211+
END_PROGRAM
1212+
";
1213+
let (unit, diagnostics) = parse_buffered(src);
1214+
assert_snapshot!(diagnostics, @r"
1215+
error[E007]: Unexpected token: expected expression but found MOD
1216+
┌─ <internal>:4:18
1217+
│
1218+
4 │ x := MOD y;
1219+
│ ^^^ Unexpected token: expected expression but found MOD
1220+
");
1221+
assert_debug_snapshot!(unit.implementations[0].statements, @r#"
1222+
[
1223+
Assignment {
1224+
left: ReferenceExpr {
1225+
kind: Member(
1226+
Identifier {
1227+
name: "x",
1228+
},
1229+
),
1230+
base: None,
1231+
},
1232+
right: ReferenceExpr {
1233+
kind: Member(
1234+
Identifier {
1235+
name: "y",
1236+
},
1237+
),
1238+
base: None,
1239+
},
1240+
},
1241+
]
1242+
"#);
1243+
}
1244+
10171245
#[test]
10181246
fn and_then_test() {
10191247
let src = "
@@ -1635,7 +1863,7 @@ fn reference_location_test() {
16351863

16361864
#[test]
16371865
fn qualified_reference_location_test() {
1638-
let source = "PROGRAM prg a.b.c;aa.bb.cc[2];aaa.bbb.ccc^;&aaa.bbb.ccc; END_PROGRAM";
1866+
let source = "PROGRAM prg a.b.c;aa.bb.cc[2];aaa.bbb.ccc^; END_PROGRAM";
16391867
let parse_result = parse(source).0;
16401868

16411869
let unit = &parse_result.implementations[0];
@@ -1648,9 +1876,6 @@ fn qualified_reference_location_test() {
16481876

16491877
let location = &unit.statements[2].get_location();
16501878
assert_eq!(source[location.to_range().unwrap()].to_string(), "aaa.bbb.ccc^");
1651-
1652-
let location = &unit.statements[3].get_location();
1653-
assert_eq!(source[location.to_range().unwrap()].to_string(), "&aaa.bbb.ccc");
16541879
}
16551880

16561881
#[test]

‎src/parser/tests/parse_errors/parse_error_containers_tests.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,11 @@ fn super_is_a_reserved_keyword() {
229229
│ ╰─────────────────^ Unexpected token: expected KeywordSemicolon but found 'VAR
230230
super'
231231
232-
error[E007]: Unexpected token: expected Literal but found END_VAR
232+
error[E007]: Unexpected token: expected expression but found END_VAR
233233
┌─ <internal>:6:9
234234
│
235235
6 │ END_VAR
236-
│ ^^^^^^^ Unexpected token: expected Literal but found END_VAR
236+
│ ^^^^^^^ Unexpected token: expected expression but found END_VAR
237237
238238
error[E007]: Unexpected token: expected KeywordSemicolon but found 'END_VAR
239239
METHOD super END_METHOD'
@@ -313,11 +313,11 @@ fn this_is_a_reserved_keyword() {
313313
│ ╰────────────────^ Unexpected token: expected KeywordSemicolon but found 'VAR
314314
this'
315315
316-
error[E007]: Unexpected token: expected Literal but found END_VAR
316+
error[E007]: Unexpected token: expected expression but found END_VAR
317317
┌─ <internal>:6:9
318318
│
319319
6 │ END_VAR
320-
│ ^^^^^^^ Unexpected token: expected Literal but found END_VAR
320+
│ ^^^^^^^ Unexpected token: expected expression but found END_VAR
321321
322322
error[E007]: Unexpected token: expected KeywordSemicolon but found 'END_VAR
323323
METHOD this END_METHOD'

‎src/parser/tests/parse_errors/parse_error_reserved_keywords_tests.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,11 @@ fn retain_as_variable_reference_in_expression_is_unexpected_token() {
194194
";
195195
let diagnostics = parse_and_validate_buffered(source);
196196
assert_snapshot!(diagnostics, @"
197-
error[E007]: Unexpected token: expected Literal but found retain
197+
error[E007]: Unexpected token: expected expression but found retain
198198
┌─ <internal>:4:18
199199
│
200200
4 │ x := retain;
201-
│ ^^^^^^ Unexpected token: expected Literal but found retain
201+
│ ^^^^^^ Unexpected token: expected expression but found retain
202202
203203
error[E007]: Unexpected token: expected KeywordSemicolon but found 'retain'
204204
┌─ <internal>:4:18
@@ -217,11 +217,11 @@ fn retain_as_call_target_is_unexpected_token() {
217217
";
218218
let diagnostics = parse_and_validate_buffered(source);
219219
assert_snapshot!(diagnostics, @"
220-
error[E007]: Unexpected token: expected Literal but found retain
220+
error[E007]: Unexpected token: expected expression but found retain
221221
┌─ <internal>:3:13
222222
│
223223
3 │ retain();
224-
│ ^^^^^^ Unexpected token: expected Literal but found retain
224+
│ ^^^^^^ Unexpected token: expected expression but found retain
225225
226226
error[E007]: Unexpected token: expected KeywordSemicolon but found 'retain()'
227227
┌─ <internal>:3:13
@@ -242,11 +242,11 @@ fn retain_as_member_access_is_unexpected_token() {
242242
";
243243
let diagnostics = parse_and_validate_buffered(source);
244244
assert_snapshot!(diagnostics, @"
245-
error[E007]: Unexpected token: expected Literal but found retain
245+
error[E007]: Unexpected token: expected expression but found retain
246246
┌─ <internal>:5:15
247247
│
248248
5 │ f.retain;
249-
│ ^^^^^^ Unexpected token: expected Literal but found retain
249+
│ ^^^^^^ Unexpected token: expected expression but found retain
250250
251251
error[E007]: Unexpected token: expected KeywordSemicolon but found 'retain'
252252
┌─ <internal>:5:15

‎src/parser/tests/parse_errors/snapshots/rusty__parser__tests__parse_errors__parse_error_classes_tests__method_with_invalid_return_type.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ error[E007]: Unexpected token: expected Datatype but found ABSTRACT
1414
1 │ CLASS TestClass METHOD foo : ABSTRACT END_METHOD END_CLASS
1515
│ ^^^^^^^^ Unexpected token: expected Datatype but found ABSTRACT
1616

17-
error[E007]: Unexpected token: expected Literal but found ABSTRACT
17+
error[E007]: Unexpected token: expected expression but found ABSTRACT
1818
┌─ <internal>:1:30
1919
│
2020
1 │ CLASS TestClass METHOD foo : ABSTRACT END_METHOD END_CLASS
21-
│ ^^^^^^^^ Unexpected token: expected Literal but found ABSTRACT
21+
│ ^^^^^^^^ Unexpected token: expected expression but found ABSTRACT
2222

2323
error[E007]: Unexpected token: expected KeywordSemicolon but found 'ABSTRACT'
2424
┌─ <internal>:1:30

0 commit comments

Comments
 (0)