From aee97bbba193399965067e5a01062b8fb783cd1b Mon Sep 17 00:00:00 2001 From: Markus <66058642+mhovd@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:14:49 +0200 Subject: [PATCH] feat: Harden DSL --- Cargo.toml | 3 +- pharmsol-dsl/Cargo.toml | 1 + pharmsol-dsl/README.md | 57 ++- pharmsol-dsl/src/authoring.rs | 38 +- pharmsol-dsl/src/diagnostic.rs | 58 ++- pharmsol-dsl/src/execution.rs | 47 +- pharmsol-dsl/src/ir.rs | 55 +- pharmsol-dsl/src/lexer.rs | 7 + pharmsol-dsl/src/lib.rs | 43 +- pharmsol-dsl/src/parser.rs | 71 ++- pharmsol-dsl/src/pipeline.rs | 128 +++++ pharmsol-dsl/src/semantic.rs | 71 ++- .../tests/dsl_authoring_edge_cases.rs | 2 +- pharmsol-dsl/tests/frontend_hardening.rs | 469 ++++++++++++++++++ src/dsl/jit.rs | 4 +- src/dsl/native.rs | 72 +-- src/simulator/equation/metadata.rs | 118 ++++- src/simulator/equation/mod.rs | 32 +- 18 files changed, 1041 insertions(+), 235 deletions(-) create mode 100644 pharmsol-dsl/src/pipeline.rs create mode 100644 pharmsol-dsl/tests/frontend_hardening.rs diff --git a/Cargo.toml b/Cargo.toml index 8e90f78a..8f78b108 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ pharmsol-dsl = { path = "pharmsol-dsl", version = "0.28.2" } pharmsol-macros = { path = "pharmsol-macros", version = "0.28.2" } serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.149" +thiserror = "2.0.18" [package] name = "pharmsol" @@ -52,7 +53,7 @@ cranelift-module = { version = "0.131.0", optional = true } cranelift-native = { version = "0.131.0", optional = true } serde = { workspace = true } serde_json = { workspace = true } -thiserror = "2.0.18" +thiserror = { workspace = true } tracing = "0.1.44" wasm-encoder = { version = "0.247.0", optional = true } diff --git a/pharmsol-dsl/Cargo.toml b/pharmsol-dsl/Cargo.toml index 78fd3586..d1cc9d2b 100644 --- a/pharmsol-dsl/Cargo.toml +++ b/pharmsol-dsl/Cargo.toml @@ -15,3 +15,4 @@ bench = false [dependencies] serde = { workspace = true } serde_json = { workspace = true } +thiserror = { workspace = true } diff --git a/pharmsol-dsl/README.md b/pharmsol-dsl/README.md index 0280727b..e825339b 100644 --- a/pharmsol-dsl/README.md +++ b/pharmsol-dsl/README.md @@ -13,25 +13,11 @@ Do not use this crate for JIT compilation, native AoT export or load, WASM runti ## Main Pipeline -The public pipeline is: - -1. `parse_model` or `parse_module` -2. `analyze_model` or `analyze_module` -3. `lower_typed_model` or `lower_typed_module` - -The main public modules are: - -- `ast` for syntax-level nodes -- `diagnostic` for spans, codes, and rendered reports -- `ir` for the typed intermediate representation -- `execution` for the lowered execution model shared by JIT, AoT, and WASM backends - -The parser accepts both canonical `model { ... }` source and the authoring shorthand used by the `pharmsol` examples. - -## Small Example +The one-shot pipeline is `compile_model` or `compile_module`, which fails with +the unified `DslError`: ```rust -use pharmsol_dsl::{analyze_model, lower_typed_model, parse_model}; +use pharmsol_dsl::compile_model; let source = r#" name = bimodal_ke @@ -47,15 +33,46 @@ dx(central) = -ke * central out(cp) = central / v "#; -let syntax = parse_model(source).expect("model parses"); -let typed = analyze_model(&syntax).expect("model analyzes"); -let execution = lower_typed_model(&typed).expect("model lowers"); +let execution = compile_model(source).expect("model compiles"); assert_eq!(execution.name, "bimodal_ke"); assert_eq!(execution.metadata.routes.len(), 1); assert_eq!(execution.metadata.outputs.len(), 1); ``` +The staged pipeline is available when you need the intermediaterepresentations: + +1. `parse_model` or `parse_module` +2. `analyze_model` or `analyze_module` +3. `lower_typed_model` or `lower_typed_module` + +The main public modules are: + +- `ast` for syntax-level nodes +- `diagnostic` for spans, codes, and rendered reports +- `ir` for the typed intermediate representation +- `execution` for the lowered execution model shared by JIT, AoT, and WASM backends + +The parser accepts both canonical `model { ... }` source and the authoring +shorthand used by the `pharmsol` examples. + +## Errors + +Every stage reports errors with source spans and renders an annotated report +when printed: + +```text +error[DSL2000]: unknown identifier `missing_state` + --> line 3, column 11 + | +3 | out(cp) = missing_state + | ^^^^^^^^^^^^^ unknown identifier `missing_state` +``` + +`DslError::phase` identifies the failing stage, `DslError::diagnostics` +exposes the structured diagnostics, and `DslError::diagnostic_report` produces +a JSON-serializable report for editors and tooling. + ## Boundary With `pharmsol` `pharmsol-dsl` owns the frontend pipeline and its data structures. diff --git a/pharmsol-dsl/src/authoring.rs b/pharmsol-dsl/src/authoring.rs index 4233d7a5..d1143ead 100644 --- a/pharmsol-dsl/src/authoring.rs +++ b/pharmsol-dsl/src/authoring.rs @@ -310,8 +310,8 @@ impl<'a> AuthoringParser<'a> { let span = Span::new(line_offset + leading, line_offset + trailing); self.note_span(span); - if find_top_level_arrow(trimmed).is_some() { - return self.parse_route_line(trimmed, span.start, span); + if let Some(arrow) = find_top_level_arrow(trimmed) { + return self.parse_route_line(trimmed, arrow, span.start, span); } let eq_index = find_top_level_assignment(trimmed).ok_or_else(|| { @@ -435,7 +435,7 @@ impl<'a> AuthoringParser<'a> { } if let Some(name_segment) = lhs_trimmed.strip_prefix("const ") { - let name_abs = span.start + (lhs.find("const").unwrap() + "const ".len()); + let name_abs = span.start + (lhs.len() - lhs_trimmed.len()) + "const ".len(); let name = parse_ident_segment(name_segment, name_abs)?; let value = parse_expr_at(rhs, rhs_abs)?; self.constants.push(Binding { @@ -484,10 +484,10 @@ impl<'a> AuthoringParser<'a> { fn parse_route_line( &mut self, trimmed: &str, + arrow: usize, line_start: usize, span: Span, ) -> Result<(), ParseError> { - let arrow = find_top_level_arrow(trimmed).unwrap(); let lhs = &trimmed[..arrow]; let rhs = &trimmed[arrow + 2..]; let call = parse_call_head(lhs, line_start)? @@ -1168,15 +1168,32 @@ fn parse_expr_at(src: &str, abs_start: usize) -> Result { } fn parse_surface_rhs(src: &str, abs_start: usize) -> Result { + parse_surface_rhs_at(src, abs_start, 0) +} + +fn parse_surface_rhs_at( + src: &str, + abs_start: usize, + depth: usize, +) -> Result { let trimmed = src.trim_start(); let leading = src.len() - trimmed.len(); if starts_with_keyword(trimmed, "if") { - return parse_if_rhs(trimmed, abs_start + leading); + return parse_if_rhs(trimmed, abs_start + leading, depth); } Ok(SurfaceRhs::Expr(parse_expr_at(src, abs_start)?)) } -fn parse_if_rhs(src: &str, abs_start: usize) -> Result { +fn parse_if_rhs(src: &str, abs_start: usize, depth: usize) -> Result { + if depth >= crate::parser::MAX_NESTING_DEPTH { + return Err(ParseError::new( + format!( + "conditional expression is nested too deeply (maximum nesting depth is {})", + crate::parser::MAX_NESTING_DEPTH + ), + Span::new(abs_start, abs_start + src.len().min(2)), + )); + } let rest = &src[2..]; let rest_leading = rest.len() - rest.trim_start().len(); let rest = &rest[rest_leading..]; @@ -1206,9 +1223,12 @@ fn parse_if_rhs(src: &str, abs_start: usize) -> Result { })?; let condition = parse_expr_at(condition_src, rest_abs + 1)?; - let then_branch = parse_surface_rhs(&remaining[..else_index], remaining_abs)?; - let else_branch = - parse_surface_rhs(&remaining[else_index + 4..], remaining_abs + else_index + 4)?; + let then_branch = parse_surface_rhs_at(&remaining[..else_index], remaining_abs, depth + 1)?; + let else_branch = parse_surface_rhs_at( + &remaining[else_index + 4..], + remaining_abs + else_index + 4, + depth + 1, + )?; let span = Span::new(abs_start, remaining_abs + remaining.len()); Ok(SurfaceRhs::If { diff --git a/pharmsol-dsl/src/diagnostic.rs b/pharmsol-dsl/src/diagnostic.rs index f7a1adda..f59d3dcc 100644 --- a/pharmsol-dsl/src/diagnostic.rs +++ b/pharmsol-dsl/src/diagnostic.rs @@ -237,8 +237,19 @@ impl Diagnostic { .to_string() .len(); rendered.push_str(&format!("{:>width$} |\n", "", width = gutter)); + let mut last_line = None; for label in &self.labels { - rendered.push_str(&render_label(src, label, &self.message, gutter)); + let label_line = line_info(src, label.span.start.min(src.len())).0; + // Repeat the source text only when the label moves to a new line. + let show_source = last_line != Some(label_line); + rendered.push_str(&render_label( + src, + label, + &self.message, + gutter, + show_source, + )); + last_line = Some(label_line); } } for note in &self.notes { @@ -500,6 +511,15 @@ impl ParseError { } } + /// Marker propagated after a fatal parse error was recorded; carries no + /// diagnostics of its own and is always merged into a non-empty error. + pub(crate) fn aborted() -> Self { + Self { + diagnostics: Vec::new(), + source: None, + } + } + pub fn with_note(mut self, note: impl Into) -> Self { self.diagnostics[0].notes.push(note.into()); self @@ -580,14 +600,22 @@ impl fmt::Display for ParseError { if let Some(source) = self.source() { return f.write_str(&self.render(source)); } - let span = self.diagnostic().primary_span(); + let diagnostic = self.diagnostic(); + let span = diagnostic.primary_span(); write!( f, - "{} at bytes {}..{}", - self.diagnostic().message, - span.start, - span.end - ) + "error[{}]: {} (at bytes {}..{})", + diagnostic.code, diagnostic.message, span.start, span.end + )?; + let remaining = self.diagnostics.len() - 1; + if remaining > 0 { + write!( + f, + " (+{remaining} more error{})", + if remaining == 1 { "" } else { "s" } + )?; + } + Ok(()) } } @@ -606,10 +634,10 @@ fn render_label( label: &DiagnosticLabel, fallback_message: &str, gutter: usize, + show_source: bool, ) -> String { let offset = label.span.start.min(src.len()); let (line, _, line_start, line_end) = line_info(src, offset); - let line_text = &src[line_start..line_end]; let marker_start = src[line_start..offset].chars().count(); let highlight_end = label.span.end.min(line_end).max(offset); let marker_len = src[offset..highlight_end].chars().count().max(1); @@ -624,12 +652,14 @@ fn render_label( }); let mut rendered = String::new(); - rendered.push_str(&format!( - "{:>width$} | {}\n", - line, - line_text, - width = gutter - )); + if show_source { + rendered.push_str(&format!( + "{:>width$} | {}\n", + line, + &src[line_start..line_end], + width = gutter + )); + } rendered.push_str(&format!( "{:>width$} | {}{}", "", diff --git a/pharmsol-dsl/src/execution.rs b/pharmsol-dsl/src/execution.rs index 2384432a..36def083 100644 --- a/pharmsol-dsl/src/execution.rs +++ b/pharmsol-dsl/src/execution.rs @@ -427,8 +427,8 @@ impl fmt::Display for LoweringError { let span = self.diagnostic.primary_span(); write!( f, - "{} at bytes {}..{}", - self.diagnostic.message, span.start, span.end + "error[{}]: {} (at bytes {}..{})", + self.diagnostic.code, self.diagnostic.message, span.start, span.end ) } } @@ -530,7 +530,12 @@ impl<'a> ExecutionLowerer<'a> { len, span: state.span, }); - next_state_offset += len; + next_state_offset = next_state_offset.checked_add(len).ok_or_else(|| { + LoweringError::new( + "combined state sizes exceed the supported state space", + state.span, + ) + })?; } let uses_authoring_route_kinds = @@ -557,21 +562,21 @@ impl<'a> ExecutionLowerer<'a> { .with_note("lag and bioavailability are bolus-only route properties")); } } - let index = if uses_authoring_route_kinds { - match route.kind.expect("authoring routes must preserve kind") { - RouteKind::Bolus => { - let index = next_bolus_index; - next_bolus_index += 1; - index - } - RouteKind::Infusion => { - let index = next_infusion_index; - next_infusion_index += 1; - index - } + // Authoring source always records a kind per route; canonical + // `model {}` source never does. Mixed models fall back to + // declaration order. + let index = match (uses_authoring_route_kinds, route.kind) { + (true, Some(RouteKind::Bolus)) => { + let index = next_bolus_index; + next_bolus_index += 1; + index + } + (true, Some(RouteKind::Infusion)) => { + let index = next_infusion_index; + next_infusion_index += 1; + index } - } else { - declaration_index + _ => declaration_index, }; route_slots.insert(route.symbol, index); let destination = @@ -1635,9 +1640,8 @@ out(cp) = central / v ~ continuous() let model = crate::parse_model(src).expect("authoring model parses"); let typed = crate::analyze_model(&model).expect("authoring model analyzes"); - let error = crate::lower_typed_model(&typed) - .err() - .expect("infusion lag should fail during lowering"); + let error = + crate::lower_typed_model(&typed).expect_err("infusion lag should fail during lowering"); assert!(error .to_string() @@ -1664,8 +1668,7 @@ out(cp) = central / v ~ continuous() let model = crate::parse_model(src).expect("authoring model parses"); let typed = crate::analyze_model(&model).expect("authoring model analyzes"); let error = crate::lower_typed_model(&typed) - .err() - .expect("infusion bioavailability should fail during lowering"); + .expect_err("infusion bioavailability should fail during lowering"); assert!(error .to_string() diff --git a/pharmsol-dsl/src/ir.rs b/pharmsol-dsl/src/ir.rs index 8909247f..e7750ff2 100644 --- a/pharmsol-dsl/src/ir.rs +++ b/pharmsol-dsl/src/ir.rs @@ -1,6 +1,3 @@ -use std::error::Error; -use std::fmt; - use serde::{Deserialize, Serialize}; use crate::name_match::{ @@ -105,6 +102,9 @@ impl ConstValue { } } + /// Converts to `i64` when the value is finite, integral, and exactly + /// representable; the 2^63 upper bound is exclusive because + /// `i64::MAX as f64` rounds up to it and would saturate the cast. pub fn as_i64(&self) -> Option { match self { ConstValue::Int(value) => Some(*value), @@ -112,7 +112,7 @@ impl ConstValue { if value.is_finite() && value.fract() == 0.0 && *value >= i64::MIN as f64 - && *value <= i64::MAX as f64 => + && *value < -(i64::MIN as f64) => { Some(*value as i64) } @@ -414,17 +414,18 @@ impl AnalyticalStructureInputPlan { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum AnalyticalStructureInputError { - DuplicatePrimary { - name: String, - }, - DuplicateDerived { - name: String, - }, - ConflictingName { - name: String, - }, + #[error("duplicate primary parameter `{name}`")] + DuplicatePrimary { name: String }, + #[error("duplicate derived parameter `{name}`")] + DuplicateDerived { name: String }, + #[error("`{name}` is declared in both `params` and `derived`")] + ConflictingName { name: String }, + #[error( + "analytical structure `{structure}` requires `{name}`; {}declare it in `params` or `derived`", + suggestion_note(.name, .suggestion) + )] MissingRequiredName { structure: &'static str, name: String, @@ -432,31 +433,13 @@ pub enum AnalyticalStructureInputError { }, } -impl fmt::Display for AnalyticalStructureInputError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::DuplicatePrimary { name } => write!(f, "duplicate primary parameter `{name}`"), - Self::DuplicateDerived { name } => write!(f, "duplicate derived parameter `{name}`"), - Self::ConflictingName { name } => { - write!(f, "`{name}` is declared in both `params` and `derived`") - } - Self::MissingRequiredName { - structure, - name, - suggestion, - } => { - write!(f, "analytical structure `{structure}` requires `{name}`; ")?; - if let Some(candidate) = suggestion { - write!(f, "did you mean `{name}` instead of `{candidate}`? ")?; - } - f.write_str("declare it in `params` or `derived`") - } - } +fn suggestion_note(name: &str, suggestion: &Option) -> String { + match suggestion { + Some(candidate) => format!("did you mean `{name}` instead of `{candidate}`? "), + None => String::new(), } } -impl Error for AnalyticalStructureInputError {} - fn best_similar_candidate<'a, I>(needle: &str, candidates: I) -> Option where I: IntoIterator, diff --git a/pharmsol-dsl/src/lexer.rs b/pharmsol-dsl/src/lexer.rs index a1593651..38daa44a 100644 --- a/pharmsol-dsl/src/lexer.rs +++ b/pharmsol-dsl/src/lexer.rs @@ -402,6 +402,13 @@ impl<'a> Lexer<'a> { Span::new(start, self.pos), ) })?; + if !value.is_finite() { + return Err(ParseError::new( + format!("number literal `{raw}` is too large and overflows to infinity"), + Span::new(start, self.pos), + ) + .with_help("use a smaller magnitude, such as `1e308`")); + } Ok(TokenKind::Number(value)) } diff --git a/pharmsol-dsl/src/lib.rs b/pharmsol-dsl/src/lib.rs index 81bc7c44..4ad8c11f 100644 --- a/pharmsol-dsl/src/lib.rs +++ b/pharmsol-dsl/src/lib.rs @@ -11,6 +11,8 @@ //! //! Main entrypoints: //! +//! - [`compile_model`] and [`compile_module`] for the one-shot +//! source-to-execution pipeline, failing with the unified [`DslError`]. //! - [`parse_model`] and [`parse_module`] for turning DSL source text into the //! syntax tree in [`ast`]. //! - [`analyze_model`] and [`analyze_module`] for semantic validation and the @@ -36,10 +38,10 @@ //! - [`execution`] for the lowered execution model shared by JIT, AoT, and //! WASM backends. //! -//! Smallest parse-analyze-lower example: +//! Smallest one-shot example: //! //! ```rust -//! use pharmsol_dsl::{analyze_model, lower_typed_model, parse_model}; +//! use pharmsol_dsl::compile_model; //! //! let source = r#" //! name = bimodal_ke @@ -55,15 +57,38 @@ //! out(cp) = central / v //! "#; //! -//! let syntax = parse_model(source).expect("model parses"); -//! let typed = analyze_model(&syntax).expect("model analyzes"); -//! let execution = lower_typed_model(&typed).expect("model lowers"); +//! let execution = compile_model(source).expect("model compiles"); //! //! assert_eq!(execution.name, "bimodal_ke"); //! assert_eq!(execution.metadata.routes.len(), 1); //! assert_eq!(execution.metadata.outputs.len(), 1); //! ``` //! +//! The same pipeline staged, for callers that need the intermediate +//! representations: +//! +//! ```rust +//! use pharmsol_dsl::{analyze_model, lower_typed_model, parse_model}; +//! +//! # let source = "name = m\nkind = ode\nstates = central\nddt(central) = 0\nout(cp) = central"; +//! let syntax = parse_model(source).expect("model parses"); +//! let typed = analyze_model(&syntax).expect("model analyzes"); +//! let execution = lower_typed_model(&typed).expect("model lowers"); +//! # assert_eq!(execution.name, "m"); +//! ``` +//! +//! Errors from any stage carry source spans and render as annotated reports +//! when printed: +//! +//! ```rust +//! use pharmsol_dsl::compile_model; +//! +//! let error = compile_model("name = m\nstates = central\nout(cp) = missing_state").unwrap_err(); +//! let report = error.to_string(); +//! assert!(report.contains("error[DSL2000]")); +//! assert!(report.contains("unknown identifier `missing_state`")); +//! ``` +//! //! If you are building an authoring tool, custom compiler, or diagnostics UI, //! stay in this crate. If you want a complete source-to-runtime workflow, //! switch to `pharmsol::dsl` in the main crate. @@ -76,6 +101,7 @@ pub mod ir; mod lexer; mod name_match; mod parser; +mod pipeline; mod semantic; #[cfg(test)] mod test_fixtures; @@ -84,6 +110,10 @@ mod test_fixtures; pub const NUMERIC_ROUTE_PREFIX: &str = "input_"; /// Canonical prefix for numeric output labels such as `outeq_1`. pub const NUMERIC_OUTPUT_PREFIX: &str = "outeq_"; +/// Upper bound for compile-time-resolved sizes: state array sizes, route +/// destination indices, and particle counts. Keeps lowered models within a +/// comfortably allocatable state space. +pub const MAX_CONST_USIZE: usize = 1_048_576; pub(crate) const RATE_FUNCTION_NAME: &str = "rate"; pub use ast::*; @@ -92,5 +122,6 @@ pub use execution::{ lower_typed_model, lower_typed_module, ExecutionModel, ExecutionModule, LoweringError, }; pub use ir::*; -pub use parser::{parse_model, parse_module}; +pub use parser::{parse_model, parse_module, MAX_NESTING_DEPTH}; +pub use pipeline::{compile_model, compile_module, DslError}; pub use semantic::{analyze_model, analyze_module, SemanticError}; diff --git a/pharmsol-dsl/src/parser.rs b/pharmsol-dsl/src/parser.rs index 98c6b0a4..355d8511 100644 --- a/pharmsol-dsl/src/parser.rs +++ b/pharmsol-dsl/src/parser.rs @@ -96,11 +96,19 @@ pub(crate) fn parse_place_fragment(src: &str) -> Result { parsed.map_err(|error| error.with_source(src)) } +/// Maximum nesting depth for expressions and statements. +/// +/// Guards against stack exhaustion from pathologically nested input such as +/// `((((...))))`; the limit is far beyond any realistic model. +pub const MAX_NESTING_DEPTH: usize = 256; + struct Parser { tokens: Vec, index: usize, src_len: usize, layout_boundaries: Vec, + depth: usize, + fatal: bool, } #[derive(Clone, Copy)] @@ -129,9 +137,39 @@ impl Parser { index: 0, src_len, layout_boundaries: Vec::new(), + depth: 0, + fatal: false, } } + /// Runs `parse` with one recursion slot; errors once nesting exceeds + /// [`MAX_NESTING_DEPTH`] instead of overflowing the call stack. The depth + /// error is fatal: recovery loops stop so the diagnostic surfaces once + /// instead of cascading per enclosing block. + fn with_depth( + &mut self, + construct: &str, + parse: impl FnOnce(&mut Self) -> Result, + ) -> Result { + if self.depth >= MAX_NESTING_DEPTH { + self.fatal = true; + let span = self + .peek_span() + .unwrap_or_else(|| Span::empty(self.src_len)); + return Err(ParseError::new( + format!( + "{construct} is nested too deeply (maximum nesting depth is {MAX_NESTING_DEPTH})" + ), + span, + ) + .with_help("split the model into simpler expressions or statements")); + } + self.depth += 1; + let result = parse(self); + self.depth -= 1; + result + } + fn parse_module(&mut self) -> Result { let start = self.peek_span().unwrap_or_else(|| Span::empty(0)); let mut models = Vec::new(); @@ -716,12 +754,12 @@ impl Parser { } fn parse_stmt(&mut self) -> Result { - match self.peek_kind() { - Some(TokenKind::If) => self.parse_if_stmt(), - Some(TokenKind::For) => self.parse_for_stmt(), - Some(TokenKind::Let) => self.parse_let_stmt(), - _ => self.parse_assign_stmt(), - } + self.with_depth("statement", |parser| match parser.peek_kind() { + Some(TokenKind::If) => parser.parse_if_stmt(), + Some(TokenKind::For) => parser.parse_for_stmt(), + Some(TokenKind::Let) => parser.parse_let_stmt(), + _ => parser.parse_assign_stmt(), + }) } fn parse_if_stmt(&mut self) -> Result { @@ -734,7 +772,7 @@ impl Parser { .is_some() { if self.at(|kind| matches!(kind, TokenKind::If)) { - let nested = self.parse_if_stmt()?; + let nested = self.with_depth("statement", |parser| parser.parse_if_stmt())?; end_span = nested.span; Some(vec![nested]) } else { @@ -911,11 +949,13 @@ impl Parser { })?; match token.kind { TokenKind::Ident(name) => Ok(Ident::new(name, token.span)), + // `usize::MAX as f64` rounds up to 2^64; keep the bound exclusive + // so the cast never saturates. TokenKind::Number(value) if value.is_finite() && value >= 0.0 && value.fract() == 0.0 - && value <= usize::MAX as f64 => + && value < usize::MAX as f64 => { Ok(Ident::new((value as usize).to_string(), token.span)) } @@ -943,6 +983,12 @@ impl Parser { } fn parse_expr(&mut self, min_precedence: u8) -> Result { + self.with_depth("expression", |parser| { + parser.parse_expr_inner(min_precedence) + }) + } + + fn parse_expr_inner(&mut self, min_precedence: u8) -> Result { let mut lhs = self.parse_prefix_expr()?; while let Some((op, precedence, right_assoc)) = self.peek_binary_op() { if precedence < min_precedence { @@ -976,6 +1022,10 @@ impl Parser { } fn parse_prefix_expr(&mut self) -> Result { + self.with_depth("expression", |parser| parser.parse_prefix_expr_inner()) + } + + fn parse_prefix_expr_inner(&mut self) -> Result { match self.peek_kind() { Some(TokenKind::Plus) => { let operator = self.bump().unwrap(); @@ -1221,6 +1271,9 @@ impl Parser { where F: FnOnce(&TokenKind) -> bool, { + if self.fatal { + return Err(ParseError::aborted()); + } let context = context.into(); let token = self.bump().ok_or_else(|| { ParseError::new( @@ -1589,7 +1642,7 @@ impl Parser { } fn is_eof(&self) -> bool { - self.index >= self.tokens.len() + self.fatal || self.index >= self.tokens.len() } fn peek(&self) -> Option<&Token> { diff --git a/pharmsol-dsl/src/pipeline.rs b/pharmsol-dsl/src/pipeline.rs new file mode 100644 index 00000000..566a519f --- /dev/null +++ b/pharmsol-dsl/src/pipeline.rs @@ -0,0 +1,128 @@ +//! One-shot source-to-execution pipeline and its unified error type. +//! +//! [`compile_model`] and [`compile_module`] run the full frontend pipeline +//! (parse, analyze, lower) in a single call. Failures are reported as +//! [`DslError`], which unifies the phase-specific error types returned by the +//! individual stages behind one `std::error::Error` implementation. + +use std::fmt; +use std::sync::Arc; + +use crate::diagnostic::{Diagnostic, DiagnosticPhase, DiagnosticReport, ParseError}; +use crate::execution::{ + lower_typed_model, lower_typed_module, ExecutionModel, ExecutionModule, LoweringError, +}; +use crate::parser::{parse_model, parse_module}; +use crate::semantic::{analyze_model, analyze_module, SemanticError}; + +/// Error produced by any stage of the DSL frontend pipeline. +/// +/// Use [`DslError::phase`] to learn which stage failed and +/// [`DslError::diagnostics`] for the structured diagnostics. When source text +/// is attached — always the case for errors returned by [`compile_model`] and +/// [`compile_module`] — printing the error renders the annotated report: +/// +/// ```rust +/// use pharmsol_dsl::compile_model; +/// +/// let error = compile_model("model broken { kind ode").unwrap_err(); +/// assert!(error.to_string().contains("error[DSL1000]")); +/// assert_eq!(error.phase(), pharmsol_dsl::DiagnosticPhase::Parse); +/// ``` +#[derive(Clone, thiserror::Error)] +pub enum DslError { + /// Parsing the source text failed. + #[error(transparent)] + Parse(#[from] ParseError), + /// Semantic analysis failed. + #[error(transparent)] + Semantic(#[from] SemanticError), + /// Lowering to the execution model failed. + #[error(transparent)] + Lowering(#[from] LoweringError), +} + +impl DslError { + /// The pipeline phase that produced this error. + pub fn phase(&self) -> DiagnosticPhase { + match self { + Self::Parse(_) => DiagnosticPhase::Parse, + Self::Semantic(_) => DiagnosticPhase::Semantic, + Self::Lowering(_) => DiagnosticPhase::Lowering, + } + } + + /// All diagnostics carried by this error, in source order where known. + pub fn diagnostics(&self) -> &[Diagnostic] { + match self { + Self::Parse(error) => error.diagnostics(), + Self::Semantic(error) => std::slice::from_ref(error.diagnostic()), + Self::Lowering(error) => std::slice::from_ref(error.diagnostic()), + } + } + + /// Attaches source text so `Display` renders the annotated report. + pub fn with_source(self, source: impl Into>) -> Self { + let source = source.into(); + match self { + Self::Parse(error) => Self::Parse(error.with_source(source)), + Self::Semantic(error) => Self::Semantic(error.with_source(source)), + Self::Lowering(error) => Self::Lowering(error.with_source(source)), + } + } + + /// Source text attached to this error, if any. + pub fn source(&self) -> Option<&str> { + match self { + Self::Parse(error) => error.source(), + Self::Semantic(error) => error.source(), + Self::Lowering(error) => error.source(), + } + } + + /// Renders the diagnostics against `src` without attaching it. + pub fn render(&self, src: &str) -> String { + match self { + Self::Parse(error) => error.render(src), + Self::Semantic(error) => error.render(src), + Self::Lowering(error) => error.render(src), + } + } + + /// Structured, JSON-serializable report for editors and tooling. + pub fn diagnostic_report(&self, source_name: impl Into) -> DiagnosticReport { + let source_name = source_name.into(); + match self { + Self::Parse(error) => error.diagnostic_report(source_name), + Self::Semantic(error) => error.diagnostic_report(source_name), + Self::Lowering(error) => error.diagnostic_report(source_name), + } + } +} + +impl fmt::Debug for DslError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self, f) + } +} + +/// Parses, analyzes, and lowers the module in `src` in one call. +/// +/// This is the one-shot equivalent of [`parse_module`], [`analyze_module`], +/// and [`lower_typed_module`] in sequence. Errors carry the source text, so +/// printing them renders the annotated report. +pub fn compile_module(src: &str) -> Result { + let parsed = parse_module(src)?; + let typed = analyze_module(&parsed).map_err(|error| error.with_source(src))?; + lower_typed_module(&typed).map_err(|error| error.with_source(src).into()) +} + +/// Parses, analyzes, and lowers the single model in `src` in one call. +/// +/// Like [`compile_module`], but requires the source to contain exactly one +/// model, mirroring [`parse_model`]. +pub fn compile_model(src: &str) -> Result { + let parsed = parse_model(src)?; + let typed = analyze_model(&parsed).map_err(|error| error.with_source(src))?; + lower_typed_model(&typed).map_err(|error| error.with_source(src).into()) +} diff --git a/pharmsol-dsl/src/semantic.rs b/pharmsol-dsl/src/semantic.rs index 981db69d..01429431 100644 --- a/pharmsol-dsl/src/semantic.rs +++ b/pharmsol-dsl/src/semantic.rs @@ -11,7 +11,9 @@ use crate::ir::*; use crate::name_match::{ common_prefix_len, edit_distance, is_high_confidence_match, is_single_adjacent_transposition, }; -use crate::{ModelKind, NUMERIC_OUTPUT_PREFIX, NUMERIC_ROUTE_PREFIX, RATE_FUNCTION_NAME}; +use crate::{ + ModelKind, MAX_CONST_USIZE, NUMERIC_OUTPUT_PREFIX, NUMERIC_ROUTE_PREFIX, RATE_FUNCTION_NAME, +}; const RESERVED_NAMES: &[&str] = &[ "abs", @@ -211,8 +213,8 @@ impl fmt::Display for SemanticError { let span = self.diagnostic.primary_span(); write!( f, - "{} at bytes {}..{}", - self.diagnostic.message, span.start, span.end + "error[{}]: {} (at bytes {}..{})", + self.diagnostic.code, self.diagnostic.message, span.start, span.end ) } } @@ -1552,6 +1554,12 @@ impl<'a> Analyzer<'a> { expr.span, )); } + if value as u64 > MAX_CONST_USIZE as u64 { + return Err(SemanticError::new( + format!("{context} exceeds the maximum supported value of {MAX_CONST_USIZE}"), + expr.span, + )); + } Ok(value as usize) } @@ -1618,6 +1626,20 @@ impl<'a> Analyzer<'a> { callee.span, ) })?; + match intrinsic.arity() { + IntrinsicArity::Exact(expected) if expected != args.len() => { + return Err(SemanticError::new( + format!( + "function `{}` expects {} argument(s), got {}", + callee.text, + expected, + args.len() + ), + callee.span, + )); + } + _ => {} + } let mut values = Vec::with_capacity(args.len()); for arg in args { values.push(self.evaluate_const_expr(arg, bindings, visiting)?); @@ -2351,7 +2373,9 @@ fn canonical_numeric_suffix<'a>(src: &'a str, prefix: &str) -> Option<&'a str> { } fn numeric_label_literal_suffix(value: f64) -> Option { - (value.is_finite() && value >= 0.0 && value.fract() == 0.0 && value <= usize::MAX as f64) + // `usize::MAX as f64` rounds up to 2^64; keep the bound exclusive so the + // cast below never saturates. + (value.is_finite() && value >= 0.0 && value.fract() == 0.0 && value < usize::MAX as f64) .then(|| (value as usize).to_string()) } @@ -2647,10 +2671,12 @@ fn collect_bare_assignment_names( } fn number_to_const(value: f64) -> ConstValue { + // `i64::MIN as f64` is exactly -2^63, but `i64::MAX as f64` rounds up to + // 2^63, so the upper bound must be exclusive to keep the cast lossless. if value.is_finite() && value.fract() == 0.0 && value >= i64::MIN as f64 - && value <= i64::MAX as f64 + && value < -(i64::MIN as f64) { ConstValue::Int(value as i64) } else { @@ -2716,7 +2742,10 @@ fn fold_unary(op: TypedUnaryOp, value: &ConstValue) -> Option { match (op, value) { (TypedUnaryOp::Plus, ConstValue::Int(value)) => Some(ConstValue::Int(*value)), (TypedUnaryOp::Plus, ConstValue::Real(value)) => Some(ConstValue::Real(*value)), - (TypedUnaryOp::Minus, ConstValue::Int(value)) => Some(ConstValue::Int(-value)), + (TypedUnaryOp::Minus, ConstValue::Int(value)) => Some(match value.checked_neg() { + Some(negated) => ConstValue::Int(negated), + None => ConstValue::Real(-(*value as f64)), + }), (TypedUnaryOp::Minus, ConstValue::Real(value)) => Some(ConstValue::Real(-value)), (TypedUnaryOp::Not, ConstValue::Bool(value)) => Some(ConstValue::Bool(!value)), _ => None, @@ -2737,24 +2766,9 @@ fn fold_binary(op: TypedBinaryOp, lhs: &ConstValue, rhs: &ConstValue) -> Option< TypedBinaryOp::LtEq => Some(ConstValue::Bool(lhs.as_f64()? <= rhs.as_f64()?)), TypedBinaryOp::Gt => Some(ConstValue::Bool(lhs.as_f64()? > rhs.as_f64()?)), TypedBinaryOp::GtEq => Some(ConstValue::Bool(lhs.as_f64()? >= rhs.as_f64()?)), - TypedBinaryOp::Add => fold_numeric( - lhs, - rhs, - |left, right| left + right, - |left, right| left + right, - ), - TypedBinaryOp::Sub => fold_numeric( - lhs, - rhs, - |left, right| left - right, - |left, right| left - right, - ), - TypedBinaryOp::Mul => fold_numeric( - lhs, - rhs, - |left, right| left * right, - |left, right| left * right, - ), + TypedBinaryOp::Add => fold_numeric(lhs, rhs, i64::checked_add, |left, right| left + right), + TypedBinaryOp::Sub => fold_numeric(lhs, rhs, i64::checked_sub, |left, right| left - right), + TypedBinaryOp::Mul => fold_numeric(lhs, rhs, i64::checked_mul, |left, right| left * right), TypedBinaryOp::Div => Some(ConstValue::Real(lhs.as_f64()? / rhs.as_f64()?)), TypedBinaryOp::Pow => Some(ConstValue::Real(lhs.as_f64()?.powf(rhs.as_f64()?))), } @@ -2763,11 +2777,16 @@ fn fold_binary(op: TypedBinaryOp, lhs: &ConstValue, rhs: &ConstValue) -> Option< fn fold_numeric( lhs: &ConstValue, rhs: &ConstValue, - int_op: impl FnOnce(i64, i64) -> i64, + int_op: impl FnOnce(i64, i64) -> Option, real_op: impl FnOnce(f64, f64) -> f64, ) -> Option { match (lhs, rhs) { - (ConstValue::Int(lhs), ConstValue::Int(rhs)) => Some(ConstValue::Int(int_op(*lhs, *rhs))), + (ConstValue::Int(lhs), ConstValue::Int(rhs)) => Some(match int_op(*lhs, *rhs) { + Some(value) => ConstValue::Int(value), + // Overflowing integer arithmetic degrades to `Real`, matching the + // f64 arithmetic the backends perform at runtime. + None => ConstValue::Real(real_op(*lhs as f64, *rhs as f64)), + }), _ => Some(ConstValue::Real(real_op(lhs.as_f64()?, rhs.as_f64()?))), } } diff --git a/pharmsol-dsl/tests/dsl_authoring_edge_cases.rs b/pharmsol-dsl/tests/dsl_authoring_edge_cases.rs index 335a7f86..081df073 100644 --- a/pharmsol-dsl/tests/dsl_authoring_edge_cases.rs +++ b/pharmsol-dsl/tests/dsl_authoring_edge_cases.rs @@ -181,7 +181,7 @@ out(outeq_1) = 3 * central / v .models .first() .expect("authoring DSL should produce one model"); - let typed = analyze_model(&model).expect("mixed output labels should analyze"); + let typed = analyze_model(model).expect("mixed output labels should analyze"); let lowered = lower_typed_model(&typed).expect("mixed output labels should lower"); assert_eq!( diff --git a/pharmsol-dsl/tests/frontend_hardening.rs b/pharmsol-dsl/tests/frontend_hardening.rs new file mode 100644 index 00000000..e3393cda --- /dev/null +++ b/pharmsol-dsl/tests/frontend_hardening.rs @@ -0,0 +1,469 @@ +//! Hardening and API tests for the DSL frontend: error quality, pathological +//! inputs, constant evaluation edges, and the unified pipeline surface. + +use pharmsol_dsl::{ + analyze_model, compile_model, compile_module, lower_typed_model, parse_model, ConstValue, + Diagnostic, DiagnosticCode, DiagnosticPhase, DiagnosticReport, DslError, ParseError, + SemanticError, Span, DSL_PARSE_GENERIC, DSL_SEMANTIC_GENERIC, MAX_CONST_USIZE, + MAX_NESTING_DEPTH, +}; + +fn ode_skeleton(body: &str) -> String { + format!( + "model m {{ kind ode states {{ central }} dynamics {{ {body} }} outputs {{ cp = central }} }}" + ) +} + +fn ode_with_constants(constants: &str) -> String { + format!( + "model m {{ kind ode constants {{ {constants} }} states {{ central }} dynamics {{ ddt(central) = 0 }} outputs {{ cp = central }} }}" + ) +} + +// --------------------------------------------------------------------------- +// Number literals +// --------------------------------------------------------------------------- + +#[test] +fn rejects_number_literals_that_overflow_to_infinity() { + for src in [ + ode_skeleton("ddt(central) = 1e999"), + ode_skeleton("ddt(central) = -1e999"), + "name = m\nkind = ode\nstates = central\nddt(central) = 1e999\nout(cp) = central" + .to_string(), + ] { + let err = parse_model(&src).expect_err("overflowing literal must be rejected"); + let rendered = err.render(&src); + assert!(rendered.contains("overflows to infinity"), "{}", rendered); + } +} + +#[test] +fn accepts_large_but_finite_number_literals() { + let src = ode_skeleton("ddt(central) = 1e308"); + let model = parse_model(&src).expect("finite literal parses"); + let typed = analyze_model(&model).expect("finite literal analyzes"); + assert!(matches!( + typed.constants.as_slice(), + [] // no constants; the literal lives in the dynamics block + )); +} + +// --------------------------------------------------------------------------- +// Recursion depth +// --------------------------------------------------------------------------- + +fn nested_expr(open: &str, close: &str, depth: usize) -> String { + let mut expr = String::new(); + for _ in 0..depth { + expr.push_str(open); + } + expr.push('1'); + for _ in 0..depth { + expr.push_str(close); + } + expr +} + +#[test] +fn rejects_deeply_nested_expressions_without_crashing() { + let parens = ode_skeleton(&format!("ddt(central) = {}", nested_expr("(", ")", 10_000))); + let err = parse_model(&parens).expect_err("deep parens must fail cleanly"); + assert_eq!(err.diagnostics().len(), 1, "{}", err.render(&parens)); + assert!(err.diagnostics()[0].message.contains("nested too deeply")); + + let unary = ode_skeleton(&format!("ddt(central) = {}1", "-".repeat(10_000))); + let err = parse_model(&unary).expect_err("deep unary chain must fail cleanly"); + assert!(err.diagnostics()[0].message.contains("nested too deeply")); + + let caret = ode_skeleton(&format!("ddt(central) = 1{}", "^1".repeat(5_000))); + let err = parse_model(&caret).expect_err("deep caret chain must fail cleanly"); + assert!(err.diagnostics()[0].message.contains("nested too deeply")); +} + +#[test] +fn rejects_deeply_nested_statements_without_crashing() { + let mut body = String::new(); + for _ in 0..1_000 { + body.push_str("if true { "); + } + body.push_str("ddt(central) = 1"); + for _ in 0..1_000 { + body.push_str(" }"); + } + let src = ode_skeleton(&body); + let err = parse_model(&src).expect_err("deep statement nesting must fail cleanly"); + assert!( + err.diagnostics() + .iter() + .any(|diagnostic| diagnostic.message.contains("nested too deeply")), + "{}", + err.render(&src) + ); + + let mut body = String::from("if true { ddt(central) = 1 }"); + for _ in 0..1_000 { + body.push_str(" else if true { ddt(central) = 1 }"); + } + let src = ode_skeleton(&body); + let err = parse_model(&src).expect_err("deep else-if chain must fail cleanly"); + assert!( + err.diagnostics() + .iter() + .any(|diagnostic| diagnostic.message.contains("nested too deeply")), + "{}", + err.render(&src) + ); +} + +#[test] +fn rejects_deeply_nested_authoring_conditionals_without_crashing() { + let mut rhs = String::from("if (true) 1"); + for _ in 0..1_000 { + rhs.push_str(" else if (true) 1"); + } + rhs.push_str(" else 2"); + let src = format!("states = central\nddt(central) = 0\nout(cp) = {rhs}\n"); + let err = parse_model(&src).expect_err("deep authoring conditionals must fail cleanly"); + let rendered = err.render(&src); + assert!(rendered.contains("nested too deeply"), "{}", rendered); +} + +#[test] +fn moderate_nesting_still_parses() { + let parens = ode_skeleton(&format!("ddt(central) = {}", nested_expr("(", ")", 64))); + parse_model(&parens).expect("64 levels of parens parse"); + + let mut body = String::new(); + for _ in 0..100 { + body.push_str("if true { "); + } + body.push_str("ddt(central) = 1"); + for _ in 0..100 { + body.push_str(" }"); + } + parse_model(&ode_skeleton(&body)).expect("100 nested if statements parse"); +} + +// --------------------------------------------------------------------------- +// Constant evaluation +// --------------------------------------------------------------------------- + +fn analyzed_constant(src: &str, name: &str) -> ConstValue { + let model = parse_model(src).expect("model parses"); + let typed = analyze_model(&model).expect("model analyzes"); + typed + .constants + .iter() + .find(|constant| { + typed + .symbols + .iter() + .any(|symbol| symbol.id == constant.symbol && symbol.name == name) + }) + .unwrap_or_else(|| panic!("constant `{name}` exists")) + .value + .clone() +} + +#[test] +fn integer_constant_overflow_degrades_to_real_instead_of_panicking() { + for constants in ["x = 4611686018427387904 * 2", "x = 9223372036854775807 + 1"] { + assert_eq!( + analyzed_constant(&ode_with_constants(constants), "x"), + ConstValue::Real(9.223372036854776e18), + "{constants}" + ); + } + assert_eq!( + analyzed_constant(&ode_with_constants("x = -9223372036854775807 - 2"), "x"), + ConstValue::Real(-9.223372036854776e18) + ); +} + +#[test] +fn negating_i64_min_degrades_to_real_instead_of_panicking() { + let src = ode_with_constants("a = -9223372036854775807 - 1, b = -a"); + assert_eq!( + analyzed_constant(&src, "b"), + ConstValue::Real(9.223372036854776e18) + ); +} + +#[test] +fn constant_two_to_the_63_stays_real() { + let src = ode_with_constants("x = 9223372036854775808"); + assert_eq!( + analyzed_constant(&src, "x"), + ConstValue::Real(9.223372036854776e18) + ); +} + +#[test] +fn compile_time_calls_enforce_intrinsic_arity() { + for (call, message) in [ + ( + "pow(2, 3, 4)", + "function `pow` expects 2 argument(s), got 3", + ), + ("max(1)", "function `max` expects 2 argument(s), got 1"), + ] { + let src = ode_with_constants(&format!("x = {call}")); + let model = parse_model(&src).expect("model parses"); + let err = analyze_model(&model).expect_err("arity mismatch must fail"); + let rendered = err.render(&src); + assert!(rendered.contains(message), "{}", rendered); + } +} + +#[test] +fn state_array_size_rejects_non_representable_integer() { + let src = "model m { kind ode states { x[9223372036854775808], central } dynamics { ddt(central) = 0 } outputs { cp = central } }"; + let model = parse_model(src).expect("model parses"); + let err = analyze_model(&model).expect_err("2^63 state size must fail"); + let rendered = err.render(src); + assert!( + rendered.contains("state array size must be an integer constant"), + "{}", + rendered + ); +} + +#[test] +fn compile_time_sizes_are_capped() { + let src = format!( + "model m {{ kind ode states {{ x[{}], central }} dynamics {{ ddt(central) = 0 }} outputs {{ cp = central }} }}", + MAX_CONST_USIZE + 1 + ); + let model = parse_model(&src).expect("model parses"); + let err = analyze_model(&model).expect_err("oversized state array must fail"); + let rendered = err.render(&src); + assert!( + rendered.contains(&format!( + "state array size exceeds the maximum supported value of {MAX_CONST_USIZE}" + )), + "{}", + rendered + ); + + let src = format!( + "model m {{ kind sde states {{ central }} particles {} drift {{ ddt(central) = 0 }} diffusion {{ noise(central) = 0 }} outputs {{ cp = central }} }}", + MAX_CONST_USIZE + 1 + ); + let model = parse_model(&src).expect("model parses"); + let err = analyze_model(&model).expect_err("oversized particle count must fail"); + let rendered = err.render(&src); + assert!( + rendered.contains(&format!( + "particles exceeds the maximum supported value of {MAX_CONST_USIZE}" + )), + "{}", + rendered + ); + + let src = format!( + "model m {{ kind ode states {{ x[{MAX_CONST_USIZE}], central }} dynamics {{ ddt(x[0]) = 0, ddt(central) = 0 }} outputs {{ cp = central }} }}" + ); + let model = parse_model(&src).expect("model parses"); + analyze_model(&model).expect("state array at the cap still analyzes"); +} + +// --------------------------------------------------------------------------- +// Unified pipeline and DslError +// --------------------------------------------------------------------------- + +const VALID_MODEL: &str = r#" +name = bimodal_ke +kind = ode + +params = ke, v +states = central +outputs = cp + +infusion(iv) -> central + +dx(central) = -ke * central +out(cp) = central / v +"#; + +#[test] +fn compile_model_matches_the_staged_pipeline() { + let staged = lower_typed_model( + &analyze_model(&parse_model(VALID_MODEL).expect("parses")).expect("analyzes"), + ) + .expect("lowers"); + let one_shot = compile_model(VALID_MODEL).expect("compiles"); + + assert_eq!(one_shot.name, staged.name); + assert_eq!(one_shot.metadata.routes.len(), 1); + assert_eq!(one_shot.metadata.outputs.len(), 1); +} + +#[test] +fn compile_module_lowers_every_model() { + let src = "model a { kind ode states { central } dynamics { ddt(central) = 0 } outputs { cp = central } }\nmodel b { kind ode states { central } dynamics { ddt(central) = 0 } outputs { cp = central } }"; + let module = compile_module(src).expect("module compiles"); + assert_eq!(module.models.len(), 2); +} + +#[test] +fn dsl_error_reports_phase_and_renders_source() { + let err = compile_model("model broken { kind ode").unwrap_err(); + assert!(matches!(err, DslError::Parse(_))); + assert_eq!(err.phase(), DiagnosticPhase::Parse); + assert!(!err.diagnostics().is_empty()); + assert!(err.source().is_some()); + assert!(err.to_string().contains("error[DSL1000]")); + assert!(format!("{err:?}").contains("error[DSL1000]")); + + let src = ode_skeleton("ddt(central) = mystery"); + let err = compile_model(&src).unwrap_err(); + assert!(matches!(err, DslError::Semantic(_))); + assert_eq!(err.phase(), DiagnosticPhase::Semantic); + assert!(err.to_string().contains("error[DSL2000]")); + assert!(err.to_string().contains("unknown identifier `mystery`")); + + let src = "name = m\nkind = ode\nstates = central\ninfusion(iv) -> central\nlag(iv) = 0.5\nddt(central) = 0\nout(cp) = central\n"; + let err = compile_model(src).unwrap_err(); + assert!(matches!(err, DslError::Lowering(_))); + assert_eq!(err.phase(), DiagnosticPhase::Lowering); + let rendered = err.to_string(); + assert!( + rendered.contains("does not allow `lag` on infusion route `iv`"), + "{}", + rendered + ); +} + +#[test] +fn dsl_error_builds_structured_reports() { + let err = compile_model("model broken { kind ode").unwrap_err(); + let report: DiagnosticReport = err.diagnostic_report("broken.dsl"); + let json = report.to_json().expect("report serializes"); + assert!(json.contains("broken.dsl")); + assert!(json.contains("DSL1000")); + + let src = ode_skeleton("ddt(central) = mystery"); + let err = compile_model(&src).unwrap_err(); + assert!(err.render(&src).contains("error[DSL2000]")); + let report = err.diagnostic_report("model.dsl"); + assert_eq!(report.diagnostics.len(), 1); +} + +#[test] +fn dsl_error_with_source_renders_after_the_fact() { + let err = parse_model("model broken { kind ode").unwrap_err(); + let err = DslError::from(err); + let rendered = err.with_source("model broken { kind ode").to_string(); + assert!(rendered.contains("error[DSL1000]")); +} + +// --------------------------------------------------------------------------- +// Console formatting +// --------------------------------------------------------------------------- + +#[test] +fn display_without_source_is_compact_and_coded() { + let err = ParseError::new("expected `->`", Span::new(3, 5)); + assert_eq!( + err.to_string(), + "error[DSL1000]: expected `->` (at bytes 3..5)" + ); + + let first = Diagnostic::error( + DSL_PARSE_GENERIC, + DiagnosticPhase::Parse, + "first problem", + Span::new(0, 1), + ); + let second = Diagnostic::error( + DSL_PARSE_GENERIC, + DiagnosticPhase::Parse, + "second problem", + Span::new(2, 3), + ); + let err = ParseError::from_diagnostics(vec![first, second]); + assert_eq!( + err.to_string(), + "error[DSL1000]: first problem (at bytes 0..1) (+1 more error)" + ); + + let err = SemanticError::new("unknown identifier `ke`", Span::new(10, 12)); + assert_eq!( + err.to_string(), + "error[DSL2000]: unknown identifier `ke` (at bytes 10..12)" + ); +} + +#[test] +fn render_does_not_repeat_source_for_labels_on_the_same_line() { + let src = "model m { kind ode constants { x = 1, x = 2 } states { central } dynamics { ddt(central) = x } outputs { cp = central } }"; + let model = parse_model(src).expect("model parses"); + let err = analyze_model(&model).expect_err("duplicate constant must fail"); + let rendered = err.render(src); + assert!(rendered.contains("duplicate constant `x`"), "{}", rendered); + assert_eq!( + rendered.matches("constants { x = 1, x = 2 }").count(), + 1, + "same-line labels share one source excerpt:\n{}", + rendered + ); +} + +#[test] +fn malformed_inputs_fail_with_errors_not_panics() { + let cases = [ + "", + "model", + "model {", + "model m {", + "model m { kind }", + "model m { kind ode", + "states = ", + "states = central\nddt(central) = ", + "states = central\nddt(central) = 1 +\n", + "bolus(iv) -> ", + "x = if (true) 1", + "x = f(", + "params = ke, ", + "kind = invalid", + "model m { kind ode states { x[] } }", + "model m { kind ode routes { oral -> } }", + "model m { kind ode outputs { 18446744073709551616 = central } }", + ]; + for src in cases { + assert!( + parse_model(src).is_err(), + "input should fail cleanly: {src:?}" + ); + } +} + +#[test] +fn route_destination_index_is_bounds_checked() { + let src = "model m { kind ode states { transit[4], central } routes { oral -> transit[9] } dynamics { ddt(transit[0]) = 0, ddt(central) = 0 } outputs { cp = central } }"; + let err = compile_model(src).unwrap_err(); + assert!(matches!(err, DslError::Lowering(_))); + let rendered = err.render(src); + assert!( + rendered.contains("indexes element 9, but state length is 4"), + "{}", + rendered + ); +} + +#[test] +fn diagnostic_codes_are_stable_and_documented() { + assert_eq!(DSL_PARSE_GENERIC, DiagnosticCode::new("DSL1000")); + assert_eq!(DSL_SEMANTIC_GENERIC, DiagnosticCode::new("DSL2000")); + + let src = ode_skeleton(&format!("ddt(central) = {}", nested_expr("(", ")", 1_000))); + let err = parse_model(&src).expect_err("deep nesting must fail"); + assert!( + err.diagnostics()[0] + .message + .contains(&format!("maximum nesting depth is {MAX_NESTING_DEPTH}")), + "{}", + err.diagnostics()[0].message + ); +} diff --git a/src/dsl/jit.rs b/src/dsl/jit.rs index d9691b57..3032c8f1 100644 --- a/src/dsl/jit.rs +++ b/src/dsl/jit.rs @@ -141,8 +141,8 @@ impl std::fmt::Display for JitCompileError { let span = self.diagnostic.primary_span(); write!( f, - "{} at bytes {}..{}", - self.diagnostic.message, span.start, span.end + "error[{}]: {} (at bytes {}..{})", + self.diagnostic.code, self.diagnostic.message, span.start, span.end ) } } diff --git a/src/dsl/native.rs b/src/dsl/native.rs index b0aba8fc..8d3ce6b1 100644 --- a/src/dsl/native.rs +++ b/src/dsl/native.rs @@ -17,7 +17,7 @@ use libloading::Library; use pharmsol_dsl::execution::KernelRole; use pharmsol_dsl::{ AnalyticalKernel, AnalyticalStructureInputKind, AnalyticalStructureInputPlan, ModelKind, - RouteKind, NUMERIC_OUTPUT_PREFIX, NUMERIC_ROUTE_PREFIX, + RouteKind, }; pub use super::model_info::{ @@ -32,6 +32,7 @@ use crate::{ DEFAULT_BOUND_ERROR_MODEL_CACHE_SIZE, DEFAULT_CACHE_SIZE, }, equation::{ + metadata::ValidatedRoute, ode::{closure_helpers::PMProblem, ExplicitRkTableau, OdeSolver, SdirkTableau}, sde::simulate_sde_event_with, EqnKind, Equation, EquationPriv, EquationTypes, Predictions, @@ -653,34 +654,14 @@ impl SharedNativeModel { self.metadata.as_ref() } - fn route_index(&self, name: &str) -> Option { - self.info - .routes - .iter() - .find(|route| route.name == name) - .map(|route| route.index) - } - - fn output_index(&self, name: &str) -> Option { - self.info - .outputs - .iter() - .find(|output| output.name == name) - .map(|output| output.index) - } - fn metadata_route_index_for_label(&self, label: &str) -> Option { - self.route_index(label).or_else(|| { - canonical_numeric_alias(label, NUMERIC_ROUTE_PREFIX) - .and_then(|alias| self.route_index(alias.as_str())) - }) + self.metadata() + .route_for_label(label) + .map(ValidatedRoute::input_index) } fn metadata_output_index_for_label(&self, label: &str) -> Option { - self.output_index(label).or_else(|| { - canonical_numeric_alias(label, NUMERIC_OUTPUT_PREFIX) - .and_then(|alias| self.output_index(alias.as_str())) - }) + self.metadata().output_for_label(label) } fn validate_support_point(&self, support_point: &[f64]) -> Result<(), PharmsolError> { @@ -2572,13 +2553,6 @@ fn sort_events(events: &mut [Event]) { }); } -fn canonical_numeric_alias(label: &str, prefix: &str) -> Option { - if label.is_empty() || !label.chars().all(|ch| ch.is_ascii_digit()) { - return None; - } - Some(format!("{prefix}{label}")) -} - fn build_analytical_parameter_projection( info: &NativeModelInfo, ) -> Result { @@ -2773,11 +2747,10 @@ fn apply_analytical_kernel( #[cfg(test)] mod tests { use super::{ - build_analytical_parameter_projection, canonical_numeric_alias, - project_analytical_parameters, KernelSession, NativeAnalyticalModel, NativeCovariateInfo, - NativeModelInfo, NativeOdeModel, NativeOutputInfo, NativeRouteInfo, NativeSdeModel, - NativeStateInfo, RuntimeArtifact, RuntimeBackend, SharedNativeModel, NUMERIC_OUTPUT_PREFIX, - NUMERIC_ROUTE_PREFIX, + build_analytical_parameter_projection, project_analytical_parameters, KernelSession, + NativeAnalyticalModel, NativeCovariateInfo, NativeModelInfo, NativeOdeModel, + NativeOutputInfo, NativeRouteInfo, NativeSdeModel, NativeStateInfo, RuntimeArtifact, + RuntimeBackend, SharedNativeModel, }; #[cfg(any( feature = "dsl-jit", @@ -3160,31 +3133,6 @@ mod tests { .build() } - #[test] - fn canonical_numeric_alias_maps_bare_numeric_labels_to_contextual_prefixes() { - assert_eq!( - canonical_numeric_alias("1", NUMERIC_ROUTE_PREFIX), - Some("input_1".to_string()) - ); - assert_eq!( - canonical_numeric_alias("10", NUMERIC_OUTPUT_PREFIX), - Some("outeq_10".to_string()) - ); - } - - #[test] - fn canonical_numeric_alias_ignores_symbolic_and_prefixed_labels() { - assert_eq!(canonical_numeric_alias("iv", NUMERIC_ROUTE_PREFIX), None); - assert_eq!( - canonical_numeric_alias("input_1", NUMERIC_ROUTE_PREFIX), - None - ); - assert_eq!( - canonical_numeric_alias("outeq_2", NUMERIC_OUTPUT_PREFIX), - None - ); - } - #[test] fn validate_input_for_kind_reports_structured_route_kind_error() { let model = bolus_only_shared_model(); diff --git a/src/simulator/equation/metadata.rs b/src/simulator/equation/metadata.rs index 43103e03..1e5dbfea 100644 --- a/src/simulator/equation/metadata.rs +++ b/src/simulator/equation/metadata.rs @@ -30,7 +30,10 @@ //! assert!(metadata.output("cp").is_some()); //! ``` -use pharmsol_dsl::{AnalyticalKernel, CovariateInterpolation, ModelKind}; +use pharmsol_dsl::{ + AnalyticalKernel, CovariateInterpolation, ModelKind, NUMERIC_OUTPUT_PREFIX, + NUMERIC_ROUTE_PREFIX, +}; use std::fmt; use thiserror::Error; @@ -206,6 +209,46 @@ impl ValidatedModelMetadata { self.outputs.iter().position(|output| output.name() == name) } + /// Resolve a public route label from data to a declared route. + /// + /// Resolution order: + /// 1. exact route name, + /// 2. canonical numeric alias (`input_`) for a bare numeric label, + /// 3. 1-based declaration ordinal: a bare numeric label `n` selects the + /// n-th declared route, matching Pmetrics `INPUT` numbering. + /// + /// Use [`ValidatedRoute::input_index`] on the result for the dense + /// execution input slot. + pub fn route_for_label(&self, label: &str) -> Option<&ValidatedRoute> { + self.route(label).or_else(|| { + if !is_bare_numeric_label(label) { + return None; + } + self.route(&format!("{NUMERIC_ROUTE_PREFIX}{label}")) + .or_else(|| self.routes.get(bare_numeric_ordinal(label)? - 1)) + }) + } + + /// Resolve a public output label from data to its dense output index. + /// + /// Uses the same resolution order as [`Self::route_for_label`]: exact + /// output name, `outeq_` alias, then the 1-based declaration ordinal + /// matching Pmetrics `OUTEQ` numbering. + pub fn output_for_label(&self, label: &str) -> Option { + self.output_index(label).or_else(|| { + if !is_bare_numeric_label(label) { + return None; + } + self.output_index(&format!("{NUMERIC_OUTPUT_PREFIX}{label}")) + .or_else(|| { + let ordinal = bare_numeric_ordinal(label)?; + ordinal + .checked_sub(1) + .filter(|index| *index < self.outputs.len()) + }) + }) + } + pub fn parameter(&self, name: &str) -> Option<&Parameter> { self.parameter_index(name) .map(|index| &self.parameters[index]) @@ -733,6 +776,22 @@ impl Route { } } +/// Returns `true` for labels consisting only of ASCII digits (`"0"`, `"12"`). +fn is_bare_numeric_label(label: &str) -> bool { + !label.is_empty() && label.chars().all(|ch| ch.is_ascii_digit()) +} + +/// Parse a bare numeric label as a 1-based ordinal. +/// +/// Returns `None` for non-numeric labels and for `0`, which is not a valid +/// Pmetrics `INPUT`/`OUTEQ` number. +fn bare_numeric_ordinal(label: &str) -> Option { + if !is_bare_numeric_label(label) { + return None; + } + label.parse::().ok().filter(|ordinal| *ordinal >= 1) +} + fn resolve_kind( declared_kind: Option, requested_kind: Option, @@ -1018,6 +1077,63 @@ mod tests { ); } + #[test] + fn numeric_labels_resolve_via_alias_before_ordinal() { + let metadata = new("mixed_labels") + .kind(ModelKind::Ode) + .parameters(["ke", "v"]) + .states(["gut", "central"]) + .outputs(["cp", "outeq_0", "outeq_1"]) + .route(Route::infusion("iv").to_state("central")) + .route(Route::bolus("input_0").to_state("gut")) + .validate() + .expect("metadata should validate"); + + // Exact names resolve first. + assert_eq!(metadata.output_for_label("cp"), Some(0)); + assert_eq!( + metadata + .route_for_label("iv") + .map(|route| route.declaration_index()), + Some(0) + ); + + // `0` is not a valid 1-based ordinal, but the `outeq_0`/`input_0` + // aliases must still be consulted. + assert_eq!(metadata.output_for_label("0"), Some(1)); + assert_eq!( + metadata + .route_for_label("0") + .map(|route| route.declaration_index()), + Some(1) + ); + + // An existing alias wins over the 1-based ordinal. + assert_eq!(metadata.output_for_label("1"), Some(2)); + + // Without an alias, the 1-based declaration ordinal applies. + assert_eq!(metadata.output_for_label("2"), Some(1)); + assert_eq!(metadata.output_for_label("3"), Some(2)); + assert_eq!( + metadata + .route_for_label("1") + .map(|route| route.declaration_index()), + Some(0) + ); + assert_eq!( + metadata + .route_for_label("2") + .map(|route| route.declaration_index()), + Some(1) + ); + + // Out-of-range ordinals and unknown labels do not resolve. + assert_eq!(metadata.output_for_label("4"), None); + assert_eq!(metadata.output_for_label("missing"), None); + assert!(metadata.route_for_label("3").is_none()); + assert!(metadata.route_for_label("missing").is_none()); + } + #[test] fn duplicate_names_fail_validation() { let error = new("dup_params") diff --git a/src/simulator/equation/mod.rs b/src/simulator/equation/mod.rs index 875f7c62..974eb89c 100644 --- a/src/simulator/equation/mod.rs +++ b/src/simulator/equation/mod.rs @@ -55,7 +55,6 @@ pub use analytical::*; pub use metadata::*; pub use ode::*; pub use pharmsol_dsl::{AnalyticalKernel, ModelKind}; -use pharmsol_dsl::{NUMERIC_OUTPUT_PREFIX, NUMERIC_ROUTE_PREFIX}; pub use sde::*; use crate::{ @@ -199,15 +198,9 @@ pub(crate) trait EquationPriv: EquationTypes { expected_kind: RouteKind, ) -> Result { if let Some(metadata) = self.metadata() { - let route = metadata - .route(label.as_str()) - .or_else(|| { - canonical_numeric_alias(label.as_str(), NUMERIC_ROUTE_PREFIX) - .and_then(|alias| metadata.route(alias.as_str())) - }) - .ok_or_else(|| { - PharmsolError::unknown_input_label(label.as_str(), &metadata.route_labels()) - })?; + let route = metadata.route_for_label(label.as_str()).ok_or_else(|| { + PharmsolError::unknown_input_label(label.as_str(), &metadata.route_labels()) + })?; if route.kind() != expected_kind { return Err(PharmsolError::UnsupportedInputRouteKind { @@ -229,15 +222,9 @@ pub(crate) trait EquationPriv: EquationTypes { fn resolve_output_label(&self, label: &OutputLabel) -> Result { if let Some(metadata) = self.metadata() { - return metadata - .output_index(label.as_str()) - .or_else(|| { - canonical_numeric_alias(label.as_str(), NUMERIC_OUTPUT_PREFIX) - .and_then(|alias| metadata.output_index(alias.as_str())) - }) - .ok_or_else(|| { - PharmsolError::unknown_output_label(label.as_str(), &metadata.output_labels()) - }); + return metadata.output_for_label(label.as_str()).ok_or_else(|| { + PharmsolError::unknown_output_label(label.as_str(), &metadata.output_labels()) + }); } label @@ -359,13 +346,6 @@ pub(crate) trait EquationPriv: EquationTypes { } } -fn canonical_numeric_alias(label: &str, prefix: &str) -> Option { - if label.is_empty() || !label.chars().all(|ch| ch.is_ascii_digit()) { - return None; - } - Some(format!("{prefix}{label}")) -} - /// Trait for handwritten model equations that can be simulated. /// /// [`Equation`] is the shared interface implemented by handwritten [`ODE`],