From 0364c84022297c5f7f8ce208848fa63ed03253f9 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Tue, 21 Apr 2026 19:58:49 +0200 Subject: [PATCH 1/5] fix(parser): unary sign must bind only to adjacent literal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from PR #130. When property_expression was split into an outer binary-op wrapper + an inner _primary for #127, the PLUS/MINUS arm in the primary kept recursing into the outer wrapper. The outer's binary-op loop then greedily consumed the following `+`/`-`/`*` operators into the signed operand — so `-1 + 2` parsed as `-(1+2)` instead of `(-1)+2`, silently inverting the sign of every additive tail downstream. AS-5506B §11.2.5 defines `numeric_term ::= [sign] numeric_literal` — the sign is atomic with the literal, not a prefix over the additive expression. Fix: one-line change at properties.rs:251, recurse into `property_expression_primary` in the signed-numeric arm. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/spar-parser/src/grammar/properties.rs | 10 ++++- crates/spar-syntax/tests/parser_tests.rs | 43 +++++++++++++++++++ .../property_value_signed_arithmetic.aadl | 23 ++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 test-data/parser/property_value_signed_arithmetic.aadl diff --git a/crates/spar-parser/src/grammar/properties.rs b/crates/spar-parser/src/grammar/properties.rs index 8537dac..3532fa2 100644 --- a/crates/spar-parser/src/grammar/properties.rs +++ b/crates/spar-parser/src/grammar/properties.rs @@ -245,10 +245,16 @@ fn property_expression_primary(p: &mut Parser) { } } SyntaxKind::PLUS | SyntaxKind::MINUS => { - // Signed numeric value + // Signed numeric value. Recurse into *primary* (not the outer + // wrapper), otherwise the binary-op loop would greedily consume + // the following `+`/`-`/`*` operators into the signed operand — + // causing `-1 + 2` to parse as `-(1+2)` instead of `(-1)+2`. + // AS-5506B §11.2.5: `numeric_term ::= [sign] numeric_literal` + // — the sign is part of the signed literal, not a prefix over + // the additive expression. let m = p.start(); p.bump_any(); - property_expression(p); + property_expression_primary(p); m.complete(p, SyntaxKind::PROPERTY_EXPRESSION); } _ => { diff --git a/crates/spar-syntax/tests/parser_tests.rs b/crates/spar-syntax/tests/parser_tests.rs index 1d1b1ed..f38a6a4 100644 --- a/crates/spar-syntax/tests/parser_tests.rs +++ b/crates/spar-syntax/tests/parser_tests.rs @@ -882,6 +882,49 @@ fn test_data_property_value_arithmetic() { check_file_no_errors("../../test-data/parser/property_value_arithmetic.aadl"); } +// Regression for the unary-sign operator-precedence bug introduced when +// property_expression was split into outer-binary-loop + _primary: the +// PLUS/MINUS arm was still recursing into the outer function, so the sign +// greedily consumed the following additive tail. +#[test] +fn test_data_property_value_signed_arithmetic() { + check_file_no_errors("../../test-data/parser/property_value_signed_arithmetic.aadl"); +} + +// Shape-level assertion: in `-1 + 2` the signed PROPERTY_EXPRESSION node +// must cover only `-1`, not the entire tail. +#[test] +fn unary_sign_binds_to_literal_only_not_to_additive_tail() { + let src = "\ +package P +public + processor PP + properties + Foo::Bar => -1 + 2; + end PP; +end P; +"; + let result = parse(src); + assert!( + result.errors().is_empty(), + "expected clean parse, got: {:?}", + result.errors() + ); + let root = result.syntax_node(); + let signed = root + .descendants() + .find(|n| { + n.kind() == SyntaxKind::PROPERTY_EXPRESSION + && n.text().to_string().trim_start().starts_with('-') + }) + .expect("signed-literal PROPERTY_EXPRESSION node"); + let text = signed.text().to_string(); + assert!( + !text.contains('+'), + "unary `-` greedy-ate the additive tail: signed node text = {text:?}" + ); +} + // ==================================================================== // OSATE2 test files // ==================================================================== diff --git a/test-data/parser/property_value_signed_arithmetic.aadl b/test-data/parser/property_value_signed_arithmetic.aadl new file mode 100644 index 0000000..054656e --- /dev/null +++ b/test-data/parser/property_value_signed_arithmetic.aadl @@ -0,0 +1,23 @@ +-- Regression test: unary sign must bind only to the adjacent numeric +-- literal, not to the whole following additive expression. Prior to the +-- fix, the PLUS/MINUS primary arm recursed into the outer +-- property_expression (which runs a left-associative binary-op loop), +-- so `-1 + 2` parsed as `-(1+2)` instead of `(-1)+2` — silently +-- inverting the sign of every downstream arithmetic property. +-- AS-5506B §11.2.5: `numeric_term ::= [sign] numeric_literal`. +package Test_Signed_Arithmetic +public + + processor P + properties + -- leading negative + additive tail + Slack => -1 + 2; + -- leading positive + additive tail + Bias => +3 + 4; + -- subtractive tail after leading negative + Adjust => -10 - 3; + -- mixed with multiplication + Scale => -2 * 5 + 1; + end P; + +end Test_Signed_Arithmetic; From 81ccc7fb66cbf7b5afdb99639cc5cc8144c6cec8 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Tue, 21 Apr 2026 20:00:42 +0200 Subject: [PATCH 2/5] fix(parser): reject duplicate component sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AS-5506B §4.5 (component_type) and §5.1 (component_implementation) grammar uses `?` on each section clause — `features`, `modes`, `properties`, `subcomponents`, `connections`, `prototypes`, `calls`, `flows`, `internal features`, `processor features`. Before this fix the `loop { match }` section dispatcher had no seen-tracking, so a component with two `features` blocks parsed cleanly. Lowering then silently dropped the duplicate because `item_tree/lower.rs` walks the CST with `.find(|c| c.kind() == FEATURE_SECTION)` — first match wins — so a reviewer who saw ports in both sections ended up with ONLY the first set in the instance model. Adds a `SeenSections` guard consulted before each arm. On a repeat we emit `p.error("duplicate `