Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

100 changes: 98 additions & 2 deletions crates/spar-hir-def/src/item_tree/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1986,8 +1986,20 @@ fn lower_range_from_tokens(node: &SyntaxNode) -> Option<PropertyExpr> {
};
PropertyExpr::Real(display, min_unit)
} else {
let val = parse_aadl_integer(&num_text).unwrap_or(0) * min_sign;
PropertyExpr::Integer(val, min_unit)
// Prior code used `unwrap_or(0)` here, silently substituting
// `0` for any integer literal that didn't fit in i64 — a range
// like `-9999999999999999999 .. 10` would lower to a range
// starting at 0 with zero diagnostic. Preserve the original
// literal as Opaque so analyses can detect the overflow
// instead of consuming a forged bound.
match parse_aadl_integer(&num_text).and_then(|v| v.checked_mul(min_sign)) {
Some(val) => PropertyExpr::Integer(val, min_unit),
None => PropertyExpr::Opaque(format!(
"{}{}",
if min_sign < 0 { "-" } else { "" },
num_text
)),
}
}
};

Expand Down Expand Up @@ -2977,6 +2989,90 @@ end Pkg;
}
}

#[cfg(test)]
mod range_lowering_tests {
use super::*;
use spar_syntax::{SyntaxKind, parse};

/// Regression for the `unwrap_or(0)` silent-substitution bug in
/// `lower_range_from_tokens`. An AADL range whose min literal
/// exceeds i64 must NOT lower to `Integer(0, _)` — a forged bound
/// that downstream analyses would consume as legitimate.
#[test]
fn overflowing_range_min_does_not_silently_become_zero() {
// -9999999999999999999 is magnitude ~1e19, well above i64::MAX.
let src = r#"
package P
public
processor Proc
properties
Compute_Execution_Time => -9999999999999999999 ms .. 10 ms;
end Proc;
end P;
"#;
let parsed = parse(src);
// Walk to the RANGE_VALUE node and lower it directly.
let range = parsed
.syntax_node()
.descendants()
.find(|n| n.kind() == SyntaxKind::RANGE_VALUE)
.expect("range value node");
let lowered = lower_range_from_tokens(&range).expect("range lowers");
match lowered {
PropertyExpr::Range { min, .. } => match *min {
PropertyExpr::Integer(0, _) => {
panic!(
"range min silently became 0 on i64 overflow; \
author wrote -9999999999999999999"
);
}
PropertyExpr::Integer(_, _) => {
// Unexpected: should not fit in i64. A non-zero i64
// would mean saturation or wrap — also wrong, but
// not the regression we're guarding against here.
panic!("range min fit in i64 unexpectedly");
}
PropertyExpr::Opaque(text) => {
assert!(
text.contains("9999999999999999999"),
"Opaque min must preserve the original literal; got {text:?}"
);
}
other => panic!("unexpected min shape: {other:?}"),
},
other => panic!("expected Range, got {other:?}"),
}
}

/// In-range positive integer range-min still lowers correctly.
#[test]
fn in_range_positive_min_lowers_correctly() {
let src = r#"
package P
public
processor Proc
properties
Compute_Execution_Time => 5 ms .. 10 ms;
end Proc;
end P;
"#;
let parsed = parse(src);
let range = parsed
.syntax_node()
.descendants()
.find(|n| n.kind() == SyntaxKind::RANGE_VALUE)
.expect("range value node");
let lowered = lower_range_from_tokens(&range).expect("range lowers");
match lowered {
PropertyExpr::Range { min, .. } => match *min {
PropertyExpr::Integer(5, _) => {}
other => panic!("expected Integer(5, _), got {other:?}"),
},
other => panic!("expected Range, got {other:?}"),
}
}
}

#[cfg(test)]
mod text_fallback_tests {
use super::*;
Expand Down
102 changes: 89 additions & 13 deletions crates/spar-parser/src/grammar/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,40 +158,116 @@ fn impl_extension(p: &mut Parser) {
m.complete(p, SyntaxKind::IMPL_EXTENSION);
}

/// Track which distinct sections have been seen so far inside a single
/// component type or implementation body.
///
/// AS-5506B §4.5 (component_type) and §5.1 (component_implementation)
/// grammar uses `?` (zero-or-one) on each section — not `*`. A component
/// declaration with two `features` clauses (or two `modes`, two
/// `properties`, …) is therefore a syntax error. Without this guard the
/// parser silently accepted duplicates, and lowering's `.find(|c| c.kind()
/// == FEATURE_SECTION)` call at item_tree/lower.rs silently dropped the
/// second section, so a reviewer who saw one interface in source ended up
/// with a different interface in the instance model.
#[derive(Default)]
struct SeenSections {
seen: Vec<(SyntaxKind, &'static str)>,
}

impl SeenSections {
/// Returns true on the first hit, false (and emits an error) on a repeat.
fn check(&mut self, p: &mut Parser, kind: SyntaxKind, label: &'static str) -> bool {
if self.seen.iter().any(|(k, _)| *k == kind) {
p.error(format!("duplicate `{label}` section"));
false
} else {
self.seen.push((kind, label));
true
}
}
}

fn component_type_sections(p: &mut Parser) {
let mut seen = SeenSections::default();
loop {
match p.current() {
SyntaxKind::PROTOTYPES_KW => prototype_section(p),
SyntaxKind::FEATURES_KW => super::features::feature_section(p),
SyntaxKind::FLOWS_KW => super::flows::flow_spec_section(p),
SyntaxKind::MODES_KW => super::modes::mode_section(p),
SyntaxKind::PROTOTYPES_KW => {
seen.check(p, SyntaxKind::PROTOTYPES_KW, "prototypes");
prototype_section(p);
}
SyntaxKind::FEATURES_KW => {
seen.check(p, SyntaxKind::FEATURES_KW, "features");
super::features::feature_section(p);
}
SyntaxKind::FLOWS_KW => {
seen.check(p, SyntaxKind::FLOWS_KW, "flows");
super::flows::flow_spec_section(p);
}
SyntaxKind::MODES_KW => {
seen.check(p, SyntaxKind::MODES_KW, "modes");
super::modes::mode_section(p);
}
SyntaxKind::REQUIRES_KW if p.nth(1) == SyntaxKind::MODES_KW => {
seen.check(p, SyntaxKind::MODES_KW, "modes");
super::modes::mode_section(p);
}
SyntaxKind::PROPERTIES_KW => super::properties::property_section(p),
SyntaxKind::PROPERTIES_KW => {
seen.check(p, SyntaxKind::PROPERTIES_KW, "properties");
super::properties::property_section(p);
}
// AS-5506B §4.5: an annex subclause may appear multiple times
// (one per annex language), so do not dedup on ANNEX_KW here.
SyntaxKind::ANNEX_KW => super::annexes::annex_subclause(p),
_ => break,
}
}
}

fn component_impl_sections(p: &mut Parser) {
let mut seen = SeenSections::default();
loop {
match p.current() {
SyntaxKind::PROTOTYPES_KW => prototype_section(p),
SyntaxKind::SUBCOMPONENTS_KW => subcomponent_section(p),
SyntaxKind::INTERNAL_KW => internal_features_section(p),
SyntaxKind::PROTOTYPES_KW => {
seen.check(p, SyntaxKind::PROTOTYPES_KW, "prototypes");
prototype_section(p);
}
SyntaxKind::SUBCOMPONENTS_KW => {
seen.check(p, SyntaxKind::SUBCOMPONENTS_KW, "subcomponents");
subcomponent_section(p);
}
SyntaxKind::INTERNAL_KW => {
seen.check(p, SyntaxKind::INTERNAL_KW, "internal features");
internal_features_section(p);
}
SyntaxKind::PROCESSOR_KW if p.nth(1) == SyntaxKind::FEATURES_KW => {
seen.check(p, SyntaxKind::PROCESSOR_KW, "processor features");
processor_features_section(p);
}
SyntaxKind::CONNECTIONS_KW => super::connections::connection_section(p),
SyntaxKind::CALLS_KW => call_section(p),
SyntaxKind::FLOWS_KW => super::flows::flow_impl_section(p),
SyntaxKind::MODES_KW => super::modes::mode_section(p),
SyntaxKind::CONNECTIONS_KW => {
seen.check(p, SyntaxKind::CONNECTIONS_KW, "connections");
super::connections::connection_section(p);
}
SyntaxKind::CALLS_KW => {
seen.check(p, SyntaxKind::CALLS_KW, "calls");
call_section(p);
}
SyntaxKind::FLOWS_KW => {
seen.check(p, SyntaxKind::FLOWS_KW, "flows");
super::flows::flow_impl_section(p);
}
SyntaxKind::MODES_KW => {
seen.check(p, SyntaxKind::MODES_KW, "modes");
super::modes::mode_section(p);
}
SyntaxKind::REQUIRES_KW if p.nth(1) == SyntaxKind::MODES_KW => {
seen.check(p, SyntaxKind::MODES_KW, "modes");
super::modes::mode_section(p);
}
SyntaxKind::PROPERTIES_KW => super::properties::property_section(p),
SyntaxKind::PROPERTIES_KW => {
seen.check(p, SyntaxKind::PROPERTIES_KW, "properties");
super::properties::property_section(p);
}
// AS-5506B §5.1: annex subclauses may repeat.
SyntaxKind::ANNEX_KW => super::annexes::annex_subclause(p),
_ => break,
}
Expand Down
10 changes: 8 additions & 2 deletions crates/spar-parser/src/grammar/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
_ => {
Expand Down
Loading
Loading