From 74c12e264e5d0dcf87d47c3f3f4e0610eb9bf7ff Mon Sep 17 00:00:00 2001 From: Peter Duchovni Date: Mon, 21 Sep 2026 13:39:44 +1000 Subject: [PATCH 01/18] Add integration test for `slice(X-4)` bug Adds a test-case that clarifies the scope of a bug noticed while implemention BSON, where unguarded subtraction that could underflow on nonconformant input bytes led to a panic rather than a recoverable decoder-error while running the doodle interpreter. Error exists for both `doodle::Expr` and `doodle::numeric::core::Expr` versions of subtraction. --- doodle-formats/tests/negative_size.rs | 119 ++++++++++++++++++++++++++ src/helper.rs | 2 +- 2 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 doodle-formats/tests/negative_size.rs diff --git a/doodle-formats/tests/negative_size.rs b/doodle-formats/tests/negative_size.rs new file mode 100644 index 00000000..4f5b0391 --- /dev/null +++ b/doodle-formats/tests/negative_size.rs @@ -0,0 +1,119 @@ +#![cfg(test)] + +use doodle::decoder::{ + Compiler, Program, Value, + seq_kind::{SeqKind, ValueSeq}, +}; +use doodle::helper::*; +use doodle::read::ReadCtxt; +use doodle::{Expr, Format, FormatModule, FormatRef}; +use doodle_numexpr_macro::numexpr; + +#[test] +fn test_native() { + let mut module = FormatModule::new(); + let fallback = + module.define_format("test.negative_size.fallback", fmt_variant("small", u32be())); + let native = module.define_format( + "test.negative_size.native", + union_nondet([ + pseudo_record( + [("x", u32be())], + fmt_variant( + "slice", + slice(sub(as_u32(var("x")), Expr::U32(4)), opaque_bytes()), + ), + ), + fallback.call(), + ]), + ); + + let ok = { + let mut tmp = Vec::new(); + tmp.extend_from_slice(&u32::to_be_bytes(10)); + tmp.extend_from_slice(b"hello!"); + tmp + }; + let err = { + let mut tmp = Vec::new(); + tmp.extend_from_slice(&u32::to_be_bytes(3)); + tmp + }; + let prog = Compiler::compile_program(&module, &native.call()).expect("compilation failed"); + let ctxt_ok = ReadCtxt::new(&ok); + let ctxt_err = ReadCtxt::new(&err); + let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); // should succeed + check_res_ok(res); + let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); // should succeed + check_res_err(res); +} + +#[test] +fn test_numeric() { + let mut module = FormatModule::new(); + let fallback = + module.define_format("test.negative_size.fallback", fmt_variant("small", u32be())); + let numeric = module.define_format( + "test.negative_size.native", + union_nondet([ + pseudo_record( + [("x", u32be())], + fmt_variant( + "slice", + slice(numeric(numexpr!("x" -u32 4)), opaque_bytes()), + ), + ), + fallback.call(), + ]), + ); + + let ok = { + let mut tmp = Vec::new(); + tmp.extend_from_slice(&u32::to_be_bytes(10)); + tmp.extend_from_slice(b"hello!"); + tmp + }; + let err = { + let mut tmp = Vec::new(); + tmp.extend_from_slice(&u32::to_be_bytes(3)); + tmp + }; + let prog = Compiler::compile_program(&module, &numeric.call()).expect("compilation failed"); + let ctxt_ok = ReadCtxt::new(&ok); + let ctxt_err = ReadCtxt::new(&err); + let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); // should succeed + check_res_ok(res); + let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); // will fail + check_res_err(res); +} + +fn check_res_ok(res: Value) { + let Value::Variant(label, inner) = res.extract_mapped_value().into_inner() else { + panic!("expected Variant"); + }; + assert_eq!(label, "slice"); + let Value::Seq(SeqKind::Strict(vs)) = *inner else { + panic!("expected Seq"); + }; + assert_eq!(vs.len(), 6); + let mut actual = Vec::with_capacity(6); + for v in vs { + if let Value::U8(b) = v { + actual.push(b); + } else { + panic!("expected U8"); + } + } + assert_eq!(&actual, b"hello!"); +} + +fn check_res_err(res: Value) { + let Value::Variant(label, inner) = res.extract_mapped_value().into_inner() else { + panic!("expected Variant"); + }; + assert_eq!(label, "small"); + let Value::U32(n) = *inner else { + panic!("expected U32"); + }; + assert_eq!(n, 3); +} diff --git a/src/helper.rs b/src/helper.rs index 38ae3cba..731fd9a9 100644 --- a/src/helper.rs +++ b/src/helper.rs @@ -446,7 +446,7 @@ pub fn alts(branches: impl IntoIterator) /// If the branches in question are all `Format::Variant`, use [`alts_nondet`] instead. /// /// If the given branches can be deterministically distinguished within a fixed finite lookahead, use [`union`] instead. -pub fn union_nondet(branches: impl IntoIterator) -> Format { +pub fn union_nondet(branches: impl IntoIterator) -> Format { Format::UnionNondet(Vec::from_iter(branches)) } From ec18eb74773c88c09cd9709b6aa86ffb3d3a544d Mon Sep 17 00:00:00 2001 From: Peter Duchovni Date: Mon, 21 Sep 2026 14:59:17 +1000 Subject: [PATCH 02/18] Refactor `Expr::eval` and its callees to return Result Adds `doodle::error::EvalError` to bridge the gap between previous panicking behavior in `Expr::eval` and the sub-functions it calls, and a revised approach that turns each non-structural (i.e. data-based) panic in to a return-err case instead. The negative_size integration test now passes as a result of `value::__arith` returning err rather than panicking. --- doodle-formats/tests/negative_size.rs | 10 +- src/decoder.rs | 461 +++++++++++++++----------- src/decoder/value.rs | 262 +++++++-------- src/error.rs | 105 +++++- src/loc_decoder.rs | 347 +++++++++---------- src/numeric/core.rs | 28 +- src/util.rs | 31 +- 7 files changed, 724 insertions(+), 520 deletions(-) diff --git a/doodle-formats/tests/negative_size.rs b/doodle-formats/tests/negative_size.rs index 4f5b0391..8301e9d3 100644 --- a/doodle-formats/tests/negative_size.rs +++ b/doodle-formats/tests/negative_size.rs @@ -42,9 +42,9 @@ fn test_native() { let prog = Compiler::compile_program(&module, &native.call()).expect("compilation failed"); let ctxt_ok = ReadCtxt::new(&ok); let ctxt_err = ReadCtxt::new(&err); - let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); // should succeed + let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); check_res_ok(res); - let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); // should succeed + let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); check_res_err(res); } @@ -81,9 +81,9 @@ fn test_numeric() { let prog = Compiler::compile_program(&module, &numeric.call()).expect("compilation failed"); let ctxt_ok = ReadCtxt::new(&ok); let ctxt_err = ReadCtxt::new(&err); - let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); // should succeed + let (res, _) = prog.run(ctxt_ok).expect("decoding failed on ok-buf"); check_res_ok(res); - let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); // will fail + let (res, _) = prog.run(ctxt_err).expect("decoding failed on err-buf"); check_res_err(res); } @@ -112,7 +112,7 @@ fn check_res_err(res: Value) { panic!("expected Variant"); }; assert_eq!(label, "small"); - let Value::U32(n) = *inner else { + let Value::U32(n) = inner.extract_mapped_value().into_inner() else { panic!("expected U32"); }; assert_eq!(n, 3); diff --git a/src/decoder.rs b/src/decoder.rs index b4d635ef..fd65e669 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -5,11 +5,11 @@ use std::rc::Rc; use anyhow::{Result as AResult, anyhow}; use crate::byte_set::ByteSet; -use crate::error::{DecodeError, DecodeErrorKind, UnknownVarError}; +use crate::error::{DecodeError, DecodeErrorKind, EvalResultExt as _, UnknownVarError}; use crate::fixed::{SpineElem, analyze_fixed_shape}; use crate::read::{BufferKind, ReadCtxt}; use crate::util::{EResult, WithErr}; -use crate::util::{ErrTrace as _, downgrade_error_with}; +use crate::util::{ErrTrace as _, try_downgrade_error_with}; use crate::validation::Condition; use crate::{ BaseKind, DynFormat, Endian, Expr, Format, FormatModule, Label, MatchTree, MaybeTyped, Next, @@ -61,62 +61,66 @@ pub(crate) fn extract_pair(mut vec: Vec) -> (T, T) { } pub mod value; -pub use value::Value; +pub use crate::error::EvalError; +pub use value::{ArithError, ArithOp, Value}; pub type DecodeResult = Result; pub type EDecodeResult = EResult; impl Expr { - pub fn eval<'a>(&'a self, scope: &'a Scope<'a>) -> Cow<'a, Value> { - match self { + pub fn eval<'a>(&'a self, scope: &'a Scope<'a>) -> Result, EvalError> { + Ok(match self { Expr::Var(name) => Cow::Borrowed(scope.get_value_by_name(name).unwrap()), Expr::Bool(b) => Cow::Owned(Value::Bool(*b)), Expr::U8(i) => Cow::Owned(Value::U8(*i)), Expr::U16(i) => Cow::Owned(Value::U16(*i)), Expr::U32(i) => Cow::Owned(Value::U32(*i)), Expr::U64(i) => Cow::Owned(Value::U64(*i)), - Expr::Numeric(n) => match n.eval(scope) { - Ok(v) => Cow::Owned(v.into()), - Err(e) => { - panic!("Expr::eval(Numeric({n:?})) failed during NumExpr evaluation: {e}") - } - }, + Expr::Numeric(n) => Cow::Owned(n.eval(scope)?.into()), Expr::Tuple(exprs) => Cow::Owned(Value::Tuple( - exprs.iter().map(|expr| expr.eval_value(scope)).collect(), + exprs + .iter() + .map(|expr| expr.eval_value(scope)) + .collect::>()?, )), - Expr::TupleProj(head, index) => cow_map(head.eval(scope), |v| { + Expr::TupleProj(head, index) => cow_map(head.eval(scope)?, |v| { v.coerce_mapped_value().tuple_proj(*index).into_inner() }), Expr::Record(fields) => { - Cow::Owned(Value::record(fields.iter().map(|(label, expr)| { - (label.clone(), expr.eval(scope).into_owned()) - }))) + let fields = fields + .iter() + .map(|(label, expr)| Ok((label.clone(), expr.eval(scope)?.into_owned()))) + .collect::, EvalError>>()?; + Cow::Owned(Value::record(fields)) } - Expr::RecordProj(head, label) => cow_map(head.eval(scope), |v| { + Expr::RecordProj(head, label) => cow_map(head.eval(scope)?, |v| { v.coerce_mapped_value() .record_proj(label.as_ref()) .into_inner() }), Expr::Variant(label, expr) => { - Cow::Owned(Value::variant(label.clone(), expr.eval_value(scope))) + Cow::Owned(Value::variant(label.clone(), expr.eval_value(scope)?)) } Expr::Seq(exprs) => Cow::Owned(Value::Seq( - exprs.iter().map(|expr| expr.eval_value(scope)).collect(), + exprs + .iter() + .map(|expr| expr.eval_value(scope)) + .collect::>()?, )), Expr::Match(head, branches) => { - let head = head.eval(scope); + let head = head.eval(scope)?; for (pattern, expr) in branches { if let Some(pattern_scope) = head.matches(scope, pattern) { - let value = expr.eval_value(&Scope::Multi(&pattern_scope)); - return Cow::Owned(value); + let value = expr.eval_value(&Scope::Multi(&pattern_scope))?; + return Ok(Cow::Owned(value)); } } panic!("non-exhaustive patterns"); } Expr::Destructure(head, pat, expr) => { - let head = head.eval(scope); + let head = head.eval(scope)?; if let Some(pattern_scope) = head.matches(scope, pat) { - let value = expr.eval_value(&Scope::Multi(&pattern_scope)); + let value = expr.eval_value(&Scope::Multi(&pattern_scope))?; Cow::Owned(value) } else { panic!("refutable pattern failed to match: {pat:?} :~ {head:?}"); @@ -125,89 +129,79 @@ impl Expr { Expr::Lambda(_, _) => panic!("cannot eval lambda"), Expr::IntRel(rel, x, y) => Cow::Owned({ - let left = x.eval_value(scope); - let right = y.eval_value(scope); - Value::int_rel(*rel, left, right) + let left = x.eval_value(scope)?; + let right = y.eval_value(scope)?; + Value::int_rel(*rel, left, right)? }), Expr::Arith(op, x, y) => Cow::Owned({ - let left = x.eval_value(scope); - let right = y.eval_value(scope); - Value::arith(*op, left, right) + let left = x.eval_value(scope)?; + let right = y.eval_value(scope)?; + Value::arith(*op, left, right)? }), Expr::Unary(op, x) => Cow::Owned({ - let value = x.eval_value(scope); - Value::unary(*op, value) + let value = x.eval_value(scope)?; + Value::unary(*op, value)? }), // FIXME - extract common logic for As-expr on Value instead of separate impl for decoder and loc_decoder - Expr::AsU8(x) => { - Cow::Owned(match x.eval_value(scope) { - Value::U8(x) => Value::U8(x), - Value::U16(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u16 {x}: {err}") - })), - Value::U32(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u32 {x}: {err}") - })), - Value::U64(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u64 {x}: {err}") - })), - Value::Usize(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on usize {x}: {err}") - })), - x => panic!("cannot convert {x:?} to U8"), - }) - } - Expr::AsU16(x) => Cow::Owned(match x.eval_value(scope) { + Expr::AsU8(x) => Cow::Owned(match x.eval_value(scope)? { + Value::U8(x) => Value::U8(x), + Value::U16(x) => Value::U8(u8::try_from(x)?), + Value::U32(x) => Value::U8(u8::try_from(x)?), + Value::U64(x) => Value::U8(u8::try_from(x)?), + Value::Usize(x) => Value::U8(u8::try_from(x)?), + x => panic!("cannot convert {x:?} to U8"), + }), + Expr::AsU16(x) => Cow::Owned(match x.eval_value(scope)? { Value::U8(x) => Value::U16(u16::from(x)), Value::U16(x) => Value::U16(x), - Value::U32(x) => Value::U16(u16::try_from(x).unwrap()), - Value::U64(x) => Value::U16(u16::try_from(x).unwrap()), - Value::Usize(x) => Value::U16(u16::try_from(x).unwrap()), + Value::U32(x) => Value::U16(u16::try_from(x)?), + Value::U64(x) => Value::U16(u16::try_from(x)?), + Value::Usize(x) => Value::U16(u16::try_from(x)?), x => panic!("cannot convert {x:?} to U16"), }), - Expr::AsU32(x) => Cow::Owned(match x.eval_value(scope) { + Expr::AsU32(x) => Cow::Owned(match x.eval_value(scope)? { Value::U8(x) => Value::U32(u32::from(x)), Value::U16(x) => Value::U32(u32::from(x)), Value::U32(x) => Value::U32(x), - Value::U64(x) => Value::U32(u32::try_from(x).unwrap()), - Value::Usize(x) => Value::U32(u32::try_from(x).unwrap()), + Value::U64(x) => Value::U32(u32::try_from(x)?), + Value::Usize(x) => Value::U32(u32::try_from(x)?), x => panic!("cannot convert {x:?} to U32"), }), - Expr::AsU64(x) => Cow::Owned(match x.eval_value(scope) { + Expr::AsU64(x) => Cow::Owned(match x.eval_value(scope)? { Value::U8(x) => Value::U64(u64::from(x)), Value::U16(x) => Value::U64(u64::from(x)), Value::U32(x) => Value::U64(u64::from(x)), Value::U64(x) => Value::U64(x), - Value::Usize(x) => Value::U64(u64::try_from(x).unwrap()), + Value::Usize(x) => Value::U64(u64::try_from(x)?), x => panic!("cannot convert {x:?} to U64"), }), - Expr::U16Be(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U16Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [Value::U8(hi), Value::U8(lo)] => { Cow::Owned(Value::U16(u16::from_be_bytes([*hi, *lo]))) } _ => panic!("U16Be: expected (U8, U8)"), }, - Expr::U16Le(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U16Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [Value::U8(lo), Value::U8(hi)] => { Cow::Owned(Value::U16(u16::from_le_bytes([*lo, *hi]))) } _ => panic!("U16Le: expected (U8, U8)"), }, - Expr::U32Be(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U32Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { Cow::Owned(Value::U32(u32::from_be_bytes([*a, *b, *c, *d]))) } _ => panic!("U32Be: expected (U8, U8, U8, U8)"), }, - Expr::U32Le(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U32Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { Cow::Owned(Value::U32(u32::from_le_bytes([*a, *b, *c, *d]))) } _ => panic!("U32Le: expected (U8, U8, U8, U8)"), }, - Expr::U64Be(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U64Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [ Value::U8(a), Value::U8(b), @@ -222,7 +216,7 @@ impl Expr { ]))), _ => panic!("U32Be: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), }, - Expr::U64Le(bytes) => match bytes.eval_value(scope).unwrap_tuple().as_slice() { + Expr::U64Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { [ Value::U8(a), Value::U8(b), @@ -237,7 +231,7 @@ impl Expr { ]))), _ => panic!("U32Le: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), }, - Expr::AsChar(bytes) => Cow::Owned(match bytes.eval_value(scope) { + Expr::AsChar(bytes) => Cow::Owned(match bytes.eval_value(scope)? { Value::U8(x) => Value::Char(char::from(x)), Value::U16(x) => { Value::Char(char::from_u32(x as u32).unwrap_or(char::REPLACEMENT_CHARACTER)) @@ -246,48 +240,45 @@ impl Expr { Value::Char(char::from_u32(x).unwrap_or(char::REPLACEMENT_CHARACTER)) } Value::U64(x) => Value::Char( - char::from_u32(u32::try_from(x).unwrap()) - .unwrap_or(char::REPLACEMENT_CHARACTER), + char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), ), Value::Usize(x) => Value::Char( - char::from_u32(u32::try_from(x).unwrap()) - .unwrap_or(char::REPLACEMENT_CHARACTER), + char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), ), _ => panic!("AsChar: expected U8, U16, U32, or U64"), }), - Expr::SeqLength(seq) => match seq.eval(scope).coerce_mapped_value().get_sequence() { + Expr::SeqLength(seq) => match seq.eval(scope)?.coerce_mapped_value().get_sequence() { Some(values) => { let len = values.len(); Cow::Owned(Value::U32(len as u32)) } _ => panic!("SeqLength: expected Seq"), }, - Expr::SeqIx(seq, index) => cow_remap(seq.eval(scope), |v| { - match v.coerce_mapped_value().get_sequence() { - Some(values) => match values { - ValueSeq::ValueSeq(values) => { - let index = index.eval_value(scope).unwrap_usize(); - Cow::Borrowed(&values[index]) - } - ValueSeq::IntRange(mut range) => { - let index = index.eval_value(scope).unwrap_usize(); - Cow::Owned(Value::from(range.nth(index).unwrap())) - } - }, - _ => panic!("SeqIx: expected Seq (or RangeFromTo)"), - } - }), + Expr::SeqIx(seq, index) => { + let index = index.eval_value(scope)?.try_as_usize()?; + cow_remap(seq.eval(scope)?, |v| { + match v.coerce_mapped_value().get_sequence() { + Some(values) => match values { + ValueSeq::ValueSeq(values) => Cow::Borrowed(&values[index]), + ValueSeq::IntRange(mut range) => { + Cow::Owned(Value::from(range.nth(index).unwrap())) + } + }, + _ => panic!("SeqIx: expected Seq (or RangeFromTo)"), + } + }) + } Expr::SubSeq(seq, start, length) => { - match seq.eval(scope).coerce_mapped_value().get_sequence() { + match seq.eval(scope)?.coerce_mapped_value().get_sequence() { Some(values) => match values { ValueSeq::ValueSeq(values) => { - let start = start.eval_value(scope).unwrap_usize(); - let length = length.eval_value(scope).unwrap_usize(); + let start = start.eval_value(scope)?.try_as_usize()?; + let length = length.eval_value(scope)?.try_as_usize()?; Cow::Owned(Value::Seq(values.sub_seq(start, length))) } ValueSeq::IntRange(range) => { - let start = start.eval_value(scope).unwrap_usize(); - let length = length.eval_value(scope).unwrap_usize(); + let start = start.eval_value(scope)?.try_as_usize()?; + let length = length.eval_value(scope)?.try_as_usize()?; Cow::Owned(Value::EnumFromTo(sub_range(range, start, length))) } }, @@ -295,10 +286,10 @@ impl Expr { } } Expr::SubSeqInflate(seq, start, length) => { - match seq.eval(scope).coerce_mapped_value().get_sequence() { + match seq.eval(scope)?.coerce_mapped_value().get_sequence() { Some(values) => { - let start = start.eval_value(scope).unwrap_usize(); - let length = length.eval_value(scope).unwrap_usize(); + let start = start.eval_value(scope)?.try_as_usize()?; + let length = length.eval_value(scope)?.try_as_usize()?; let mut vs = Vec::new(); match values { ValueSeq::ValueSeq(vs0) => { @@ -329,27 +320,33 @@ impl Expr { } } Expr::Append(seq0, seq1) => { - match seq0.eval(scope).coerce_mapped_value().get_sequence() { - Some(val_seq0) => match seq1.eval(scope).coerce_mapped_value().get_sequence() { - Some(val_seq1) => { - if val_seq0.is_empty() { - return Cow::Owned(seq1.eval(scope).coerce_mapped_value().cloned()); - } else if val_seq1.is_empty() { - return Cow::Owned(seq0.eval(scope).coerce_mapped_value().cloned()); + match seq0.eval(scope)?.coerce_mapped_value().get_sequence() { + Some(val_seq0) => { + match seq1.eval(scope)?.coerce_mapped_value().get_sequence() { + Some(val_seq1) => { + if val_seq0.is_empty() { + return Ok(Cow::Owned( + seq1.eval(scope)?.coerce_mapped_value().cloned(), + )); + } else if val_seq1.is_empty() { + return Ok(Cow::Owned( + seq0.eval(scope)?.coerce_mapped_value().cloned(), + )); + } + Cow::Owned(Value::Seq(val_seq0.append(val_seq1))) } - Cow::Owned(Value::Seq(val_seq0.append(val_seq1))) + _ => unreachable!("Append: expected Seq in (lhs)"), } - _ => unreachable!("Append: expected Seq in (lhs)"), - }, + } _ => unreachable!("Append: expected Seq in (lhs)"), } } Expr::FlatMap(expr, seq) => { - match seq.eval(scope).coerce_mapped_value().get_sequence() { + match seq.eval(scope)?.coerce_mapped_value().get_sequence() { Some(values) => { let mut vs = Vec::new(); for v in values { - match expr.eval_lambda(scope, &v) { + match expr.eval_lambda(scope, &v)? { Value::Seq(vn) => { vs.extend(vn); } @@ -366,12 +363,12 @@ impl Expr { _ => panic!("FlatMap: expected Seq"), } } - Expr::FlatMapAccum(expr, accum, _accum_type, seq) => match seq.eval_value(scope) { + Expr::FlatMapAccum(expr, accum, _accum_type, seq) => match seq.eval_value(scope)? { Value::Seq(values) => { - let mut accum = accum.eval_value(scope); + let mut accum = accum.eval_value(scope)?; let mut vs = Vec::new(); for v in values { - let ret = expr.eval_lambda(scope, &Value::Tuple(vec![accum, v])); + let ret = expr.eval_lambda(scope, &Value::Tuple(vec![accum, v]))?; accum = match extract_pair(ret.unwrap_tuple()) { (accum, Value::Seq(vn)) => { vs.extend(vn); @@ -384,45 +381,51 @@ impl Expr { } _ => panic!("FlatMapAccum: expected Seq"), }, - Expr::LeftFold(expr, accum, _accum_type, seq) => match seq.eval_value(scope) { + Expr::LeftFold(expr, accum, _accum_type, seq) => match seq.eval_value(scope)? { Value::Seq(values) => { - let mut accum = accum.eval_value(scope); + let mut accum = accum.eval_value(scope)?; for v in values { - let tmp = expr.eval_lambda(scope, &Value::Tuple(vec![accum, v])); + let tmp = expr.eval_lambda(scope, &Value::Tuple(vec![accum, v]))?; accum = tmp } Cow::Owned(accum) } _ => panic!("LeftFold: expected Seq"), }, - Expr::FindByKey(is_sorted, f_get_key, query_key, seq) => match seq.eval_value(scope) { + Expr::FindByKey(is_sorted, f_get_key, query_key, seq) => match seq.eval_value(scope)? { Value::Seq(values) => { - let query = query_key.eval_value(scope); - let eval = |lambda: &Expr, arg: &Value| lambda.eval_lambda(scope, arg); - if *is_sorted { - match search::find_index_by_key_sorted(f_get_key, &query, &values, eval) { - Some(ix) => { - Cow::Owned(Value::Option(Some(Box::new(values[ix].clone())))) - } - None => Cow::Owned(Value::Option(None)), + let query = query_key.eval_value(scope)?; + // `search` is shared with `loc_decoder` and expects an infallible evaluator, so the first + // error is stashed here (the sentinel `query` key makes the search terminate immediately) + let eval_err = std::cell::Cell::new(None); + let eval = |lambda: &Expr, arg: &Value| match lambda.eval_lambda(scope, arg) { + Ok(key) => key, + Err(err) => { + eval_err.set(Some(err)); + query.clone() } + }; + let found = if *is_sorted { + search::find_index_by_key_sorted(f_get_key, &query, &values, eval) } else { - match search::find_index_by_key_unsorted(f_get_key, &query, &values, eval) { - Some(ix) => { - Cow::Owned(Value::Option(Some(Box::new(values[ix].clone())))) - } - None => Cow::Owned(Value::Option(None)), - } + search::find_index_by_key_unsorted(f_get_key, &query, &values, eval) + }; + if let Some(err) = eval_err.take() { + return Err(err); + } + match found { + Some(ix) => Cow::Owned(Value::Option(Some(Box::new(values[ix].clone())))), + None => Cow::Owned(Value::Option(None)), } } _ => panic!("FindByKey: expected Seq"), }, - Expr::FlatMapList(expr, _ret_type, seq) => match seq.eval_value(scope) { + Expr::FlatMapList(expr, _ret_type, seq) => match seq.eval_value(scope)? { Value::Seq(values) => { let mut vs = Vec::new(); for v in values { let arg = Value::Tuple(vec![Value::Seq(SeqKind::Strict(vs)), v]); - if let Value::Seq(vn) = expr.eval_lambda(scope, &arg) { + if let Value::Seq(vn) = expr.eval_lambda(scope, &arg)? { vs = match arg { Value::Tuple(mut args) => match args.remove(0) { Value::Seq(vs) => vs.into_vec(), @@ -440,34 +443,37 @@ impl Expr { _ => panic!("FlatMapList: expected Seq"), }, Expr::Dup(count, expr) => { - let count = count.eval_value(scope).unwrap_usize(); - let v = expr.eval_value(scope); + let count = count.eval_value(scope)?.try_as_usize()?; + let v = expr.eval_value(scope)?; Cow::Owned(Value::Seq(SeqKind::Dup(count, Box::new(v)))) } Expr::EnumFromTo(start, stop) => { - let start = start.eval_value(scope).unwrap_usize(); - let stop = stop.eval_value(scope).unwrap_usize(); + let start = start.eval_value(scope)?.try_as_usize()?; + let stop = stop.eval_value(scope)?.try_as_usize()?; Cow::Owned(Value::EnumFromTo(start..stop)) } Expr::LiftOption(opt) => match opt { - Some(expr) => Cow::Owned(Value::Option(Some(Box::new(expr.eval_value(scope))))), + Some(expr) => Cow::Owned(Value::Option(Some(Box::new(expr.eval_value(scope)?)))), None => Cow::Owned(Value::Option(None)), }, - } + }) } - fn eval_value_ref<'a, 'b: 'a>(&'b self, scope: &'a Scope<'a>) -> Cow<'a, Value> { - match self.eval(scope) { + fn eval_value_ref<'a, 'b: 'a>( + &'b self, + scope: &'a Scope<'a>, + ) -> Result, EvalError> { + Ok(match self.eval(scope)? { Cow::Borrowed(value) => Cow::Borrowed(value.coerce_mapped_value().into_inner()), Cow::Owned(v) => Cow::Owned(v.extract_mapped_value().into_inner()), - } + }) } - pub fn eval_value<'a>(&self, scope: &'a Scope<'a>) -> Value { - self.eval_value_ref(scope).into_owned() + pub fn eval_value<'a>(&self, scope: &'a Scope<'a>) -> Result { + Ok(self.eval_value_ref(scope)?.into_owned()) } - fn eval_lambda<'a>(&self, scope: &'a Scope<'a>, arg: &Value) -> Value { + fn eval_lambda<'a>(&self, scope: &'a Scope<'a>, arg: &Value) -> Result { match self { Expr::Lambda(name, expr) => { let child_scope = SingleScope::new(scope, name, arg); @@ -620,6 +626,7 @@ impl<'a> Compiler<'a> { } } + /// Compiles a top-level entry-point `Format` into a `Program` using the provided `FormatModule`. pub fn compile_program(module: &FormatModule, format: &Format) -> AResult { let mut compiler = Compiler::new(module); // type @@ -641,6 +648,9 @@ impl<'a> Compiler<'a> { n } + /// Single-entrypoint for compiling a standalone `Format` that does not call into any `FormatRef`s. + /// + /// Constructs an empty `FormatModule` to use for registration. pub fn compile_one(format: &Format) -> AResult { let module = FormatModule::new(); let mut compiler = Compiler::new(&module); @@ -1261,7 +1271,9 @@ impl Decoder { Decoder::Call(n, es, vs) => { let mut new_scope = MultiScope::with_capacity(&Scope::Empty, es.len()); for (name, e) in es { - let v = e.eval_value(scope); + let v = e + .eval_value(scope) + .trace_eval(|| ("Call(e)", format!("{e:?}")))?; new_scope.push_owned(name.clone(), v); } for (name, v) in vs { @@ -1400,7 +1412,9 @@ impl Decoder { } Decoder::DecodeBytes(bytes, a) => { let bytes = { - let raw = bytes.eval_value(scope); + let raw = bytes + .eval_value(scope) + .trace_eval(|| ("DecodeBytes(bytes)", format!("{bytes:?}")))?; let seq_vals = raw.get_sequence().expect("bad type for DecodeBytes input"); seq_vals .into_iter() @@ -1439,7 +1453,9 @@ impl Decoder { } Decoder::ForEach(expr, lbl, a) => { // we need val because it would otherwise be a dropped temporary binding - let val = expr.eval_value(scope); + let val = expr + .eval_value(scope) + .trace_eval(|| ("ForEach(expr)", format!("{expr:?}")))?; let seq = val.get_sequence().expect("bad type for ForEach input"); Ok(WithErr::fold( (Vec::with_capacity(seq.len()), input), @@ -1456,7 +1472,10 @@ impl Decoder { .map(|(v, input)| (Value::Seq(v.into()), input))) } Decoder::RepeatCount(expr, a) => { - let count = expr.eval_value(scope).unwrap_usize(); + let count = expr + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("RepeatCount(expr)", format!("{expr:?}")))?; Ok(WithErr::fold( (Vec::with_capacity(count), input), 0..count, @@ -1473,8 +1492,14 @@ impl Decoder { .map(|(v, input)| (Value::Seq(v.into()), input))) } Decoder::RepeatBetween(reps_left_tree, min, max, a) => { - let min = min.eval_value(scope).unwrap_usize(); - let max = max.eval_value(scope).unwrap_usize(); + let min = min + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("RepeatBetween(min)", format!("{min:?}")))?; + let max = max + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("RepeatBetween(max)", format!("{max:?}")))?; let mut res = WithErr::new((Vec::new(), input)); loop { let v = &res.as_ref().0; @@ -1505,7 +1530,10 @@ impl Decoder { Ok(res.map(|(v, input)| (Value::Seq(v.into()), input))) } Decoder::Maybe(expr, a) => { - let is_present = expr.eval_value(scope).unwrap_bool(); + let is_present = expr + .eval_value(scope) + .trace_eval(|| ("Maybe(expr)", format!("{expr:?}")))? + .unwrap_bool(); if is_present { Ok(try_with!(a.parse(program, scope, input) => "Maybe") .map(|(val, input)| (Value::Option(Some(Box::new(val))), input))) @@ -1518,11 +1546,15 @@ impl Decoder { let mut res = WithErr::new((Vec::new(), input, false)); loop { res = res.join(|(mut v, input, _done)| { - Ok(try_with!(a.parse(program, scope, input) => ("RepeatUntilLast", v.len())).map(|(va, next_input)| { - let done = expr.eval_lambda(scope, &va).unwrap_bool(); - v.push(va); - (v, next_input, done) - })) + try_with!(a.parse(program, scope, input) => ("RepeatUntilLast", v.len())) + .join(|(va, next_input)| { + let done = expr + .eval_lambda(scope, &va) + .trace_eval(|| ("RepeatUntilLast(expr)", format!("{expr:?}")))? + .unwrap_bool(); + v.push(va); + Ok(WithErr::new((v, next_input, done))) + }) })?; break_if_done!(res => (v, input)); } @@ -1533,42 +1565,42 @@ impl Decoder { let mut res = WithErr::new((Vec::new(), input, false)); loop { res = res.join(|(mut v, input, _done)| { - Ok( - try_with!(a.parse(program, scope, input) => ("RepeatUntilSeq", format!("len={}", v.len()), format!("{a:?}"))) - .map(|(va, next_input)| { - v.push(va); - let vs = Value::Seq(v.into()); - let done = expr.eval_lambda(scope, &vs).unwrap_bool(); - let v = match vs { - Value::Seq(v) => v.into_vec(), - _ => unreachable!(), - }; - (v, next_input, done) - }), - ) + try_with!(a.parse(program, scope, input) => ("RepeatUntilSeq", format!("len={}", v.len()), format!("{a:?}"))) + .join(|(va, next_input)| { + v.push(va); + let vs = Value::Seq(v.into()); + let done = expr.eval_lambda(scope, &vs).trace_eval(|| ("RepeatUntilSeq(expr)", format!("{expr:?}")))?.unwrap_bool(); + let v = match vs { + Value::Seq(v) => v.into_vec(), + _ => unreachable!(), + }; + Ok(WithErr::new((v, next_input, done))) + }) })?; break_if_done!(res => (v, input)); } Ok(res.map(|(v, input, _)| (Value::Seq(v.into()), input))) } Decoder::AccumUntil(f_done, f_update, init, _vt, a) => { - let accum = init.eval_value(scope); + let accum = init + .eval_value(scope) + .trace_eval(|| ("AccumUntil(init)", format!("{init:?}")))?; let mut res = WithErr::new((Vec::new(), accum, input, false)); loop { res = res.join(|(mut v, accum, input, _done)| { let done_arg = Value::Tuple(vec![accum.clone(), Value::Seq(v.clone().into())]); - let is_done = f_done.eval_lambda(scope, &done_arg).unwrap_bool(); + let is_done = f_done.eval_lambda(scope, &done_arg).trace_eval(|| ("AccumUntil(f_done)", format!("{f_done:?}")))?.unwrap_bool(); if is_done { return Ok(WithErr::new((v, accum, input, true))); } - Ok(try_with!(a.parse(program, scope, input) => ("AccumUntil", format!("len={}", v.len()), format!("accum={:?}", accum))) - .map(|(next_elem, next_input)| { + try_with!(a.parse(program, scope, input) => ("AccumUntil", format!("len={}", v.len()), format!("accum={:?}", accum))) + .join(|(next_elem, next_input)| { v.push(next_elem.clone()); let update_arg = Value::Tuple(vec![accum.clone(), next_elem]); - let next_accum = f_update.eval_lambda(scope, &update_arg); - (v, next_accum, next_input, false) - })) + let next_accum = f_update.eval_lambda(scope, &update_arg).trace_eval(|| ("AccumUntil(f_update)", format!("{f_update:?}")))?; + Ok(WithErr::new((v, next_accum, next_input, false))) + }) })?; break_if_done!(res => (v, accum, input)); } @@ -1586,7 +1618,10 @@ impl Decoder { } } Decoder::Slice(expr, a) => { - let size = expr.eval_value(scope).unwrap_usize(); + let size = expr + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("Slice(expr)", format!("{expr:?}")))?; let (mut slice, input) = input.split_at(size).ok_or( input .kind @@ -1615,8 +1650,14 @@ impl Decoder { }) } Decoder::WithRelativeOffset(base_addr, expr, a) => { - let base = base_addr.eval_value(scope).unwrap_usize(); - let offset = expr.eval_value(scope).unwrap_usize(); + let base = base_addr + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("WithRelativeOffset(base_addr)", format!("{base_addr:?}")))?; + let offset = expr + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("WithRelativeOffset(expr)", format!("{expr:?}")))?; let abs_offset = base + offset; let seek_input = input.seek_to(abs_offset).ok_or( input @@ -1627,19 +1668,28 @@ impl Decoder { Ok(try_with!(a.parse(program, scope, seek_input) => ("WithRelativeOffset(parse)", format!("{a:?}"))) .map(|(v, _)| (v, input))) } - Decoder::Map(d, expr) => Ok( - try_with!(d.parse(program, scope, input) => ("Map(parse)", format!("{d:?}"))).map( + Decoder::Map(d, expr) => { + try_with!(d.parse(program, scope, input) => ("Map(parse)", format!("{d:?}"))).join( |(orig, input)| { - let v = expr.eval_lambda(scope, &orig); - (Value::Mapped(Box::new(orig), Box::new(v)), input) + let v = expr + .eval_lambda(scope, &orig) + .trace_eval(|| ("Map(expr)", format!("{expr:?}")))?; + Ok(WithErr::new(( + Value::Mapped(Box::new(orig), Box::new(v)), + input, + ))) }, - ), - ), + ) + } Decoder::Where(d, cond) => { try_with!(d.parse(program, scope, input) => ("Where(parse)", format!("{d:?}"))) .join(|(v, input)| { let Condition { expr, severity } = cond; - match expr.eval_lambda(scope, &v).unwrap_bool() { + match expr + .eval_lambda(scope, &v) + .trace_eval(|| ("Where(expr)", format!("{expr:?}")))? + .unwrap_bool() + { true => Ok(WithErr::new((v, input))), false => { let err = DecodeErrorKind::bad_where( @@ -1658,11 +1708,15 @@ impl Decoder { }) } Decoder::Compute(expr) => { - let v = expr.eval_value(scope); + let v = expr + .eval_value(scope) + .trace_eval(|| ("Compute(expr)", format!("{expr:?}")))?; Ok(WithErr::new((v, input))) } Decoder::Let(name, expr, d) => { - let v = expr.eval_value(scope); + let v = expr + .eval_value(scope) + .trace_eval(|| ("Let(expr)", format!("{expr:?}")))?; let let_scope = SingleScope::new(scope, name, &v); Ok( try_with!(d.parse(program, &Scope::Single(let_scope), input) => ("Let(parse)", format!("{} := {:?} <- {:?}", name, v, expr), format!("{d:?}"))), @@ -1677,7 +1731,9 @@ impl Decoder { ) } Decoder::Match(head, branches) => { - let head = head.eval(scope); + let head = head + .eval(scope) + .trace_eval(|| ("Match(head)", format!("{head:?}")))?; for (index, (pattern, decoder)) in branches.iter().enumerate() { if let Some(pattern_scope) = head.matches(scope, pattern) { return Ok(try_with!(decoder.parse(program, &Scope::Multi(&pattern_scope), input) @@ -1694,12 +1750,18 @@ impl Decoder { })) } Decoder::Dynamic(name, DynFormat::Huffman(lengths_expr, opt_values_expr), d) => { - let lengths_val = lengths_expr.eval(scope); + let lengths_val = lengths_expr + .eval(scope) + .trace_eval(|| ("Dynamic(lengths_expr)", format!("{lengths_expr:?}")))?; let lengths = value_to_vec_usize(lengths_val.as_ref()); let lengths = match opt_values_expr { None => lengths, Some(e) => { - let values = value_to_vec_usize(e.eval(scope).as_ref()); + let values = value_to_vec_usize( + e.eval(scope) + .trace_eval(|| ("Dynamic(e)", format!("{e:?}")))? + .as_ref(), + ); let mut new_lengths = [0].repeat(values.len()); for i in 0..lengths.len() { new_lengths[values[i]] = lengths[i]; @@ -1725,7 +1787,10 @@ impl Decoder { .parse(program, scope, input) => ("LiftedOption(parse)", format!("{dec:?}"))) .map(|(v, input)| (Value::Option(Some(Box::new(v))), input))), Decoder::CaptureBytes(v_expr, len) => { - let len = len.eval_value(scope).unwrap_usize(); + let len = len + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("CaptureBytes(len)", format!("{len:?}")))?; let view_window = Self::eval_view_expr(scope, v_expr)?; @@ -1748,7 +1813,10 @@ impl Decoder { Ok(WithErr::new((Value::Seq(SeqKind::Strict(accum)), input))) } Decoder::ReadArray(v_expr, len, kind) => { - let len = len.eval_value(scope).unwrap_usize(); + let len = len + .eval_value(scope) + .and_then(|v| v.try_as_usize()) + .trace_eval(|| ("ReadArray(len)", format!("{len:?}")))?; let view_window = Self::eval_view_expr(scope, v_expr)?; let folded = WithErr::fold( @@ -1783,16 +1851,17 @@ impl Decoder { } } } - Decoder::Permit(a, expr) => Ok(downgrade_error_with( + Decoder::Permit(a, expr) => Ok(try_downgrade_error_with( a.parse(program, scope, input) .map(|ok| ok.map(|(v, input)| (Value::Permit(Ok(Box::new(v))), input))), || { - ( - Value::Permit(Err(Some(Box::new(expr.eval_value(scope))))), + Ok::<_, EvalError>(( + Value::Permit(Err(Some(Box::new(expr.eval_value(scope)?)))), input, - ) + )) }, - )), + ) + .trace_eval(|| ("Permit(default)", format!("{expr:?}")))?), } } /// Given a `ViewExpr` and a `Scope` to evaluate named views under, returns the appropriate `View` @@ -1813,7 +1882,7 @@ impl Decoder { Ok(view) } ViewExpr::Offset(base, offset) => { - let offset = offset.eval_value(scope).unwrap_usize(); + let offset = offset.eval_value(scope)?.try_as_usize()?; let base_view = Self::eval_view_expr(scope, base)?; let Some((_, mut view_window)) = base_view.split_at(offset) else { return Err(base_view.kind.overrun(offset, base_view.offset).into()); diff --git a/src/decoder/value.rs b/src/decoder/value.rs index d69825ef..3b1af9a0 100644 --- a/src/decoder/value.rs +++ b/src/decoder/value.rs @@ -3,6 +3,7 @@ use std::rc::Rc; use num_bigint::BigInt; use serde::Serialize; +use crate::error::EvalError; use crate::numeric::core::{TypedConst, Value as NumValue}; use crate::{Arith, IntRel, IntoLabel, Label, Pattern, UnaryOp}; @@ -488,23 +489,22 @@ impl Value { Value::Variant(label.into(), value.into()) } - /// Unwraps any compatible numeric-typed `Value` and returns the contained number as a `usize`. + /// Converts any compatible numeric-typed `Value` to a `usize`, returning `Err` if the number is not + /// representable as one (e.g. a negative or oversized `Numeric`). /// /// # Panics /// - /// Panics if the value is not numeric. - /// - /// May additionally panic in rare cases such as `u64`-to-`usize` conversion on 32-bit architectures. - pub(crate) fn unwrap_usize(&self) -> usize { - match self { + /// Panics if the value is not numeric at all, as this is an invariant enforced by the type-checker. + pub(crate) fn try_as_usize(&self) -> Result { + Ok(match self { Value::U8(n) => usize::from(*n), Value::U16(n) => usize::from(*n), - Value::U32(n) => usize::try_from(*n).unwrap(), - Value::U64(n) => usize::try_from(*n).unwrap(), + Value::U32(n) => usize::try_from(*n)?, + Value::U64(n) => usize::try_from(*n)?, Value::Usize(n) => *n, - Value::Numeric(tc) => tc.as_usize().unwrap(), + Value::Numeric(tc) => tc.as_usize()?, other => panic!("value is not a number: {other:?}"), - } + }) } /// Unwraps `Value::U8` and returns the contained value, or panics if the value is not `Value::U8`. @@ -548,7 +548,33 @@ where } } -fn __arith(arith: Arith, left: T, right: T) -> T +/// The operator that failed in an [`ArithError`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ArithOp { + Binary(Arith), + Unary(UnaryOp), +} + +/// Error produced when checked integer arithmetic underflows, overflows, or divides by zero. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ArithError { + pub op: ArithOp, + pub type_name: &'static str, +} + +impl std::fmt::Display for ArithError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "arithmetic error: {:?} on {} underflowed, overflowed, or divided by zero", + self.op, self.type_name + ) + } +} + +impl std::error::Error for ArithError {} + +fn __arith(arith: Arith, left: T, right: T) -> Result where T: num_traits::CheckedAdd, T: num_traits::CheckedSub, @@ -561,117 +587,81 @@ where T: std::ops::BitOr, T: std::ops::BitAnd, { + let err = || ArithError { + op: ArithOp::Binary(arith), + type_name: std::any::type_name::(), + }; match arith { - Arith::Add => left - .checked_add(&right) - .unwrap_or_else(|| panic!("integer overflow")), - Arith::Sub => left - .checked_sub(&right) - .unwrap_or_else(|| panic!("integer overflow")), - Arith::Mul => left - .checked_mul(&right) - .unwrap_or_else(|| panic!("integer overflow")), - Arith::Div => left - .checked_div(&right) - .unwrap_or_else(|| panic!("integer overflow")), - Arith::Rem => left - .checked_rem(&right) - .unwrap_or_else(|| panic!("integer overflow")), - Arith::Shl => left << right.as_(), - Arith::Shr => left >> right.as_(), - Arith::BitOr => left | right, - Arith::BitAnd => left & right, + Arith::Add => left.checked_add(&right).ok_or_else(err), + Arith::Sub => left.checked_sub(&right).ok_or_else(err), + Arith::Mul => left.checked_mul(&right).ok_or_else(err), + Arith::Div => left.checked_div(&right).ok_or_else(err), + Arith::Rem => left.checked_rem(&right).ok_or_else(err), + Arith::Shl => left.checked_shl(right.as_()).ok_or_else(err), + Arith::Shr => left.checked_shr(right.as_()).ok_or_else(err), + Arith::BitOr => Ok(left | right), + Arith::BitAnd => Ok(left & right), Arith::BoolOr | Arith::BoolAnd => unreachable!("bool ops should be handled separately"), } } -fn __unary(op: UnaryOp, value: T) -> T +fn __unary(op: UnaryOp, value: T) -> Result where T: num_traits::CheckedAdd, T: num_traits::CheckedSub, T: num_traits::One, { + let err = || ArithError { + op: ArithOp::Unary(op), + type_name: std::any::type_name::(), + }; match op { - UnaryOp::IntPred => value.checked_sub(&T::one()).unwrap_or_else(|| { - panic!( - "__unary::<{}>@IntPred: integer underflow", - std::any::type_name::() - ) - }), - UnaryOp::IntSucc => value.checked_add(&T::one()).unwrap_or_else(|| { - panic!( - "__unary::<{}>@IntSucc: integer overflow", - std::any::type_name::() - ) - }), + UnaryOp::IntPred => value.checked_sub(&T::one()).ok_or_else(err), + UnaryOp::IntSucc => value.checked_add(&T::one()).ok_or_else(err), UnaryOp::BoolNot => unreachable!("bool ops should be handled separately"), } } impl Value { - pub fn int_rel(rel: IntRel, left: Value, right: Value) -> Value { + pub fn int_rel(rel: IntRel, left: Value, right: Value) -> Result { match (left, right) { - (Value::U8(l), Value::U8(r)) => Value::Bool(__rel(rel, l, r)), - (Value::U16(l), Value::U16(r)) => Value::Bool(__rel(rel, l, r)), - (Value::U32(l), Value::U32(r)) => Value::Bool(__rel(rel, l, r)), - (Value::U64(l), Value::U64(r)) => Value::Bool(__rel(rel, l, r)), - (Value::Usize(l), Value::Usize(r)) => Value::Bool(__rel(rel, l, r)), - (Value::Numeric(l), Value::Numeric(r)) => Value::Bool(TypedConst::rel(rel, &l, &r)), - (ref left @ Value::Numeric(ref num), ref right @ Value::U8(r)) => { - if let Ok(l) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::U8(l), Value::U8(r)) => Ok(Value::Bool(__rel(rel, l, r))), + (Value::U16(l), Value::U16(r)) => Ok(Value::Bool(__rel(rel, l, r))), + (Value::U32(l), Value::U32(r)) => Ok(Value::Bool(__rel(rel, l, r))), + (Value::U64(l), Value::U64(r)) => Ok(Value::Bool(__rel(rel, l, r))), + (Value::Usize(l), Value::Usize(r)) => Ok(Value::Bool(__rel(rel, l, r))), + (Value::Numeric(l), Value::Numeric(r)) => Ok(Value::Bool(TypedConst::rel(rel, &l, &r))), + (Value::Numeric(ref num), Value::U8(r)) => { + let l = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::U8(l), ref right @ Value::Numeric(ref num)) => { - if let Ok(r) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::U8(l), Value::Numeric(ref num)) => { + let r = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::Numeric(ref num), ref right @ Value::U16(r)) => { - if let Ok(l) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::Numeric(ref num), Value::U16(r)) => { + let l = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::U16(l), ref right @ Value::Numeric(ref num)) => { - if let Ok(r) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::U16(l), Value::Numeric(ref num)) => { + let r = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::Numeric(ref num), ref right @ Value::U32(r)) => { - if let Ok(l) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::Numeric(ref num), Value::U32(r)) => { + let l = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::U32(l), ref right @ Value::Numeric(ref num)) => { - if let Ok(r) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::U32(l), Value::Numeric(ref num)) => { + let r = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::Numeric(ref num), ref right @ Value::U64(r)) => { - if let Ok(l) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::Numeric(ref num), Value::U64(r)) => { + let l = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } - (ref left @ Value::U64(l), ref right @ Value::Numeric(ref num)) => { - if let Ok(r) = num.get_as_unsized::() { - Value::Bool(__rel(rel, l, r)) - } else { - panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") - } + (Value::U64(l), Value::Numeric(ref num)) => { + let r = num.get_as_unsized::()?; + Ok(Value::Bool(__rel(rel, l, r))) } (left, right) => { panic!("cannot apply int-rel {rel:?} to (`{left:?}`, `{right:?}`)") @@ -679,12 +669,12 @@ impl Value { } } - pub fn arith(arith: Arith, left: Value, right: Value) -> Value { + pub fn arith(arith: Arith, left: Value, right: Value) -> Result { if matches!(arith, Arith::BoolOr | Arith::BoolAnd) { match (left, right) { (Value::Bool(l), Value::Bool(r)) => match arith { - Arith::BoolOr => Value::Bool(l || r), - Arith::BoolAnd => Value::Bool(l && r), + Arith::BoolOr => Ok(Value::Bool(l || r)), + Arith::BoolAnd => Ok(Value::Bool(l && r)), _ => unreachable!(), }, (left, right) => { @@ -695,11 +685,11 @@ impl Value { } } else { match (left, right) { - (Value::U8(l), Value::U8(r)) => Value::U8(__arith(arith, l, r)), - (Value::U16(l), Value::U16(r)) => Value::U16(__arith(arith, l, r)), - (Value::U32(l), Value::U32(r)) => Value::U32(__arith(arith, l, r)), - (Value::U64(l), Value::U64(r)) => Value::U64(__arith(arith, l, r)), - (Value::Usize(l), Value::Usize(r)) => Value::Usize(__arith(arith, l, r)), + (Value::U8(l), Value::U8(r)) => __arith(arith, l, r).map(Value::U8), + (Value::U16(l), Value::U16(r)) => __arith(arith, l, r).map(Value::U16), + (Value::U32(l), Value::U32(r)) => __arith(arith, l, r).map(Value::U32), + (Value::U64(l), Value::U64(r)) => __arith(arith, l, r).map(Value::U64), + (Value::Usize(l), Value::Usize(r)) => __arith(arith, l, r).map(Value::Usize), (Value::Numeric(_l), Value::Numeric(_r)) => { panic!( "raw arithmetic on numerics should be done in numeric model, or with Expr-level casts beforehand" @@ -712,18 +702,18 @@ impl Value { } } - pub fn unary(op: UnaryOp, value: Value) -> Value { + pub fn unary(op: UnaryOp, value: Value) -> Result { match op { UnaryOp::BoolNot => match value { - Value::Bool(b) => Value::Bool(!b), + Value::Bool(b) => Ok(Value::Bool(!b)), _ => panic!("cannot apply bool-not to non-boolean operand (`{value:?}`)"), }, op => match value { - Value::U8(i) => Value::U8(__unary(op, i)), - Value::U16(i) => Value::U16(__unary(op, i)), - Value::U32(i) => Value::U32(__unary(op, i)), - Value::U64(i) => Value::U64(__unary(op, i)), - Value::Usize(i) => Value::Usize(__unary(op, i)), + Value::U8(i) => Ok(Value::U8(__unary(op, i)?)), + Value::U16(i) => Ok(Value::U16(__unary(op, i)?)), + Value::U32(i) => Ok(Value::U32(__unary(op, i)?)), + Value::U64(i) => Ok(Value::U64(__unary(op, i)?)), + Value::Usize(i) => Ok(Value::Usize(__unary(op, i)?)), Value::Numeric(_i) => { panic!("top-level unary operations should not be performed on raw-numeric"); } @@ -761,17 +751,21 @@ mod tests { Value::Numeric(Rc::new(TypedConst::new_auto(n))) } + fn int_rel_ok(rel: IntRel, left: Value, right: Value) -> Value { + Value::int_rel(rel, left, right).unwrap() + } + /// Checks that `Value::int_rel` agrees, for every `IntRel` variant and both operand /// orderings, with the natural `i128` comparison of `l` and `r`. fn check_all_rels(left: &Value, right: &Value, l: i128, r: i128) { for &rel in &ALL_RELS { assert_eq!( - Value::int_rel(rel, left.clone(), right.clone()), + int_rel_ok(rel, left.clone(), right.clone()), Value::Bool(__rel(rel, l, r)), "int_rel({rel:?}, {left:?}, {right:?})" ); assert_eq!( - Value::int_rel(rel, right.clone(), left.clone()), + int_rel_ok(rel, right.clone(), left.clone()), Value::Bool(__rel(rel, r, l)), "int_rel({rel:?}, {right:?}, {left:?})" ); @@ -892,28 +886,28 @@ mod tests { let large = Value::U8(10); assert_eq!( - Value::int_rel(IntRel::Lt, small.clone(), large.clone()), + int_rel_ok(IntRel::Lt, small.clone(), large.clone()), Value::Bool(true) ); assert_eq!( - Value::int_rel(IntRel::Gt, small.clone(), large.clone()), + int_rel_ok(IntRel::Gt, small.clone(), large.clone()), Value::Bool(false) ); assert_eq!( - Value::int_rel(IntRel::Lte, small.clone(), large.clone()), + int_rel_ok(IntRel::Lte, small.clone(), large.clone()), Value::Bool(true) ); assert_eq!( - Value::int_rel(IntRel::Gte, small.clone(), large.clone()), + int_rel_ok(IntRel::Gte, small.clone(), large.clone()), Value::Bool(false) ); assert_eq!( - Value::int_rel(IntRel::Lt, large.clone(), small.clone()), + int_rel_ok(IntRel::Lt, large.clone(), small.clone()), Value::Bool(false) ); assert_eq!( - Value::int_rel(IntRel::Gt, large.clone(), small.clone()), + int_rel_ok(IntRel::Gt, large.clone(), small.clone()), Value::Bool(true) ); } @@ -921,34 +915,40 @@ mod tests { // ---- error cases ---- #[test] - #[should_panic(expected = "cannot apply int-rel")] - fn numeric_with_mismatched_concrete_rep_panics() { + fn numeric_with_mismatched_concrete_rep_errs() { let left = numeric(5u8, NumRep::Concrete(MachineRep::U16)); let right = Value::U8(5); - Value::int_rel(IntRel::Eq, left, right); + assert!(matches!( + Value::int_rel(IntRel::Eq, left, right), + Err(EvalError::NumericConvert(_)) + )); } #[test] - #[should_panic(expected = "cannot apply int-rel")] - fn numeric_with_unrepresentable_concrete_rep_panics() { + fn numeric_with_unrepresentable_concrete_rep_errs() { // 300 does not fit in u8, despite being tagged with `NumRep::U8` let left = numeric(300i32, NumRep::Concrete(MachineRep::U8)); let right = Value::U8(5); - Value::int_rel(IntRel::Eq, left, right); + assert!(matches!( + Value::int_rel(IntRel::Eq, left, right), + Err(EvalError::NumericConvert(_)) + )); } #[test] - #[should_panic(expected = "cannot apply int-rel")] - fn numeric_auto_negative_vs_u8_panics() { + fn numeric_auto_negative_vs_u8_errs() { // -1 cannot be converted to u8, even under `NumRep::Auto` let left = numeric_auto(-1i32); let right = Value::U8(5); - Value::int_rel(IntRel::Eq, left, right); + assert!(matches!( + Value::int_rel(IntRel::Eq, left, right), + Err(EvalError::NumericConvert(_)) + )); } #[test] #[should_panic(expected = "cannot apply int-rel")] fn fully_unsupported_combination_panics() { - Value::int_rel(IntRel::Eq, Value::Bool(true), Value::U8(1)); + int_rel_ok(IntRel::Eq, Value::Bool(true), Value::U8(1)); } } diff --git a/src/error.rs b/src/error.rs index b1117cf8..7493f4e0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,7 @@ use crate::byte_set::ByteSet; -use crate::decoder::{Scope, ScopeEntry, Value}; +use crate::decoder::{ArithError, Scope, ScopeEntry, Value}; use crate::loc_decoder::{LocScope, ParsedValue}; +use crate::numeric::core::EvalError as NumExprError; use crate::read::{BufferKind, ReadCtxt}; use crate::{Expr, Label, Pattern}; @@ -17,6 +18,92 @@ impl std::fmt::Display for UnknownVarError { impl std::error::Error for UnknownVarError {} +/// Error produced while evaluating an [`Expr`] to a [`Value`] (see `Expr::eval`). +/// +/// Only covers failures that can be caused by the data being decoded; violations of invariants +/// that the type-checker is responsible for (e.g. applying `AsChar` to a `Seq`) remain panics. +#[derive(Debug)] +pub enum EvalError { + /// Checked integer arithmetic underflowed, overflowed, or divided by zero. + Arith(ArithError), + /// Evaluation of an embedded [`NumExpr`](crate::numeric::core::NumExpr) failed. + Numeric(NumExprError), + /// A fixed-width integer conversion (e.g. a narrowing `AsU8`) was out of range. + IntCast(std::num::TryFromIntError), + /// A [`TypedConst`](crate::numeric::core::TypedConst) could not be converted to a native integer type. + NumericConvert(anyhow::Error), +} + +impl std::fmt::Display for EvalError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Arith(err) => err.fmt(f), + Self::Numeric(err) => write!(f, "numeric expression evaluation failed: {err}"), + Self::IntCast(err) => write!(f, "integer conversion failed: {err}"), + Self::NumericConvert(err) => write!(f, "numeric conversion failed: {err}"), + } + } +} + +impl std::error::Error for EvalError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Arith(err) => Some(err), + Self::Numeric(err) => Some(err), + Self::IntCast(err) => Some(err), + Self::NumericConvert(err) => Some(err.as_ref()), + } + } +} + +/// Extension for attaching decode-context to an [`EvalError`] as it crosses into the decoder, +/// so that an otherwise narrow failure (e.g. a bare `TryFromIntError`) records which `Decoder` +/// and which `Expr` it originated from. +pub(crate) trait EvalResultExt { + /// Converts the error into a [`DecodeError`], pushing the value produced by `ctx` onto its trace. + /// + /// `ctx` is only evaluated if `self` is an `Err`. + fn trace_eval(self, ctx: impl FnOnce() -> C) -> Result> + where + V: Clone + std::fmt::Debug, + C: std::fmt::Debug + Send + Sync + 'static; +} + +impl EvalResultExt for Result { + fn trace_eval(self, ctx: impl FnOnce() -> C) -> Result> + where + V: Clone + std::fmt::Debug, + C: std::fmt::Debug + Send + Sync + 'static, + { + use crate::util::ErrTrace as _; + self.map_err(|err| DecodeError::::from(err).with_trace(ctx())) + } +} + +impl From for EvalError { + fn from(err: ArithError) -> Self { + Self::Arith(err) + } +} + +impl From for EvalError { + fn from(err: NumExprError) -> Self { + Self::Numeric(err) + } +} + +impl From for EvalError { + fn from(err: std::num::TryFromIntError) -> Self { + Self::IntCast(err) + } +} + +impl From for EvalError { + fn from(err: anyhow::Error) -> Self { + Self::NumericConvert(err) + } +} + #[derive(Debug)] pub struct DecodeError { pub err: Box>, @@ -188,6 +275,8 @@ pub enum DecodeErrorKind { /// The value that failed to match any of the provided patterns value: Box, }, + /// Evaluation of an [`Expr`] failed (see [`EvalError`]). + Eval(EvalError), } impl BufferLimitError { @@ -269,6 +358,7 @@ impl std::fmt::Display for DecodeErrorKind { "value `{value:?}` failed to match any of the provided patterns: {cases:?}" ) } + Self::Eval(err) => err.fmt(f), } } } @@ -277,11 +367,24 @@ impl std::error::Error for DecodeErrorKind { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { Self::BufferLimit(err) => Some(err), + Self::Eval(err) => Some(err), _ => None, } } } +impl From for DecodeErrorKind { + fn from(err: EvalError) -> Self { + Self::Eval(err) + } +} + +impl From for DecodeError { + fn from(err: EvalError) -> Self { + DecodeErrorKind::from(err).into() + } +} + impl From for DecodeErrorKind { fn from(err: BufferLimitError) -> Self { Self::BufferLimit(err) diff --git a/src/loc_decoder.rs b/src/loc_decoder.rs index 5bdf1c5b..1c8f418f 100644 --- a/src/loc_decoder.rs +++ b/src/loc_decoder.rs @@ -13,11 +13,11 @@ use crate::decoder::{ search::{find_index_by_key_sorted, find_index_by_key_unsorted}, seq_kind::sub_range, }; -use crate::error::{DecodeError, DecodeErrorKind, UnknownVarError}; +use crate::error::{DecodeError, DecodeErrorKind, EvalError, EvalResultExt as _, UnknownVarError}; use crate::read::{BufferKind, ReadCtxt}; use crate::try_with; use crate::util::ErrTrace as _; -use crate::util::downgrade_error_with; +use crate::util::try_downgrade_error_with; use crate::util::{EResult, WithErr}; use crate::validation::Condition; use crate::{BaseKind, DynFormat, Endian, Expr, Format, Label, Pattern, ViewExpr}; @@ -591,8 +591,11 @@ impl ParsedValue { } impl Expr { - pub fn eval_with_loc<'a>(&'a self, scope: &'a LocScope<'a>) -> Cow<'a, ParsedValue> { - match self { + pub fn eval_with_loc<'a>( + &'a self, + scope: &'a LocScope<'a>, + ) -> Result, EvalError> { + Ok(match self { Expr::Var(name) => Cow::Borrowed(scope.get_value_by_name(name).unwrap()), Expr::Bool(b) => Cow::Owned(ParsedValue::from_evaluated(Value::Bool(*b))), Expr::U8(i) => Cow::Owned(ParsedValue::from_evaluated(Value::U8(*i))), @@ -600,129 +603,117 @@ impl Expr { Expr::U32(i) => Cow::Owned(ParsedValue::from_evaluated(Value::U32(*i))), Expr::U64(i) => Cow::Owned(ParsedValue::from_evaluated(Value::U64(*i))), Expr::Numeric(n) => { - let num_val = n.eval(scope); - match num_val { - Ok(v) => Cow::Owned(ParsedValue::from_evaluated(Value::from(v))), - Err(e) => panic!( - "Expr::eval_with_loc(Numeric({n:?})) failed during NumExpr evaluation: {e}" - ), - } + Cow::Owned(ParsedValue::from_evaluated(Value::from(n.eval(scope)?))) } Expr::Tuple(exprs) => Cow::Owned(ParsedValue::from_evaluated(Value::Tuple( exprs .iter() .map(|expr| expr.eval_value_with_loc(scope)) - .collect(), + .collect::>()?, ))), - Expr::TupleProj(head, index) => cow_map(head.eval_with_loc(scope), |v| { + Expr::TupleProj(head, index) => cow_map(head.eval_with_loc(scope)?, |v| { v.coerce_mapped_value().tuple_proj(*index) }), Expr::Record(fields) => Cow::Owned(ParsedValue::collect_fields( fields .iter() - .map(|(label, expr)| (label.clone(), expr.eval_with_loc(scope).into_owned())) - .collect(), + .map(|(label, expr)| { + Ok((label.clone(), expr.eval_with_loc(scope)?.into_owned())) + }) + .collect::>()?, )), - Expr::RecordProj(head, label) => cow_map(head.eval_with_loc(scope), |v| { + Expr::RecordProj(head, label) => cow_map(head.eval_with_loc(scope)?, |v| { v.coerce_mapped_value().record_proj(label.as_ref()) }), Expr::Variant(label, expr) => Cow::Owned(ParsedValue::from_evaluated(Value::variant( label.clone(), - expr.eval_value_with_loc(scope), + expr.eval_value_with_loc(scope)?, ))), Expr::Seq(exprs) => Cow::Owned(ParsedValue::from_evaluated(Value::Seq( exprs .iter() .map(|expr| expr.eval_value_with_loc(scope)) - .collect(), + .collect::>()?, ))), Expr::Match(head, branches) => { - let head = head.eval_with_loc(scope); + let head = head.eval_with_loc(scope)?; for (pattern, expr) in branches { if let Some(pattern_scope) = head.matches(scope, pattern) { - let value = expr.eval_value_with_loc(&LocScope::Multi(&pattern_scope)); - return Cow::Owned(ParsedValue::from_evaluated(value)); + let value = expr.eval_value_with_loc(&LocScope::Multi(&pattern_scope))?; + return Ok(Cow::Owned(ParsedValue::from_evaluated(value))); } } panic!("non-exhaustive patterns"); } Expr::Destructure(head, pattern, expr) => { - let head = head.eval_with_loc(scope); + let head = head.eval_with_loc(scope)?; if let Some(pattern_scope) = head.matches(scope, pattern) { - let value = expr.eval_value_with_loc(&LocScope::Multi(&pattern_scope)); - return Cow::Owned(ParsedValue::from_evaluated(value)); + let value = expr.eval_value_with_loc(&LocScope::Multi(&pattern_scope))?; + return Ok(Cow::Owned(ParsedValue::from_evaluated(value))); } panic!("refuted pattern: {head:?} does not match {pattern:?}") } Expr::Lambda(_, _) => panic!("cannot eval lambda"), Expr::IntRel(rel, x, y) => Cow::Owned(ParsedValue::from_evaluated({ - let left = x.eval_value_with_loc(scope); - let right = y.eval_value_with_loc(scope); - Value::int_rel(*rel, left, right) + let left = x.eval_value_with_loc(scope)?; + let right = y.eval_value_with_loc(scope)?; + Value::int_rel(*rel, left, right)? })), Expr::Arith(op, x, y) => Cow::Owned(ParsedValue::from_evaluated({ - let left = x.eval_value_with_loc(scope); - let right = y.eval_value_with_loc(scope); - Value::arith(*op, left, right) + let left = x.eval_value_with_loc(scope)?; + let right = y.eval_value_with_loc(scope)?; + Value::arith(*op, left, right)? })), Expr::Unary(op, x) => Cow::Owned(ParsedValue::from_evaluated({ - let value = x.eval_value_with_loc(scope); - Value::unary(*op, value) + let value = x.eval_value_with_loc(scope)?; + Value::unary(*op, value)? })), // FIXME - extract common logic for As-expr on Value instead of separate impl for decoder and loc_decoder Expr::AsU8(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope) { + match x.eval_value_with_loc(scope)? { Value::U8(x) => Value::U8(x), - Value::U16(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u16 {x}: {err}") - })), - Value::U32(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u32 {x}: {err}") - })), - Value::U64(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on u64 {x}: {err}") - })), - Value::Usize(x) => Value::U8(u8::try_from(x).unwrap_or_else(|err| { - panic!("cannot perform AsU8 cast on usize {x}: {err}") - })), + Value::U16(x) => Value::U8(u8::try_from(x)?), + Value::U32(x) => Value::U8(u8::try_from(x)?), + Value::U64(x) => Value::U8(u8::try_from(x)?), + Value::Usize(x) => Value::U8(u8::try_from(x)?), x => panic!("cannot convert {x:?} to U8"), }, )), Expr::AsU16(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope) { + match x.eval_value_with_loc(scope)? { Value::U8(x) => Value::U16(u16::from(x)), Value::U16(x) => Value::U16(x), - Value::U32(x) => Value::U16(u16::try_from(x).unwrap()), - Value::U64(x) => Value::U16(u16::try_from(x).unwrap()), - Value::Usize(x) => Value::U16(u16::try_from(x).unwrap()), + Value::U32(x) => Value::U16(u16::try_from(x)?), + Value::U64(x) => Value::U16(u16::try_from(x)?), + Value::Usize(x) => Value::U16(u16::try_from(x)?), x => panic!("cannot convert {x:?} to U16"), }, )), Expr::AsU32(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope) { + match x.eval_value_with_loc(scope)? { Value::U8(x) => Value::U32(u32::from(x)), Value::U16(x) => Value::U32(u32::from(x)), Value::U32(x) => Value::U32(x), - Value::U64(x) => Value::U32(u32::try_from(x).unwrap()), - Value::Usize(x) => Value::U32(u32::try_from(x).unwrap()), + Value::U64(x) => Value::U32(u32::try_from(x)?), + Value::Usize(x) => Value::U32(u32::try_from(x)?), x => panic!("cannot convert {x:?} to U32"), }, )), Expr::AsU64(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope) { + match x.eval_value_with_loc(scope)? { Value::U8(x) => Value::U64(u64::from(x)), Value::U16(x) => Value::U64(u64::from(x)), Value::U32(x) => Value::U64(u64::from(x)), Value::U64(x) => Value::U64(x), - Value::Usize(x) => Value::U64(u64::try_from(x).unwrap()), + Value::Usize(x) => Value::U64(u64::try_from(x)?), x => panic!("cannot convert {x:?} to U64"), }, )), Expr::U16Be(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [Value::U8(hi), Value::U8(lo)] => Cow::Owned(ParsedValue::from_evaluated( Value::U16(u16::from_be_bytes([*hi, *lo])), )), @@ -730,7 +721,7 @@ impl Expr { } } Expr::U16Le(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [Value::U8(lo), Value::U8(hi)] => Cow::Owned(ParsedValue::from_evaluated( Value::U16(u16::from_le_bytes([*lo, *hi])), )), @@ -738,7 +729,7 @@ impl Expr { } } Expr::U32Be(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_be_bytes( [*a, *b, *c, *d], @@ -748,7 +739,7 @@ impl Expr { } } Expr::U32Le(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_le_bytes( [*a, *b, *c, *d], @@ -758,7 +749,7 @@ impl Expr { } } Expr::U64Be(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [ Value::U8(a), Value::U8(b), @@ -775,7 +766,7 @@ impl Expr { } } Expr::U64Le(bytes) => { - match bytes.eval_value_with_loc(scope).unwrap_tuple().as_slice() { + match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { [ Value::U8(a), Value::U8(b), @@ -792,7 +783,7 @@ impl Expr { } } Expr::AsChar(bytes) => Cow::Owned(ParsedValue::from_evaluated( - match bytes.eval_value_with_loc(scope) { + match bytes.eval_value_with_loc(scope)? { Value::U8(x) => Value::Char(char::from(x)), Value::U16(x) => { Value::Char(char::from_u32(x as u32).unwrap_or(char::REPLACEMENT_CHARACTER)) @@ -801,14 +792,13 @@ impl Expr { Value::Char(char::from_u32(x).unwrap_or(char::REPLACEMENT_CHARACTER)) } Value::U64(x) => Value::Char( - char::from_u32(u32::try_from(x).unwrap()) - .unwrap_or(char::REPLACEMENT_CHARACTER), + char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), ), _ => panic!("AsChar: expected U8, U16, U32, or U64"), }, )), Expr::SeqLength(seq) => match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { @@ -818,31 +808,31 @@ impl Expr { } _ => panic!("SeqLength: expected Seq (or EnumFromTo)"), }, - Expr::SeqIx(seq, index) => cow_remap(seq.eval_with_loc(scope), |v| { - match v.coerce_mapped_value().get_sequence() { - Some(values) => { - let index = index.eval_value_with_loc(scope).unwrap_usize(); - match values { + Expr::SeqIx(seq, index) => { + let index = index.eval_value_with_loc(scope)?.try_as_usize()?; + cow_remap(seq.eval_with_loc(scope)?, |v| { + match v.coerce_mapped_value().get_sequence() { + Some(values) => match values { ValueSeq::ValueSeq(values) => Cow::Borrowed(&values[index]), ValueSeq::IntRange(mut range) => { Cow::Owned(ParsedValue::from_evaluated(Value::Usize( range.nth(index).unwrap(), ))) } - } + }, + _ => panic!("SeqIx: expected Seq (or EnumFromTo)"), } - _ => panic!("SeqIx: expected Seq (or EnumFromTo)"), - } - }), + }) + } Expr::SubSeq(seq, start, length) => { match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(values) => { - let start = start.eval_value_with_loc(scope).unwrap_usize(); - let length = length.eval_value_with_loc(scope).unwrap_usize(); + let start = start.eval_value_with_loc(scope)?.try_as_usize()?; + let length = length.eval_value_with_loc(scope)?.try_as_usize()?; match values { ValueSeq::ValueSeq(values) => Cow::Owned( ParsedValue::from_evaluated_seq(values.sub_seq(start, length)), @@ -857,13 +847,13 @@ impl Expr { } Expr::SubSeqInflate(seq, start, length) => { match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(values) => { - let start = start.eval_value_with_loc(scope).unwrap_usize(); - let length = length.eval_value_with_loc(scope).unwrap_usize(); + let start = start.eval_value_with_loc(scope)?.try_as_usize()?; + let length = length.eval_value_with_loc(scope)?.try_as_usize()?; let mut vs = Vec::new(); match values { ValueSeq::ValueSeq(vs0) => { @@ -894,18 +884,18 @@ impl Expr { } } Expr::Append(seq0, seq1) => { - let tmp0 = seq0.eval_with_loc(scope); + let tmp0 = seq0.eval_with_loc(scope)?; let val0 = tmp0.coerce_mapped_value(); match val0.get_sequence() { Some(val_seq0) => { - let tmp1 = seq1.eval_with_loc(scope); + let tmp1 = seq1.eval_with_loc(scope)?; let val1 = tmp1.coerce_mapped_value(); match val1.get_sequence() { Some(val_seq1) => { if val_seq0.is_empty() { - return Cow::Owned(val1.clone()); + return Ok(Cow::Owned(val1.clone())); } else if val_seq1.is_empty() { - return Cow::Owned(val0.clone()); + return Ok(Cow::Owned(val0.clone())); } Cow::Owned(ParsedValue::Seq(Parsed { loc: ParseLoc::Synthesized, @@ -920,14 +910,14 @@ impl Expr { } Expr::FlatMap(expr, seq) => { match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(values) => { let mut vs: Vec = Vec::new(); for v in values { - if let Value::Seq(vn) = expr.eval_lambda_with_loc(scope, &v) { + if let Value::Seq(vn) = expr.eval_lambda_with_loc(scope, &v)? { vs.extend(vn); } else { panic!("FlatMap: expected Seq"); @@ -939,12 +929,12 @@ impl Expr { } } Expr::FlatMapAccum(expr, accum, _accum_type, seq) => match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(values) => { - let mut accum = accum.eval_value_with_loc(scope); + let mut accum = accum.eval_value_with_loc(scope)?; let mut vs = Vec::new(); for v in values { let ret = expr.eval_lambda_with_loc( @@ -953,7 +943,7 @@ impl Expr { accum, v.clone_into_value(), ])), - ); + )?; accum = match extract_pair(ret.unwrap_tuple()) { (accum, Value::Seq(vn)) => { vs.extend(vn); @@ -967,12 +957,12 @@ impl Expr { None => panic!("FlatMapAccum: expected Seq"), }, Expr::LeftFold(expr, accum, _accum_type, seq) => match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(values) => { - let mut accum = accum.eval_value_with_loc(scope); + let mut accum = accum.eval_value_with_loc(scope)?; for v in values { let new_accum = expr.eval_lambda_with_loc( scope, @@ -980,7 +970,7 @@ impl Expr { accum, v.clone_into_value(), ])), - ); + )?; accum = new_accum; } Cow::Owned(ParsedValue::from_evaluated(accum)) @@ -989,32 +979,37 @@ impl Expr { }, Expr::FindByKey(is_sorted, f_get_key, query_key, seq) => { match seq - .eval_with_loc(scope) + .eval_with_loc(scope)? .coerce_mapped_value() .get_sequence() { Some(ValueSeq::ValueSeq(values)) => { - let eval = - |lambda: &Expr, v: &ParsedValue| lambda.eval_lambda_with_loc(scope, v); - let query = query_key.eval_value_with_loc(scope); - if *is_sorted { - match find_index_by_key_sorted(f_get_key, &query, values, eval) { - Some(ix) => Cow::Owned(ParsedValue::Option(Some(Box::new( - values[ix].clone(), - )))), - None => { - Cow::Owned(ParsedValue::from_evaluated(Value::Option(None))) - } + // `search` is shared with `decoder` and expects an infallible evaluator, so the first + // error is stashed here (the sentinel `query` key makes the search terminate immediately) + let query = query_key.eval_value_with_loc(scope)?; + let eval_err = std::cell::Cell::new(None); + let eval = |lambda: &Expr, v: &ParsedValue| match lambda + .eval_lambda_with_loc(scope, v) + { + Ok(key) => key, + Err(err) => { + eval_err.set(Some(err)); + query.clone() } + }; + let found = if *is_sorted { + find_index_by_key_sorted(f_get_key, &query, values, eval) } else { - match find_index_by_key_unsorted(f_get_key, &query, values, eval) { - Some(ix) => Cow::Owned(ParsedValue::Option(Some(Box::new( - values[ix].clone(), - )))), - None => { - Cow::Owned(ParsedValue::from_evaluated(Value::Option(None))) - } + find_index_by_key_unsorted(f_get_key, &query, values, eval) + }; + if let Some(err) = eval_err.take() { + return Err(err); + } + match found { + Some(ix) => { + Cow::Owned(ParsedValue::Option(Some(Box::new(values[ix].clone())))) } + None => Cow::Owned(ParsedValue::from_evaluated(Value::Option(None))), } } Some(ValueSeq::IntRange(_)) => { @@ -1023,15 +1018,16 @@ impl Expr { None => panic!("FindByKey: expected Seq"), } } - Expr::FlatMapList(expr, _ret_type, seq) => match seq.eval_value_with_loc(scope) { + Expr::FlatMapList(expr, _ret_type, seq) => match seq.eval_value_with_loc(scope)? { Value::Seq(values) => { let mut vs = Vec::new(); for v in values { let arg = Value::Tuple(vec![Value::Seq(vs.into()), v]); // TODO can we avoid cloning arg here? - if let Value::Seq(vn) = expr - .eval_lambda_with_loc(scope, &ParsedValue::from_evaluated(arg.clone())) - { + if let Value::Seq(vn) = expr.eval_lambda_with_loc( + scope, + &ParsedValue::from_evaluated(arg.clone()), + )? { vs = match arg { Value::Tuple(mut args) => match args.remove(0) { Value::Seq(vs) => vs.into_vec(), @@ -1049,32 +1045,38 @@ impl Expr { _ => panic!("FlatMapList: expected Seq"), }, Expr::Dup(count, expr) => { - let count = count.eval_value_with_loc(scope).unwrap_usize(); - let v = expr.eval_value_with_loc(scope); + let count = count.eval_value_with_loc(scope)?.try_as_usize()?; + let v = expr.eval_value_with_loc(scope)?; Cow::Owned(ParsedValue::from_evaluated(Value::Seq(SeqKind::Dup( count, Box::new(v), )))) } Expr::EnumFromTo(start, stop) => { - let start = start.eval_value_with_loc(scope).unwrap_usize(); - let stop = stop.eval_value_with_loc(scope).unwrap_usize(); + let start = start.eval_value_with_loc(scope)?.try_as_usize()?; + let stop = stop.eval_value_with_loc(scope)?.try_as_usize()?; Cow::Owned(ParsedValue::from_evaluated(Value::EnumFromTo(start..stop))) } Expr::LiftOption(opt) => Cow::Owned(ParsedValue::from_evaluated(Value::Option( opt.as_ref() - .map(|expr| Box::new(expr.eval_value_with_loc(scope))), + .map(|expr| expr.eval_value_with_loc(scope).map(Box::new)) + .transpose()?, ))), - } + }) } - pub fn eval_value_with_loc<'a>(&self, scope: &'a LocScope<'a>) -> Value { - self.eval_with_loc(scope) + pub fn eval_value_with_loc<'a>(&self, scope: &'a LocScope<'a>) -> Result { + Ok(self + .eval_with_loc(scope)? .coerce_mapped_value() - .clone_into_value() + .clone_into_value()) } - fn eval_lambda_with_loc<'a>(&self, scope: &'a LocScope<'a>, arg: &ParsedValue) -> Value { + fn eval_lambda_with_loc<'a>( + &self, + scope: &'a LocScope<'a>, + arg: &ParsedValue, + ) -> Result { match self { Expr::Lambda(name, expr) => { let child_scope = LocSingleScope::new(scope, name, arg); @@ -1345,7 +1347,7 @@ impl Decoder { Decoder::Call(n, es, vs) => { let mut new_scope = LocMultiScope::with_capacity(&LocScope::Empty, es.len()); for (name, e) in es { - let v = e.eval_with_loc(scope).as_ref().clone(); + let v = e.eval_with_loc(scope).trace_eval(|| ("Call(e)", format!("{e:?}")))?.as_ref().clone(); new_scope.push(name.clone(), v); } for (name, vv) in vs { @@ -1501,7 +1503,7 @@ impl Decoder { } Decoder::DecodeBytes(bytes, a) => { let bytes = { - let raw = bytes.eval_value_with_loc(scope); + let raw = bytes.eval_value_with_loc(scope).trace_eval(|| ("DecodeBytes(bytes)", format!("{bytes:?}")))?; let seq_vals = raw.get_sequence().expect("bad type for DecodeBytes input"); seq_vals .into_iter() @@ -1538,7 +1540,7 @@ impl Decoder { }) } Decoder::ForEach(expr, lbl, a) => { - let val = expr.eval_with_loc(scope); + let val = expr.eval_with_loc(scope).trace_eval(|| ("ForEach(expr)", format!("{expr:?}")))?; let seq = val.get_sequence().expect("bad type for ForEach input"); Ok(WithErr::fold( (Vec::with_capacity(seq.len()), input), @@ -1558,7 +1560,7 @@ impl Decoder { })) } Decoder::RepeatCount(expr, a) => { - let count = expr.eval_value_with_loc(scope).unwrap_usize(); + let count = expr.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("RepeatCount(expr)", format!("{expr:?}")))?; Ok(WithErr::fold( (Vec::with_capacity(count), input), 0..count, @@ -1577,8 +1579,8 @@ impl Decoder { })) } Decoder::RepeatBetween(reps_left_tree, min, max, a) => { - let min = min.eval_value_with_loc(scope).unwrap_usize(); - let max = max.eval_value_with_loc(scope).unwrap_usize(); + let min = min.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("RepeatBetween(min)", format!("{min:?}")))?; + let max = max.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("RepeatBetween(max)", format!("{max:?}")))?; let mut res = WithErr::new((Vec::new(), input)); loop { let v = &res.as_ref().0; @@ -1611,7 +1613,7 @@ impl Decoder { })) } Decoder::Maybe(expr, a) => { - let is_present = expr.eval_value_with_loc(scope).unwrap_bool(); + let is_present = expr.eval_value_with_loc(scope).trace_eval(|| ("Maybe(expr)", format!("{expr:?}")))?.unwrap_bool(); if is_present { Ok(try_with!(a.parse_with_loc(program, scope, input) => "Maybe") .map(|(val, input)| (ParsedValue::Option(Some(Box::new(val))), input)) @@ -1625,12 +1627,12 @@ impl Decoder { let mut res = WithErr::new((Vec::new(), input, false)); loop { res = res.join(|(mut v, input, _done)| { - Ok(try_with!(a.parse_with_loc(program, scope, input) => ("RepeatUntilLast", v.len())) - .map(move |(va, next_input)| { - let done = expr.eval_lambda_with_loc(scope, &va).unwrap_bool(); + try_with!(a.parse_with_loc(program, scope, input) => ("RepeatUntilLast", v.len())) + .join(move |(va, next_input)| { + let done = expr.eval_lambda_with_loc(scope, &va).trace_eval(|| ("RepeatUntilLast(expr)", format!("{expr:?}")))?.unwrap_bool(); v.push(va); - (v, next_input, done) - })) + Ok(WithErr::new((v, next_input, done))) + }) })?; let done = res.as_ref().2; if done { @@ -1647,18 +1649,17 @@ impl Decoder { let mut res = WithErr::new((Vec::new(), input, false)); loop { res = res.join(|(mut v, input, _done)| { - Ok(try_with!(a.parse_with_loc(program, scope, input) => ("RepeatUntilSeq", format!("len={}", v.len()), format!("{a:?}"))) - .map(|(va, next_input)| { + try_with!(a.parse_with_loc(program, scope, input) => ("RepeatUntilSeq", format!("len={}", v.len()), format!("{a:?}"))) + .join(|(va, next_input)| { v.push(va); let vs = ParsedValue::from_evaluated_seq(v); - let done = expr.eval_lambda_with_loc(scope, &vs).unwrap_bool(); + let done = expr.eval_lambda_with_loc(scope, &vs).trace_eval(|| ("RepeatUntilSeq(expr)", format!("{expr:?}")))?.unwrap_bool(); v = match vs { ParsedValue::Seq(v) => v.inner.into_vec(), _ => unreachable!(), }; - (v, next_input, done) - }), - ) + Ok(WithErr::new((v, next_input, done))) + }) })?; break_if_done!(res => (v, input)); } @@ -1667,7 +1668,7 @@ impl Decoder { .map(|(v, input, _)| (ParsedValue::new_seq(v, start_offset, total_len), input))) } Decoder::AccumUntil(f_done, f_update, init, _vt, a) => { - let accum = init.eval_value_with_loc(scope); + let accum = init.eval_value_with_loc(scope).trace_eval(|| ("AccumUntil(init)", format!("{init:?}")))?; let mut res = WithErr::new((Vec::new(), accum, input, false)); loop { res = res.join(|(mut v, accum, input, _done)| { @@ -1680,22 +1681,22 @@ impl Decoder { ) .clone_into_value(), ])); - let is_done = f_done.eval_lambda_with_loc(scope, &done_arg).unwrap_bool(); + let is_done = f_done.eval_lambda_with_loc(scope, &done_arg).trace_eval(|| ("AccumUntil(f_done)", format!("{f_done:?}")))?.unwrap_bool(); if is_done { return Ok(WithErr::new((v, accum, input, true))); } - Ok(try_with!(a.parse_with_loc(program, scope, input) => ("AccumUntil", v.len())) - .map( + try_with!(a.parse_with_loc(program, scope, input) => ("AccumUntil", v.len())) + .join( |(next_elem, next_input)| { v.push(next_elem.clone()); let update_arg = ParsedValue::from_evaluated(Value::Tuple(vec![ accum.clone(), next_elem.clone_into_value(), ])); - let next_accum = f_update.eval_lambda_with_loc(scope, &update_arg); - (v, next_accum, next_input, false) + let next_accum = f_update.eval_lambda_with_loc(scope, &update_arg).trace_eval(|| ("AccumUntil(f_update)", format!("{f_update:?}")))?; + Ok(WithErr::new((v, next_accum, next_input, false))) }, - )) + ) })?; break_if_done!(res => (v, accum, input)); } @@ -1726,7 +1727,7 @@ impl Decoder { } } Decoder::Slice(expr, a) => { - let size = expr.eval_value_with_loc(scope).unwrap_usize(); + let size = expr.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("Slice(expr)", format!("{expr:?}")))?; let (slice, input) = input .split_at(size) .ok_or(input.kind.overrun(size, input.offset).with_trace(("Slice(create)", format!("{:?}->{size}", expr))))?; @@ -1752,8 +1753,8 @@ impl Decoder { }) } Decoder::WithRelativeOffset(base_addr, expr, a) => { - let base_addr = base_addr.eval_value_with_loc(scope).unwrap_usize(); - let offset = expr.eval_value_with_loc(scope).unwrap_usize(); + let base_addr = base_addr.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("WithRelativeOffset(base_addr)", format!("{base_addr:?}")))?; + let offset = expr.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("WithRelativeOffset(expr)", format!("{expr:?}")))?; let abs_offset = base_addr + offset; let seek_input = input .seek_to(abs_offset) @@ -1761,20 +1762,19 @@ impl Decoder { Ok(try_with!(a.parse_with_loc(program, scope, seek_input) => ("WithRelativeOffset(parse)", format!("{a:?}"))) .map(|(v, _)| (v, input))) } - Decoder::Map(d, expr) => Ok( + Decoder::Map(d, expr) => { try_with!(d.parse_with_loc(program, scope, input) => ("Map(parse)", format!("{d:?}"))) - .map(|(orig, input)| { - let v = expr.eval_lambda_with_loc(scope, &orig); + .join(|(orig, input)| { + let v = expr.eval_lambda_with_loc(scope, &orig).trace_eval(|| ("Map(expr)", format!("{expr:?}")))?; let image = ParsedValue::inherit(&orig, v); - (ParsedValue::Mapped(Box::new(orig), Box::new(image)), input) - }, - ), - ), + Ok(WithErr::new((ParsedValue::Mapped(Box::new(orig), Box::new(image)), input))) + }) + } Decoder::Where(d, cond) => { try_with!(d.parse_with_loc(program, scope, input) => ("Where(parse)", format!("{d:?}"))) .join(|(v, input)| { let Condition { expr, severity } = cond; - match expr.eval_lambda_with_loc(scope, &v).unwrap_bool() { + match expr.eval_lambda_with_loc(scope, &v).trace_eval(|| ("Where(expr)", format!("{expr:?}")))?.unwrap_bool() { true => Ok(WithErr::new((v, input))), false => { let err = DecodeErrorKind::loc_bad_where( @@ -1792,11 +1792,11 @@ impl Decoder { }) } Decoder::Compute(expr) => { - let v = expr.eval_with_loc(scope); + let v = expr.eval_with_loc(scope).trace_eval(|| ("Compute(expr)", format!("{expr:?}")))?; Ok(WithErr::new((v.as_ref().clone(), input))) } Decoder::Let(name, expr, d) => { - let v = expr.eval_with_loc(scope).as_ref().clone(); + let v = expr.eval_with_loc(scope).trace_eval(|| ("Let(expr)", format!("{expr:?}")))?.as_ref().clone(); let let_scope = LocSingleScope::new(scope, name, &v); Ok( try_with!(d.parse_with_loc(program, &LocScope::Single(let_scope), input) => ("Let(parse)", format!("{} := {:?} <- {:?}", name, v, expr), format!("{d:?}"))), @@ -1811,7 +1811,7 @@ impl Decoder { ) } Decoder::Match(head, branches) => { - let head = head.eval_with_loc(scope); + let head = head.eval_with_loc(scope).trace_eval(|| ("Match(head)", format!("{head:?}")))?; for (index, (pattern, decoder)) in branches.iter().enumerate() { if let Some(pattern_scope) = head.matches(scope, pattern) { return Ok( @@ -1830,12 +1830,12 @@ impl Decoder { })) } Decoder::Dynamic(name, DynFormat::Huffman(lengths_expr, opt_values_expr), d) => { - let lengths_val = lengths_expr.eval_with_loc(scope); + let lengths_val = lengths_expr.eval_with_loc(scope).trace_eval(|| ("Dynamic(lengths_expr)", format!("{lengths_expr:?}")))?; let lengths = value_to_vec_usize(lengths_val.as_ref()); let lengths = match opt_values_expr { None => lengths, Some(e) => { - let values = value_to_vec_usize(e.eval_with_loc(scope).as_ref()); + let values = value_to_vec_usize(e.eval_with_loc(scope).trace_eval(|| ("Dynamic(e)", format!("{e:?}")))?.as_ref()); let mut new_lengths = [0].repeat(values.len()); for i in 0..lengths.len() { new_lengths[values[i]] = lengths[i]; @@ -1864,7 +1864,7 @@ impl Decoder { .parse_with_loc(program, scope, input) => ("LiftedOption(parse)", format!("{dec:?}"))) .map(|(v, input)| (ParsedValue::Option(Some(Box::new(v))), input))), Decoder::CaptureBytes(v_expr, len) => { - let len = len.eval_value_with_loc(scope).unwrap_usize(); + let len = len.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("CaptureBytes(len)", format!("{len:?}")))?; let view_window = Self::eval_view_expr_with_loc(scope, v_expr)?; // accumulate `len` bytes into a Vec @@ -1896,7 +1896,7 @@ impl Decoder { ))) } Decoder::ReadArray(v_expr, len, kind) => { - let len = len.eval_value_with_loc(scope).unwrap_usize(); + let len = len.eval_value_with_loc(scope).and_then(|v| v.try_as_usize()).trace_eval(|| ("ReadArray(len)", format!("{len:?}")))?; let view_window = Self::eval_view_expr_with_loc(scope, v_expr)?; let folded = WithErr::fold( @@ -1919,19 +1919,20 @@ impl Decoder { }); Ok(WithErr::new((v, input))) } - Decoder::Permit(inner, dft) => Ok(downgrade_error_with( + Decoder::Permit(inner, dft) => Ok(try_downgrade_error_with( inner .parse_with_loc(program, scope, input) .map(|ok| ok.map(|(v, input)| (ParsedValue::Permit(Ok(Box::new(v))), input))), || { - ( + Ok::<_, EvalError>(( ParsedValue::Permit(Err(Some(Box::new( - dft.eval_with_loc(scope).into_owned(), + dft.eval_with_loc(scope)?.into_owned(), )))), input, - ) + )) }, - )), + ) + .trace_eval(|| ("Permit(default)", format!("{dft:?}")))?), #[cfg(feature = "format_enforce")] Decoder::Enforce(inner) => { let res = inner.parse_with_loc(program, scope, input)?; @@ -1957,7 +1958,7 @@ impl Decoder { Ok(view) } ViewExpr::Offset(base, offset) => { - let offset = offset.eval_value_with_loc(scope).unwrap_usize(); + let offset = offset.eval_value_with_loc(scope)?.try_as_usize()?; let base_view = Self::eval_view_expr_with_loc(scope, base)?; let Some((_, mut view_window)) = base_view.split_at(offset) else { return Err(base_view.kind.overrun(offset, base_view.offset).into()); diff --git a/src/numeric/core.rs b/src/numeric/core.rs index c2264beb..82cd4864 100644 --- a/src/numeric/core.rs +++ b/src/numeric/core.rs @@ -1053,6 +1053,8 @@ impl From for EvalError { } } +/// Error returned when a variable identifier used within [`Expr::NumVar`] is scope-bound +/// to a `Value` that is not numeric. #[derive(Debug)] pub struct CoerceValueError { bad_value: crate::decoder::Value, @@ -1068,6 +1070,8 @@ impl std::fmt::Display for CoerceValueError { } } +impl std::error::Error for CoerceValueError {} + impl<'a> TryFrom<&'a crate::decoder::Value> for StrictValue { type Error = CoerceValueError; @@ -1149,7 +1153,17 @@ impl std::fmt::Display for EvalError { } } -impl std::error::Error for EvalError {} +impl std::error::Error for EvalError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + EvalError::UnknownVar(err) => Some(err), + EvalError::BadVariable(err) => Some(err), + EvalError::DivideByZero + | EvalError::RemainderNonPositive + | EvalError::Ambiguous(..) => None, + } + } +} #[derive(Debug, Clone)] pub struct Strict { @@ -1331,7 +1345,7 @@ pub fn bitwise_cast(num: BigInt, rep_in: NumRep, rep_out: MachineRep) -> BigInt } impl Expr { - /// Like `eval`, except that the representability of every individual sub-term is also checked, + /// Like [`Expr::eval`], except that the representability of every individual sub-term is also checked, /// and if any term is unrepresentable, the validity flag of the return-value will be `false`. pub fn eval_strict<'a, S, V>(&self, scope: &'a S) -> Result, EvalError> where @@ -1441,6 +1455,12 @@ impl Expr { } } + /// Attempts to evaluate `self` in a given scope, returning `Err` when: + /// - A referenced variable is not found in-scope + /// - Division by zero is attempted + /// - Remainder with a non-positive divisor is attempted + /// - A binary operation with ambiguous output representation is attempted (i.e. terms have different non-auto reps and operator itself isn't rep-tagged) + /// - Conversion from [`doodle::Value`](crate::decoder::value::Value) to [`numeric::Value`](Value) fails (i.e. a NumVar is scoped to a non-numeric value) pub fn eval<'a, S, V>(&self, scope: &'a S) -> Result where S: 'a + EvalScope<'a, Output = &'a V, Error = UnknownVarError>, @@ -1475,9 +1495,7 @@ impl Expr { } else { return Err(EvalError::RemainderNonPositive); } - } // (_, Value::Opt(..), _) | (_, _, Value::Opt(..)) => { - // return Err(EvalError::ArithOrCastOption) - // } + } }; let rep_out = match out_rep { Some(rep) => NumRep::Concrete(rep), diff --git a/src/util.rs b/src/util.rs index bd521751..55705c95 100644 --- a/src/util.rs +++ b/src/util.rs @@ -136,7 +136,7 @@ pub(crate) mod with_err { /// /// If `f` returns `Ok(other)`, this function returns `other` with any errors from `self` appended. /// Otherwise, returns the same `Err` that `f` returned. - pub fn join(self, mut f: impl FnMut(T) -> EResult) -> EResult { + pub fn join(self, f: impl FnOnce(T) -> EResult) -> EResult { let mut this_errs = self.errs; let mut ret = f(self.value)?; ret.errs.append(&mut this_errs); @@ -217,24 +217,25 @@ pub(crate) mod with_err { pub type EResult = Result, E1>; - /// Given `val: Result, E0>`, returns `v` if `val` is `Ok(v)`, and for `Err(e)`, returns `WithErr::with_err(default(), e)`. - pub fn downgrade_error_with( + /// Downgrades an error in `val` to a logged warning by substituting `default`, which is itself fallible: + /// if `default` fails, its error is propagated instead. + pub fn try_downgrade_error_with( val: EResult, - default: impl FnOnce() -> T, - ) -> WithErr + default: impl FnOnce() -> Result, + ) -> Result, E1> where E0: std::fmt::Display, { match val { - Ok(v) => v, + Ok(v) => Ok(v), Err(e) => { log::error!("downgraded error: {e}"); - WithErr::with_err(default(), e) + Ok(WithErr::with_err(default()?, e)) } } } } -pub(crate) use with_err::{EResult, WithErr, downgrade_error_with}; +pub(crate) use with_err::{EResult, WithErr, try_downgrade_error_with}; pub trait ErrTrace { fn with_trace(self, trace: T) -> Self @@ -286,9 +287,21 @@ mod tests { #[test] fn test_downgrade_error() { - let x = downgrade_error_with::<&'static str, &'static str>(Err("error"), || "default"); + let x = try_downgrade_error_with::<&'static str, &'static str, ()>(Err("error"), || { + Ok("default") + }) + .unwrap(); assert!(x.has_errs()); assert!(x.iter_errs().next().is_some_and(|e| *e == "error")); assert_eq!(x.into_inner(), "default"); } + + #[test] + fn test_downgrade_error_default_fails() { + let res = try_downgrade_error_with::<&'static str, &'static str, &'static str>( + Err("error"), + || Err("default failed"), + ); + assert_eq!(res.err(), Some("default failed")); + } } From 8f99b0a9f9aa5b143a3e7308d52cc9a868f28f2c Mon Sep 17 00:00:00 2001 From: Peter Duchovni Date: Mon, 21 Sep 2026 17:41:38 +1000 Subject: [PATCH 03/18] Add differential test for Expr::eval vs Expr::eval_with_loc parity The two evaluators are hand-maintained mirrors. This harness runs ~75 Expr cases through both and compares the resulting Value, error message, or panic. Known drift is recorded as `Expect::Diverges` and fails once the evaluators start to agree, so it cannot be fixed silently: - AsChar on Usize (loc lacks the arm) - FlatMap with an EnumFromTo-returning lambda (loc lacks the arm) - FlatMapAccum/LeftFold over EnumFromTo (main requires Value::Seq) - Match on Permit(Err(Some(_))) (main coerces nominally, loc does not) Safety net for unifying the two evaluators. Co-Authored-By: Claude Sonnet 5 --- src/decoder.rs | 2 + src/decoder/eval_parity_tests.rs | 476 +++++++++++++++++++++++++++++++ 2 files changed, 478 insertions(+) create mode 100644 src/decoder/eval_parity_tests.rs diff --git a/src/decoder.rs b/src/decoder.rs index fd65e669..f62f2501 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -17,6 +17,8 @@ use crate::{ }; use crate::{FixedReadKind, try_with}; +#[cfg(test)] +mod eval_parity_tests; pub mod seq_kind; use seq_kind::sub_range; pub use seq_kind::{SeqKind, ValueSeq}; diff --git a/src/decoder/eval_parity_tests.rs b/src/decoder/eval_parity_tests.rs new file mode 100644 index 00000000..d02df9a2 --- /dev/null +++ b/src/decoder/eval_parity_tests.rs @@ -0,0 +1,476 @@ +//! Differential tests asserting that `Expr::eval_value` (see `decoder.rs`) and `Expr::eval_value_with_loc` +//! (see `loc_decoder.rs`) agree on every input. +//! +//! The two evaluators are hand-maintained mirrors of one another. These tests exist as a safety net for +//! unifying them: each [`Case`] is run through both, and the observable outcomes (result `Value`, error +//! message, or "panicked") are compared. +//! +//! Known, not-yet-fixed divergences are recorded as [`Expect::Diverges`]. Such a case *fails* once the +//! evaluators start to agree, at which point it should be switched to [`Expect::Parity`]. + +use std::panic::{AssertUnwindSafe, catch_unwind}; + +use super::{MultiScope, Scope, SeqKind, Value}; +use crate::loc_decoder::{LocMultiScope, LocScope, ParsedValue}; +use crate::{Arith, Expr, IntRel, Label, Pattern, TypeHint, ValueType}; + +#[derive(Debug, PartialEq, Eq)] +enum Outcome { + Value(String), + Err(String), + Panic, +} + +/// Rewrites every `SeqKind::Dup` into `SeqKind::Strict`, since `ParsedValue::from_evaluated` materializes +/// `Dup` sequences and the two evaluators would otherwise differ in representation only. +fn normalize(v: Value) -> Value { + match v { + Value::Seq(elts) => Value::Seq(SeqKind::Strict(elts.into_iter().map(normalize).collect())), + Value::Tuple(vs) => Value::Tuple(vs.into_iter().map(normalize).collect()), + Value::Record(fs) => { + Value::Record(fs.into_iter().map(|(l, v)| (l, normalize(v))).collect()) + } + Value::Variant(l, inner) => Value::Variant(l, Box::new(normalize(*inner))), + Value::Option(opt) => Value::Option(opt.map(|inner| Box::new(normalize(*inner)))), + other => other, + } +} + +fn observe(f: impl FnOnce() -> Result) -> Outcome { + match catch_unwind(AssertUnwindSafe(f)) { + Ok(Ok(v)) => Outcome::Value(format!("{:?}", normalize(v))), + Ok(Err(e)) => Outcome::Err(e.to_string()), + Err(_) => Outcome::Panic, + } +} + +fn eval_main(expr: &Expr, vars: &[(&'static str, Value)]) -> Outcome { + observe(|| { + let root = Scope::Empty; + let mut multi = MultiScope::with_capacity(&root, vars.len()); + for (name, value) in vars { + multi.push(*name, value); + } + let scope = Scope::Multi(&multi); + expr.eval_value(&scope) + }) +} + +fn eval_loc(expr: &Expr, vars: &[(&'static str, Value)]) -> Outcome { + observe(|| { + let root = LocScope::Empty; + let mut multi = LocMultiScope::with_capacity(&root, vars.len()); + for (name, value) in vars { + multi.push(*name, ParsedValue::from_evaluated(value.clone())); + } + let scope = LocScope::Multi(&multi); + expr.eval_value_with_loc(&scope) + }) +} + +enum Expect { + Parity, + Diverges(&'static str), +} + +struct Case { + name: &'static str, + expr: Expr, + vars: Vec<(&'static str, Value)>, + expect: Expect, +} + +fn b(e: Expr) -> Box { + Box::new(e) +} + +/// The evaluators ignore type hints, so any `ValueType` will do +fn hint() -> TypeHint { + ValueType::NumericHole.into() +} + +fn var(name: &'static str) -> Expr { + Expr::Var(Label::from(name)) +} + +fn lam(name: &'static str, body: Expr) -> Expr { + Expr::Lambda(Label::from(name), b(body)) +} + +fn seq_u8(items: &[u8]) -> Expr { + Expr::Seq(items.iter().copied().map(Expr::U8).collect()) +} + +fn bytes(items: &[u8]) -> Expr { + Expr::Tuple(items.iter().copied().map(Expr::U8).collect()) +} + +fn parity(name: &'static str, expr: Expr) -> Case { + Case { + name, + expr, + vars: Vec::new(), + expect: Expect::Parity, + } +} + +fn parity_with(name: &'static str, expr: Expr, vars: Vec<(&'static str, Value)>) -> Case { + Case { + name, + expr, + vars, + expect: Expect::Parity, + } +} + +fn diverges(name: &'static str, why: &'static str, expr: Expr) -> Case { + diverges_with(name, why, expr, Vec::new()) +} + +fn diverges_with( + name: &'static str, + why: &'static str, + expr: Expr, + vars: Vec<(&'static str, Value)>, +) -> Case { + Case { + name, + expr, + vars, + expect: Expect::Diverges(why), + } +} + +/// `\p -> (p.0, [p.1])`, the lambda shape used by `FlatMapAccum` +fn accum_pass_through() -> Expr { + lam( + "p", + Expr::Tuple(vec![ + Expr::TupleProj(b(var("p")), 0), + Expr::Seq(vec![Expr::TupleProj(b(var("p")), 1)]), + ]), + ) +} + +/// `\p -> p.1`, the lambda shape used by `LeftFold` (keeps the last element seen) +fn fold_last() -> Expr { + lam("p", Expr::TupleProj(b(var("p")), 1)) +} + +fn range(start: u32, stop: u32) -> Expr { + Expr::EnumFromTo(b(Expr::U32(start)), b(Expr::U32(stop))) +} + +fn cases() -> Vec { + let mut cases = vec![ + // -- literals and structural constructors + parity("bool", Expr::Bool(true)), + parity("u8", Expr::U8(7)), + parity("u64", Expr::U64(u64::MAX)), + parity("tuple", Expr::Tuple(vec![Expr::U8(1), Expr::Bool(false)])), + parity( + "tuple_proj", + Expr::TupleProj(b(Expr::Tuple(vec![Expr::U8(1), Expr::U16(2)])), 1), + ), + parity( + "record_proj", + Expr::RecordProj( + b(Expr::Record(vec![ + ("a".into(), Expr::U8(1)), + ("b".into(), Expr::U16(2)), + ])), + "b".into(), + ), + ), + parity("variant", Expr::Variant("v".into(), b(Expr::U8(3)))), + parity("seq", seq_u8(&[1, 2, 3])), + parity("lift_option_some", Expr::LiftOption(Some(b(Expr::U8(1))))), + parity("lift_option_none", Expr::LiftOption(None)), + // -- variables + parity_with( + "var_tuple_proj", + Expr::TupleProj(b(var("t")), 0), + vec![("t", Value::Tuple(vec![Value::U8(9), Value::Bool(true)]))], + ), + parity_with( + "var_record_proj", + Expr::RecordProj(b(var("r")), "x".into()), + vec![("r", Value::record([("x", Value::U32(5))]))], + ), + // -- arithmetic and relations + parity( + "add_ok", + Expr::Arith(Arith::Add, b(Expr::U8(1)), b(Expr::U8(2))), + ), + parity( + "add_overflow", + Expr::Arith(Arith::Add, b(Expr::U8(255)), b(Expr::U8(1))), + ), + parity( + "sub_underflow", + Expr::Arith(Arith::Sub, b(Expr::U16(0)), b(Expr::U16(1))), + ), + parity( + "div_zero", + Expr::Arith(Arith::Div, b(Expr::U32(4)), b(Expr::U32(0))), + ), + parity( + "rel_lt", + Expr::IntRel(IntRel::Lt, b(Expr::U8(1)), b(Expr::U8(2))), + ), + parity( + "rel_eq_false", + Expr::IntRel(IntRel::Eq, b(Expr::U64(1)), b(Expr::U64(2))), + ), + // -- narrowing / widening casts + parity("as_u8_ok", Expr::AsU8(b(Expr::U32(200)))), + parity("as_u8_overflow", Expr::AsU8(b(Expr::U32(300)))), + parity("as_u16_widen", Expr::AsU16(b(Expr::U8(9)))), + parity("as_u32_overflow", Expr::AsU32(b(Expr::U64(u64::MAX)))), + parity("as_u64_widen", Expr::AsU64(b(Expr::U32(9)))), + parity( + "as_u8_from_usize", + Expr::AsU8(b(Expr::SeqLength(b(seq_u8(&[1, 2]))))), + ), + parity("as_char_u8", Expr::AsChar(b(Expr::U8(65)))), + parity("as_char_u32_invalid", Expr::AsChar(b(Expr::U32(0xD800)))), + parity("as_char_u64_overflow", Expr::AsChar(b(Expr::U64(u64::MAX)))), + // -- byte-tuple decoders + parity("u16be", Expr::U16Be(b(bytes(&[1, 2])))), + parity("u16le", Expr::U16Le(b(bytes(&[1, 2])))), + parity("u32be", Expr::U32Be(b(bytes(&[1, 2, 3, 4])))), + parity("u32le", Expr::U32Le(b(bytes(&[1, 2, 3, 4])))), + parity("u64be", Expr::U64Be(b(bytes(&[1, 2, 3, 4, 5, 6, 7, 8])))), + parity("u64le", Expr::U64Le(b(bytes(&[1, 2, 3, 4, 5, 6, 7, 8])))), + parity("u16be_bad_shape", Expr::U16Be(b(bytes(&[1, 2, 3])))), + // -- sequences + parity("seq_length", Expr::SeqLength(b(seq_u8(&[1, 2, 3])))), + parity("seq_length_range", Expr::SeqLength(b(range(2, 7)))), + parity( + "seq_ix", + Expr::SeqIx(b(seq_u8(&[10, 20, 30])), b(Expr::U32(1))), + ), + parity( + "seq_ix_oob", + Expr::SeqIx(b(seq_u8(&[10, 20, 30])), b(Expr::U32(3))), + ), + parity("seq_ix_range", Expr::SeqIx(b(range(5, 9)), b(Expr::U32(2)))), + parity( + "seq_ix_range_oob", + Expr::SeqIx(b(range(5, 9)), b(Expr::U32(4))), + ), + parity( + "sub_seq", + Expr::SubSeq(b(seq_u8(&[1, 2, 3, 4])), b(Expr::U32(1)), b(Expr::U32(2))), + ), + parity( + "sub_seq_oob", + Expr::SubSeq(b(seq_u8(&[1, 2, 3, 4])), b(Expr::U32(3)), b(Expr::U32(5))), + ), + parity( + "sub_seq_range", + Expr::SubSeq(b(range(10, 20)), b(Expr::U32(2)), b(Expr::U32(3))), + ), + parity( + "sub_seq_inflate_wrap", + Expr::SubSeqInflate(b(seq_u8(&[1, 2, 3])), b(Expr::U32(1)), b(Expr::U32(6))), + ), + parity( + "sub_seq_inflate_range", + Expr::SubSeqInflate(b(range(0, 3)), b(Expr::U32(1)), b(Expr::U32(6))), + ), + parity("append", Expr::Append(b(seq_u8(&[1])), b(seq_u8(&[2, 3])))), + parity( + "append_lhs_empty", + Expr::Append(b(seq_u8(&[])), b(seq_u8(&[2, 3]))), + ), + parity( + "append_rhs_empty", + Expr::Append(b(seq_u8(&[1])), b(seq_u8(&[]))), + ), + parity("append_range", Expr::Append(b(range(0, 2)), b(range(5, 7)))), + parity("dup", Expr::Dup(b(Expr::U32(3)), b(Expr::U8(7)))), + parity("dup_zero", Expr::Dup(b(Expr::U32(0)), b(Expr::U8(7)))), + parity("enum_from_to", range(3, 6)), + parity("enum_from_to_empty", range(3, 3)), + // -- higher-order sequence operations + parity( + "flat_map_seq", + Expr::FlatMap( + b(lam("x", Expr::Seq(vec![var("x"), var("x")]))), + b(seq_u8(&[1, 2, 3])), + ), + ), + parity( + "find_by_key_unsorted", + Expr::FindByKey( + false, + b(lam("x", var("x"))), + b(Expr::U8(2)), + b(seq_u8(&[3, 2, 1])), + ), + ), + parity( + "find_by_key_sorted_hit", + Expr::FindByKey( + true, + b(lam("x", var("x"))), + b(Expr::U8(2)), + b(seq_u8(&[1, 2, 3])), + ), + ), + parity( + "find_by_key_missing", + Expr::FindByKey( + true, + b(lam("x", var("x"))), + b(Expr::U8(9)), + b(seq_u8(&[1, 2, 3])), + ), + ), + // -- pattern-matching + parity( + "match_first_branch", + Expr::Match( + b(Expr::U8(1)), + vec![ + (Pattern::U8(1), Expr::U8(10)), + (Pattern::Wildcard, Expr::U8(20)), + ], + ), + ), + parity( + "match_fallthrough_binding", + Expr::Match( + b(Expr::U8(5)), + vec![ + (Pattern::U8(1), Expr::U8(10)), + (Pattern::Binding("n".into()), var("n")), + ], + ), + ), + parity( + "match_non_exhaustive", + Expr::Match(b(Expr::U8(5)), vec![(Pattern::U8(1), Expr::U8(10))]), + ), + parity( + "destructure_tuple", + Expr::Destructure( + b(Expr::Tuple(vec![Expr::U8(1), Expr::U8(2)])), + Pattern::Tuple(vec![ + Pattern::Binding("a".into()), + Pattern::Binding("b".into()), + ]), + b(Expr::Arith(Arith::Add, b(var("a")), b(var("b")))), + ), + ), + parity( + "destructure_refuted", + Expr::Destructure(b(Expr::U8(1)), Pattern::U8(2), b(Expr::U8(0))), + ), + ]; + + // -- accumulating / folding operations (seq input) + cases.push(parity( + "flat_map_accum_seq", + Expr::FlatMapAccum( + b(accum_pass_through()), + b(Expr::U8(0)), + hint(), + b(seq_u8(&[1, 2, 3])), + ), + )); + cases.push(parity( + "left_fold_seq", + Expr::LeftFold( + b(fold_last()), + b(Expr::U8(0)), + hint(), + b(seq_u8(&[1, 2, 3])), + ), + )); + cases.push(parity( + "flat_map_list_seq", + Expr::FlatMapList( + b(lam("p", Expr::Seq(vec![Expr::TupleProj(b(var("p")), 1)]))), + hint(), + b(seq_u8(&[1, 2, 3])), + ), + )); + cases.push(parity( + "flat_map_list_range", + Expr::FlatMapList( + b(lam("p", Expr::Seq(vec![Expr::TupleProj(b(var("p")), 1)]))), + hint(), + b(range(0, 3)), + ), + )); + + // -- known divergences (fill in from observed results) + cases.push(diverges( + "as_char_usize", + "loc `AsChar` has no `Usize` arm", + Expr::AsChar(b(Expr::SeqIx(b(range(2, 7)), b(Expr::U32(0))))), + )); + cases.push(diverges( + "flat_map_accum_range", + "main `FlatMapAccum` requires `Value::Seq`, loc accepts any sequence (incl. `EnumFromTo`)", + Expr::FlatMapAccum( + b(accum_pass_through()), + b(Expr::U8(0)), + hint(), + b(range(0, 3)), + ), + )); + cases.push(diverges( + "left_fold_range", + "main `LeftFold` requires `Value::Seq`, loc accepts any sequence (incl. `EnumFromTo`)", + Expr::LeftFold(b(fold_last()), b(Expr::U8(0)), hint(), b(range(0, 3))), + )); + cases.push(diverges_with( + "match_permit_err_some", + "main `Match` coerces via `coerce_nominal_value` (unwraps `Permit(Err(Some))`), loc via `coerce_mapped_value` (does not)", + Expr::Match( + b(var("p")), + vec![(Pattern::U8(1), Expr::U8(10)), (Pattern::Wildcard, Expr::U8(20))], + ), + vec![("p", Value::Permit(Err(Some(Box::new(Value::U8(1))))))], + )); + cases.push(diverges( + "flat_map_returns_range", + "loc `FlatMap` has no `EnumFromTo` arm for the lambda result", + Expr::FlatMap(b(lam("x", range(0, 2))), b(seq_u8(&[1, 2]))), + )); + + cases +} + +#[test] +fn eval_and_eval_with_loc_agree() { + let mut failures = Vec::new(); + for Case { + name, + expr, + vars, + expect, + } in cases() + { + let main = eval_main(&expr, &vars); + let loc = eval_loc(&expr, &vars); + match expect { + Expect::Parity if main != loc => failures.push(format!( + "{name}: DIVERGED\n main: {main:?}\n loc: {loc:?}" + )), + Expect::Diverges(why) if main == loc => failures.push(format!( + "{name}: now agrees ({main:?}); drift fixed? switch to `Parity` (was: {why})" + )), + _ => {} + } + } + assert!( + failures.is_empty(), + "{} case(s) failed:\n{}", + failures.len(), + failures.join("\n") + ); +} From 5413d396770710248d437cbe20db0191c7b922dd Mon Sep 17 00:00:00 2001 From: Peter Duchovni Date: Tue, 22 Sep 2026 10:50:56 +1000 Subject: [PATCH 04/18] Extract shared leaf logic from Expr::eval / eval_with_loc AsU8/16/32/64, AsChar, the U16Be/Le..U64Be/Le byte-tuple decoders, and SubSeqInflate's back-reference expansion were duplicated near-verbatim between the two evaluators. Pull the common logic onto shared Value::cast_to_u8/16/32/64, Value::cast_to_char, Value::unwrap_byte_array::, and a free decoder::sub_seq_inflate generic over both value types, so both `eval` bodies now just call through in one or two lines. This is Stage 1 of unifying Expr::eval/eval_with_loc (Stage 0: the eval_parity_tests differential harness, 8f99b0a9). It incidentally fixes a real divergence: loc's AsChar had no Usize arm, so `cast_to_char` now covers it and the as_char_usize parity case moves from `diverges` to `parity`. Byte-tuple panic messages for U64Be/Le are also corrected (they previously said "U32Be"/"U32Le", a copy-paste typo). No other behavior change; full workspace test suite and the parity test both pass. --- src/decoder.rs | 194 +++++++++++-------------------- src/decoder/eval_parity_tests.rs | 4 +- src/decoder/value.rs | 92 +++++++++++++++ src/loc_decoder.rs | 170 +++++++-------------------- 4 files changed, 199 insertions(+), 261 deletions(-) diff --git a/src/decoder.rs b/src/decoder.rs index f62f2501..695df2c6 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -62,6 +62,46 @@ pub(crate) fn extract_pair(mut vec: Vec) -> (T, T) { } } +/// Implements `Expr::SubSeqInflate`: yields `length` items starting at `start`, where an index that runs past +/// the end of `values` refers back into the output built so far (i.e. the sequence repeats, LZ77-style). +/// +/// Shared by the `Value` and `ParsedValue` evaluators. +/// +/// # Panics +/// +/// Panics if a back-reference falls outside the output built so far (e.g. `start > values.len()`). +pub(crate) fn sub_seq_inflate>( + values: ValueSeq<'_, V>, + start: usize, + length: usize, +) -> Vec { + let mut vs = Vec::new(); + match values { + ValueSeq::ValueSeq(vs0) => { + for i in 0..length { + if i + start < vs0.len() { + vs.push(vs0[i + start].clone()); + } else { + vs.push(vs[i + start - vs0.len()].clone()); + } + } + } + ValueSeq::IntRange(range) => { + // REVIEW - double-check this logic + let len = range.len(); + let mut iter = range.skip(start); + for i in 0..length { + if let Some(val) = iter.next() { + vs.push(val.into()); + } else { + vs.push(vs[i + start - len].clone()); + } + } + } + } + vs +} + pub mod value; pub use crate::error::EvalError; pub use value::{ArithError, ArithOp, Value}; @@ -145,110 +185,30 @@ impl Expr { Value::unary(*op, value)? }), - // FIXME - extract common logic for As-expr on Value instead of separate impl for decoder and loc_decoder - Expr::AsU8(x) => Cow::Owned(match x.eval_value(scope)? { - Value::U8(x) => Value::U8(x), - Value::U16(x) => Value::U8(u8::try_from(x)?), - Value::U32(x) => Value::U8(u8::try_from(x)?), - Value::U64(x) => Value::U8(u8::try_from(x)?), - Value::Usize(x) => Value::U8(u8::try_from(x)?), - x => panic!("cannot convert {x:?} to U8"), - }), - Expr::AsU16(x) => Cow::Owned(match x.eval_value(scope)? { - Value::U8(x) => Value::U16(u16::from(x)), - Value::U16(x) => Value::U16(x), - Value::U32(x) => Value::U16(u16::try_from(x)?), - Value::U64(x) => Value::U16(u16::try_from(x)?), - Value::Usize(x) => Value::U16(u16::try_from(x)?), - x => panic!("cannot convert {x:?} to U16"), - }), - Expr::AsU32(x) => Cow::Owned(match x.eval_value(scope)? { - Value::U8(x) => Value::U32(u32::from(x)), - Value::U16(x) => Value::U32(u32::from(x)), - Value::U32(x) => Value::U32(x), - Value::U64(x) => Value::U32(u32::try_from(x)?), - Value::Usize(x) => Value::U32(u32::try_from(x)?), - x => panic!("cannot convert {x:?} to U32"), - }), - Expr::AsU64(x) => Cow::Owned(match x.eval_value(scope)? { - Value::U8(x) => Value::U64(u64::from(x)), - Value::U16(x) => Value::U64(u64::from(x)), - Value::U32(x) => Value::U64(u64::from(x)), - Value::U64(x) => Value::U64(x), - Value::Usize(x) => Value::U64(u64::try_from(x)?), - x => panic!("cannot convert {x:?} to U64"), - }), - - Expr::U16Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [Value::U8(hi), Value::U8(lo)] => { - Cow::Owned(Value::U16(u16::from_be_bytes([*hi, *lo]))) - } - _ => panic!("U16Be: expected (U8, U8)"), - }, - Expr::U16Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [Value::U8(lo), Value::U8(hi)] => { - Cow::Owned(Value::U16(u16::from_le_bytes([*lo, *hi]))) - } - _ => panic!("U16Le: expected (U8, U8)"), - }, - Expr::U32Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { - Cow::Owned(Value::U32(u32::from_be_bytes([*a, *b, *c, *d]))) - } - _ => panic!("U32Be: expected (U8, U8, U8, U8)"), - }, - Expr::U32Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { - Cow::Owned(Value::U32(u32::from_le_bytes([*a, *b, *c, *d]))) - } - _ => panic!("U32Le: expected (U8, U8, U8, U8)"), - }, - Expr::U64Be(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [ - Value::U8(a), - Value::U8(b), - Value::U8(c), - Value::U8(d), - Value::U8(e), - Value::U8(f), - Value::U8(g), - Value::U8(h), - ] => Cow::Owned(Value::U64(u64::from_be_bytes([ - *a, *b, *c, *d, *e, *f, *g, *h, - ]))), - _ => panic!("U32Be: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), - }, - Expr::U64Le(bytes) => match bytes.eval_value(scope)?.unwrap_tuple().as_slice() { - [ - Value::U8(a), - Value::U8(b), - Value::U8(c), - Value::U8(d), - Value::U8(e), - Value::U8(f), - Value::U8(g), - Value::U8(h), - ] => Cow::Owned(Value::U64(u64::from_le_bytes([ - *a, *b, *c, *d, *e, *f, *g, *h, - ]))), - _ => panic!("U32Le: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), - }, - Expr::AsChar(bytes) => Cow::Owned(match bytes.eval_value(scope)? { - Value::U8(x) => Value::Char(char::from(x)), - Value::U16(x) => { - Value::Char(char::from_u32(x as u32).unwrap_or(char::REPLACEMENT_CHARACTER)) - } - Value::U32(x) => { - Value::Char(char::from_u32(x).unwrap_or(char::REPLACEMENT_CHARACTER)) - } - Value::U64(x) => Value::Char( - char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), - ), - Value::Usize(x) => Value::Char( - char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), - ), - _ => panic!("AsChar: expected U8, U16, U32, or U64"), - }), + Expr::AsU8(x) => Cow::Owned(x.eval_value(scope)?.cast_to_u8()?), + Expr::AsU16(x) => Cow::Owned(x.eval_value(scope)?.cast_to_u16()?), + Expr::AsU32(x) => Cow::Owned(x.eval_value(scope)?.cast_to_u32()?), + Expr::AsU64(x) => Cow::Owned(x.eval_value(scope)?.cast_to_u64()?), + + Expr::U16Be(bytes) => Cow::Owned(Value::U16(u16::from_be_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<2>("U16Be"), + ))), + Expr::U16Le(bytes) => Cow::Owned(Value::U16(u16::from_le_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<2>("U16Le"), + ))), + Expr::U32Be(bytes) => Cow::Owned(Value::U32(u32::from_be_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<4>("U32Be"), + ))), + Expr::U32Le(bytes) => Cow::Owned(Value::U32(u32::from_le_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<4>("U32Le"), + ))), + Expr::U64Be(bytes) => Cow::Owned(Value::U64(u64::from_be_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<8>("U64Be"), + ))), + Expr::U64Le(bytes) => Cow::Owned(Value::U64(u64::from_le_bytes( + bytes.eval_value(scope)?.unwrap_byte_array::<8>("U64Le"), + ))), + Expr::AsChar(x) => Cow::Owned(x.eval_value(scope)?.cast_to_char()?), Expr::SeqLength(seq) => match seq.eval(scope)?.coerce_mapped_value().get_sequence() { Some(values) => { let len = values.len(); @@ -292,31 +252,7 @@ impl Expr { Some(values) => { let start = start.eval_value(scope)?.try_as_usize()?; let length = length.eval_value(scope)?.try_as_usize()?; - let mut vs = Vec::new(); - match values { - ValueSeq::ValueSeq(vs0) => { - for i in 0..length { - if i + start < vs0.len() { - vs.push(vs0[i + start].clone()); - } else { - vs.push(vs[i + start - vs0.len()].clone()); - } - } - } - ValueSeq::IntRange(range) => { - // REVIEW - double-check this logic - let len = range.len(); - let mut iter = range.skip(start); - for i in 0..length { - if let Some(val) = iter.next() { - vs.push(val.into()); - } else { - vs.push(vs[i + start - len].clone()); - } - } - } - } - Cow::Owned(Value::Seq(vs.into())) + Cow::Owned(Value::Seq(sub_seq_inflate(values, start, length).into())) } _ => panic!("SubSeqInflate: expected Seq"), } diff --git a/src/decoder/eval_parity_tests.rs b/src/decoder/eval_parity_tests.rs index d02df9a2..dbe331ce 100644 --- a/src/decoder/eval_parity_tests.rs +++ b/src/decoder/eval_parity_tests.rs @@ -407,11 +407,11 @@ fn cases() -> Vec { )); // -- known divergences (fill in from observed results) - cases.push(diverges( + cases.push(parity( "as_char_usize", - "loc `AsChar` has no `Usize` arm", Expr::AsChar(b(Expr::SeqIx(b(range(2, 7)), b(Expr::U32(0))))), )); + cases.push(diverges( "flat_map_accum_range", "main `FlatMapAccum` requires `Value::Seq`, loc accepts any sequence (incl. `EnumFromTo`)", diff --git a/src/decoder/value.rs b/src/decoder/value.rs index 3b1af9a0..b7fb4049 100644 --- a/src/decoder/value.rs +++ b/src/decoder/value.rs @@ -519,6 +519,98 @@ impl Value { } } + /// Converts any fixed-width integer `Value` to a `U8`, returning `Err` if the value does not fit. + /// + /// # Panics + /// + /// Panics if the value is not an integer, as this is an invariant enforced by the type-checker. + pub(crate) fn cast_to_u8(self) -> Result { + Ok(Value::U8(match self { + Value::U8(x) => x, + Value::U16(x) => u8::try_from(x)?, + Value::U32(x) => u8::try_from(x)?, + Value::U64(x) => u8::try_from(x)?, + Value::Usize(x) => u8::try_from(x)?, + x => panic!("cannot convert {x:?} to U8"), + })) + } + + /// As [`Self::cast_to_u8`], but to `U16`. + pub(crate) fn cast_to_u16(self) -> Result { + Ok(Value::U16(match self { + Value::U8(x) => u16::from(x), + Value::U16(x) => x, + Value::U32(x) => u16::try_from(x)?, + Value::U64(x) => u16::try_from(x)?, + Value::Usize(x) => u16::try_from(x)?, + x => panic!("cannot convert {x:?} to U16"), + })) + } + + /// As [`Self::cast_to_u8`], but to `U32`. + pub(crate) fn cast_to_u32(self) -> Result { + Ok(Value::U32(match self { + Value::U8(x) => u32::from(x), + Value::U16(x) => u32::from(x), + Value::U32(x) => x, + Value::U64(x) => u32::try_from(x)?, + Value::Usize(x) => u32::try_from(x)?, + x => panic!("cannot convert {x:?} to U32"), + })) + } + + /// As [`Self::cast_to_u8`], but to `U64`. + pub(crate) fn cast_to_u64(self) -> Result { + Ok(Value::U64(match self { + Value::U8(x) => u64::from(x), + Value::U16(x) => u64::from(x), + Value::U32(x) => u64::from(x), + Value::U64(x) => x, + Value::Usize(x) => u64::try_from(x)?, + x => panic!("cannot convert {x:?} to U64"), + })) + } + + /// Converts any fixed-width integer `Value` to a `Char`, substituting `char::REPLACEMENT_CHARACTER` for + /// any value that is not a Unicode scalar value. Returns `Err` only if the value does not fit in a `u32`. + /// + /// # Panics + /// + /// Panics if the value is not an integer, as this is an invariant enforced by the type-checker. + pub(crate) fn cast_to_char(self) -> Result { + let code_point = match self { + Value::U8(x) => u32::from(x), + Value::U16(x) => u32::from(x), + Value::U32(x) => x, + Value::U64(x) => u32::try_from(x)?, + Value::Usize(x) => u32::try_from(x)?, + _ => panic!("AsChar: expected U8, U16, U32, U64, or Usize"), + }; + Ok(Value::Char( + char::from_u32(code_point).unwrap_or(char::REPLACEMENT_CHARACTER), + )) + } + + /// Unwraps a `Value::Tuple` of exactly `N` `Value::U8`s into an array of bytes. + /// + /// `ctx` names the calling operation, for the panic message. + /// + /// # Panics + /// + /// Panics if the value is not a tuple of `N` `U8`s, as this is an invariant enforced by the type-checker. + pub(crate) fn unwrap_byte_array(self, ctx: &str) -> [u8; N] { + match <[Value; N]>::try_from(self.unwrap_tuple()) { + Ok(values) => values.map(|v| match v { + Value::U8(b) => b, + other => panic!("{ctx}: expected a tuple of {N} U8s, found element {other:?}"), + }), + Err(values) => panic!( + "{ctx}: expected a tuple of {N} U8s, found {} elements", + values.len() + ), + } + } + pub(crate) fn unwrap_tuple(self) -> Vec { match self { Value::Tuple(values) => values, diff --git a/src/loc_decoder.rs b/src/loc_decoder.rs index 1c8f418f..84fff463 100644 --- a/src/loc_decoder.rs +++ b/src/loc_decoder.rs @@ -12,6 +12,7 @@ use crate::decoder::{ cow_map, cow_remap, extract_pair, search::{find_index_by_key_sorted, find_index_by_key_unsorted}, seq_kind::sub_range, + sub_seq_inflate, }; use crate::error::{DecodeError, DecodeErrorKind, EvalError, EvalResultExt as _, UnknownVarError}; use crate::read::{BufferKind, ReadCtxt}; @@ -670,132 +671,63 @@ impl Expr { Value::unary(*op, value)? })), - // FIXME - extract common logic for As-expr on Value instead of separate impl for decoder and loc_decoder Expr::AsU8(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope)? { - Value::U8(x) => Value::U8(x), - Value::U16(x) => Value::U8(u8::try_from(x)?), - Value::U32(x) => Value::U8(u8::try_from(x)?), - Value::U64(x) => Value::U8(u8::try_from(x)?), - Value::Usize(x) => Value::U8(u8::try_from(x)?), - x => panic!("cannot convert {x:?} to U8"), - }, + x.eval_value_with_loc(scope)?.cast_to_u8()?, )), Expr::AsU16(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope)? { - Value::U8(x) => Value::U16(u16::from(x)), - Value::U16(x) => Value::U16(x), - Value::U32(x) => Value::U16(u16::try_from(x)?), - Value::U64(x) => Value::U16(u16::try_from(x)?), - Value::Usize(x) => Value::U16(u16::try_from(x)?), - x => panic!("cannot convert {x:?} to U16"), - }, + x.eval_value_with_loc(scope)?.cast_to_u16()?, )), Expr::AsU32(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope)? { - Value::U8(x) => Value::U32(u32::from(x)), - Value::U16(x) => Value::U32(u32::from(x)), - Value::U32(x) => Value::U32(x), - Value::U64(x) => Value::U32(u32::try_from(x)?), - Value::Usize(x) => Value::U32(u32::try_from(x)?), - x => panic!("cannot convert {x:?} to U32"), - }, + x.eval_value_with_loc(scope)?.cast_to_u32()?, )), Expr::AsU64(x) => Cow::Owned(ParsedValue::from_evaluated( - match x.eval_value_with_loc(scope)? { - Value::U8(x) => Value::U64(u64::from(x)), - Value::U16(x) => Value::U64(u64::from(x)), - Value::U32(x) => Value::U64(u64::from(x)), - Value::U64(x) => Value::U64(x), - Value::Usize(x) => Value::U64(u64::try_from(x)?), - x => panic!("cannot convert {x:?} to U64"), - }, + x.eval_value_with_loc(scope)?.cast_to_u64()?, )), Expr::U16Be(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [Value::U8(hi), Value::U8(lo)] => Cow::Owned(ParsedValue::from_evaluated( - Value::U16(u16::from_be_bytes([*hi, *lo])), - )), - _ => panic!("U16Be: expected (U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U16(u16::from_be_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<2>("U16Be"), + )))) } Expr::U16Le(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [Value::U8(lo), Value::U8(hi)] => Cow::Owned(ParsedValue::from_evaluated( - Value::U16(u16::from_le_bytes([*lo, *hi])), - )), - _ => panic!("U16Le: expected (U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U16(u16::from_le_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<2>("U16Le"), + )))) } Expr::U32Be(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { - Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_be_bytes( - [*a, *b, *c, *d], - )))) - } - _ => panic!("U32Be: expected (U8, U8, U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_be_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<4>("U32Be"), + )))) } Expr::U32Le(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [Value::U8(a), Value::U8(b), Value::U8(c), Value::U8(d)] => { - Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_le_bytes( - [*a, *b, *c, *d], - )))) - } - _ => panic!("U32Le: expected (U8, U8, U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U32(u32::from_le_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<4>("U32Le"), + )))) } Expr::U64Be(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [ - Value::U8(a), - Value::U8(b), - Value::U8(c), - Value::U8(d), - Value::U8(e), - Value::U8(f), - Value::U8(g), - Value::U8(h), - ] => Cow::Owned(ParsedValue::from_evaluated(Value::U64(u64::from_be_bytes( - [*a, *b, *c, *d, *e, *f, *g, *h], - )))), - _ => panic!("U32Be: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U64(u64::from_be_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<8>("U64Be"), + )))) } Expr::U64Le(bytes) => { - match bytes.eval_value_with_loc(scope)?.unwrap_tuple().as_slice() { - [ - Value::U8(a), - Value::U8(b), - Value::U8(c), - Value::U8(d), - Value::U8(e), - Value::U8(f), - Value::U8(g), - Value::U8(h), - ] => Cow::Owned(ParsedValue::from_evaluated(Value::U64(u64::from_le_bytes( - [*a, *b, *c, *d, *e, *f, *g, *h], - )))), - _ => panic!("U32Le: expected (U8, U8, U8, U8, U8, U8, U8, U8)"), - } + Cow::Owned(ParsedValue::from_evaluated(Value::U64(u64::from_le_bytes( + bytes + .eval_value_with_loc(scope)? + .unwrap_byte_array::<8>("U64Le"), + )))) } - Expr::AsChar(bytes) => Cow::Owned(ParsedValue::from_evaluated( - match bytes.eval_value_with_loc(scope)? { - Value::U8(x) => Value::Char(char::from(x)), - Value::U16(x) => { - Value::Char(char::from_u32(x as u32).unwrap_or(char::REPLACEMENT_CHARACTER)) - } - Value::U32(x) => { - Value::Char(char::from_u32(x).unwrap_or(char::REPLACEMENT_CHARACTER)) - } - Value::U64(x) => Value::Char( - char::from_u32(u32::try_from(x)?).unwrap_or(char::REPLACEMENT_CHARACTER), - ), - _ => panic!("AsChar: expected U8, U16, U32, or U64"), - }, + Expr::AsChar(x) => Cow::Owned(ParsedValue::from_evaluated( + x.eval_value_with_loc(scope)?.cast_to_char()?, )), Expr::SeqLength(seq) => match seq .eval_with_loc(scope)? @@ -854,31 +786,9 @@ impl Expr { Some(values) => { let start = start.eval_value_with_loc(scope)?.try_as_usize()?; let length = length.eval_value_with_loc(scope)?.try_as_usize()?; - let mut vs = Vec::new(); - match values { - ValueSeq::ValueSeq(vs0) => { - for i in 0..length { - if i + start < vs0.len() { - vs.push(vs0[i + start].clone()); - } else { - vs.push(vs[i + start - vs0.len()].clone()); - } - } - } - ValueSeq::IntRange(range) => { - // REVIEW - double-check this logic - let len = range.len(); - let mut iter = range.skip(start); - for i in 0..length { - if let Some(val) = iter.next() { - vs.push(val.into()); - } else { - vs.push(vs[i + start - len].clone()); - } - } - } - } - Cow::Owned(ParsedValue::from_evaluated_seq(vs)) + Cow::Owned(ParsedValue::from_evaluated_seq(sub_seq_inflate( + values, start, length, + ))) } _ => panic!("SubSeqInflate: expected Seq"), } From 1fc7aca72306b1659d2f11a49ecdeb3af6bd1a0d Mon Sep 17 00:00:00 2001 From: Peter Duchovni Date: Tue, 22 Sep 2026 11:03:04 +1000 Subject: [PATCH 05/18] Unify Scope/LocScope as one generic scope module Scope<'a> (decoder.rs) and LocScope<'a> (loc_decoder.rs), along with their Multi/Single/Decoder/View companions, were two hand-maintained mirrors differing only in their leaf value type (Value vs. ParsedValue). Replace both with a single generic family in scope.rs: GScope<'a, V>, GMultiScope<'a, V>, GSingleScope<'a, V>, GDecoderScope<'a, V>, GViewScope<'a, V>, and a generic ScopeEntry, with Scope<'a>/MultiScope<'a>/... and LocScope<'a>/ LocMultiScope<'a>/... now type aliases instantiating V = Value / ParsedValue respectively. decoder.rs and loc_decoder.rs re-export the aliases so no external call site changes. This is Stage 2 of unifying Expr::eval/eval_with_loc (Stage 0: parity tests, 8f99b0a9; Stage 1: shared leaf logic, 5413d396). The one representational change: LocMultiScope's entries used to store ParsedValue directly, whereas MultiScope stored Cow. The generic type uses Cow uniformly, so LocMultiScope::push (which only ever took an owned ParsedValue) is now push_owned, matching MultiScope's existing push (borrowed) / push_owned (owned) split; the three call sites are updated accordingly. No behavior change: full workspace test suite (`cargo testall`), decode snapshot tests, and the eval_parity_tests differential harness all pass unchanged. Stage 3 (a single generic Expr::eval) is next. --- src/decoder.rs | 242 +-------------------------- src/decoder/eval_parity_tests.rs | 2 +- src/loc_decoder.rs | 251 ++-------------------------- src/scope.rs | 276 ++++++++++++++++++++++++++++++- 4 files changed, 291 insertions(+), 480 deletions(-) diff --git a/src/decoder.rs b/src/decoder.rs index 695df2c6..05d038ab 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -5,7 +5,7 @@ use std::rc::Rc; use anyhow::{Result as AResult, anyhow}; use crate::byte_set::ByteSet; -use crate::error::{DecodeError, DecodeErrorKind, EvalResultExt as _, UnknownVarError}; +use crate::error::{DecodeError, DecodeErrorKind, EvalResultExt as _}; use crate::fixed::{SpineElem, analyze_fixed_shape}; use crate::read::{BufferKind, ReadCtxt}; use crate::util::{EResult, WithErr}; @@ -961,242 +961,10 @@ impl<'a> Compiler<'a> { } } -#[derive(Clone, Debug)] -pub enum ScopeEntry { - Value(Value), - Decoder(Decoder), - View(usize), -} - -pub enum Scope<'a> { - Empty, - Multi(&'a MultiScope<'a>), - Single(SingleScope<'a>), - Decoder(DecoderScope<'a>), - View(ViewScope<'a>), -} - -#[derive(Clone)] -enum ViewOrValue<'a> { - View(View<'a>), - Value(Cow<'a, Value>), -} - -pub struct MultiScope<'a> { - parent: &'a Scope<'a>, - entries: Vec<(Label, ViewOrValue<'a>)>, -} - -pub struct SingleScope<'a> { - parent: &'a Scope<'a>, - name: &'a str, - value: &'a Value, -} - -pub struct DecoderScope<'a> { - parent: &'a Scope<'a>, - name: &'a str, - decoder: Decoder, -} - -pub struct ViewScope<'a> { - parent: &'a Scope<'a>, - name: &'a str, - view: View<'a>, -} - -// REVIEW - do we want a specialized type for holding views? -pub type View<'a> = ReadCtxt<'a>; - -impl<'a> Scope<'a> { - pub(crate) fn get_value_by_name(&self, name: &str) -> Result<&Value, UnknownVarError> { - match self { - Scope::Empty => Err(UnknownVarError(Label::Owned(name.to_string()))), - Scope::Multi(multi) => multi.get_value_by_name(name), - Scope::Single(single) => single.get_value_by_name(name), - Scope::Decoder(decoder) => decoder.parent.get_value_by_name(name), - Scope::View(view) => view.parent.get_value_by_name(name), - } - } - - fn get_decoder_by_name(&self, name: &str) -> &Decoder { - match self { - Scope::Empty => panic!("decoder not found: {name}"), - Scope::Multi(multi) => multi.parent.get_decoder_by_name(name), - Scope::Single(single) => single.parent.get_decoder_by_name(name), - Scope::Decoder(decoder) => decoder.get_decoder_by_name(name), - Scope::View(view) => view.parent.get_decoder_by_name(name), - } - } - - fn get_view_by_name(&self, name: &str) -> View<'a> { - match self { - Scope::Empty => panic!("view not found: {name}"), - Scope::Multi(multi) => multi.get_view_by_name(name), - Scope::Single(single) => single.parent.get_view_by_name(name), - Scope::Decoder(decoder) => decoder.parent.get_view_by_name(name), - Scope::View(view) => view.get_view_by_name(name), - } - } - - pub fn get_bindings(&self, bindings: &mut Vec<(Label, ScopeEntry)>) { - match self { - Scope::Empty => {} - Scope::Multi(multi) => multi.get_bindings(bindings), - Scope::Single(single) => single.get_bindings(bindings), - Scope::Decoder(decoder) => decoder.get_bindings(bindings), - Scope::View(view) => view.get_bindings(bindings), - } - } -} - -impl<'a> MultiScope<'a> { - fn new(parent: &'a Scope<'a>) -> MultiScope<'a> { - let entries = Vec::new(); - MultiScope { parent, entries } - } - - pub fn with_capacity(parent: &'a Scope<'a>, capacity: usize) -> MultiScope<'a> { - let entries = Vec::with_capacity(capacity); - MultiScope { parent, entries } - } - - /// Pushes a new binding to the scope using a borrow that lives at least as long as the scope itself - pub fn push(&mut self, name: impl Into