diff --git a/crates/message-format-compiler/src/compile/tests.rs b/crates/message-format-compiler/src/compile/tests.rs index 1fd696e..adf6396 100644 --- a/crates/message-format-compiler/src/compile/tests.rs +++ b/crates/message-format-compiler/src/compile/tests.rs @@ -166,8 +166,9 @@ fn markup_option_variables_are_canonicalized_like_other_variables() { Value::Str("https://example.test".to_string()), )]; let mut sink = MarkupOptionSink::default(); - let diagnostics = formatter - .format_to(message, &args, &mut sink) + let mut diagnostics = vec![]; + formatter + .format_to(message, &args, &mut sink, Some(&mut diagnostics)) .expect("format"); assert!(diagnostics.is_empty()); diff --git a/crates/message-format-compiler/src/test_support.rs b/crates/message-format-compiler/src/test_support.rs index fcc7536..9cd3d4e 100644 --- a/crates/message-format-compiler/src/test_support.rs +++ b/crates/message-format-compiler/src/test_support.rs @@ -46,7 +46,7 @@ impl FormatterTestExt for Formatter<'_, H> { ) -> Result { let message = self.resolve(message_id)?; let mut sink = OutputStringSink::default(); - let _diagnostics = self.format_to(message, args, &mut sink)?; + self.format_to(message, args, &mut sink, None)?; Ok(sink.out) } } diff --git a/crates/message-format-conformance/src/runtime_helpers.rs b/crates/message-format-conformance/src/runtime_helpers.rs index 1ae4ebd..892bc18 100644 --- a/crates/message-format-conformance/src/runtime_helpers.rs +++ b/crates/message-format-conformance/src/runtime_helpers.rs @@ -15,7 +15,7 @@ pub(crate) fn format( args: &dyn Args, ) -> Result { let mut out = String::new(); - let _diagnostics = formatter.format_to(message, args, &mut out)?; + formatter.format_to(message, args, &mut out, None)?; Ok(out) } @@ -35,7 +35,8 @@ pub(crate) fn format_with_diagnostics_by_id( ) -> Result { let message = formatter.resolve(message_id)?; let mut out = String::new(); - let errors = formatter.format_to(message, args, &mut out)?; + let mut errors = vec![]; + formatter.format_to(message, args, &mut out, Some(&mut errors))?; Ok(FormatOutput { value: out, errors }) } @@ -47,5 +48,7 @@ pub(crate) fn format_to_by_id( sink: &mut dyn message_format::runtime::FormatSink, ) -> Result, FormatError> { let message = formatter.resolve(message_id)?; - formatter.format_to(message, args, sink) + let mut errors = vec![]; + formatter.format_to(message, args, sink, Some(&mut errors))?; + Ok(errors) } diff --git a/crates/message-format-runtime/src/formatter.rs b/crates/message-format-runtime/src/formatter.rs index 390e63d..94fa111 100644 --- a/crates/message-format-runtime/src/formatter.rs +++ b/crates/message-format-runtime/src/formatter.rs @@ -10,7 +10,7 @@ use crate::{ catalog::Catalog, error::{FormatError, Trap}, value::{Args, MessageArgs, Value}, - vm::{FormatSink, Host, MessageHandle, VecDiagnostics, run_bytecode}, + vm::{DiagnosticsSink, FormatSink, Host, MessageHandle, run_bytecode}, }; #[derive(Default)] @@ -43,7 +43,7 @@ pub(crate) struct VmState { /// } /// let mut out = String::new(); /// let mut sink = StringSink(&mut out); -/// let _errors = formatter.format_to(message, &[], &mut sink)?; +/// formatter.format_to(message, &[], &mut sink, None)?; /// # Ok(out) /// # } /// ``` @@ -97,8 +97,8 @@ impl<'a, H: Host> Formatter<'a, H> { /// Format one message from a previously resolved handle, dispatching events to a [`FormatSink`]. /// - /// Returns recoverable formatting diagnostics collected during fallback - /// rendering. Fatal execution failures are returned as `Err`. + /// Recoverable formatting diagnostics are collected into [`diagnostics`](DiagnosticsSink) + /// during fallback rendering. Fatal execution failures are returned as `Err`. /// /// This is the runtime API that preserves structured markup. In contrast, /// string-oriented convenience helpers flatten only literal/expression text @@ -108,10 +108,10 @@ impl<'a, H: Host> Formatter<'a, H> { message: MessageHandle, args: &dyn Args, sink: &mut S, - ) -> Result, FormatError> { + diagnostics: Option<&mut dyn DiagnosticsSink>, + ) -> Result<(), FormatError> { #[cfg(feature = "profiling")] profiling::function_scope!(); - let mut diagnostics = VecDiagnostics::default(); run_bytecode( self.catalog, &mut self.host, @@ -121,11 +121,11 @@ impl<'a, H: Host> Formatter<'a, H> { self.vm.fuel, &mut self.vm.stack, sink, - Some(&mut diagnostics), + diagnostics, &mut self.vm.call_args, &mut self.vm.call_options, )?; - Ok(diagnostics.into_inner()) + Ok(()) } } @@ -251,6 +251,9 @@ impl<'a, H: Host> MultiFormatter<'a, H> { /// Format one message, dispatching events to a [`FormatSink`]. /// + /// Recoverable formatting diagnostics are collected into [`diagnostics`](DiagnosticsSink) + /// during fallback rendering. Fatal execution failures are returned as `Err`. + /// /// The handle identifies both the catalog and the message entry point. /// /// # Args safety @@ -270,14 +273,14 @@ impl<'a, H: Host> MultiFormatter<'a, H> { message: MultiMessageHandle, args: &dyn Args, sink: &mut S, - ) -> Result, FormatError> { + diagnostics: Option<&mut dyn DiagnosticsSink>, + ) -> Result<(), FormatError> { #[cfg(feature = "profiling")] profiling::function_scope!(); let (catalog, index) = self .catalogs .get(message.catalog_idx as usize) .ok_or(FormatError::Trap(Trap::InvalidCatalogIndex))?; - let mut diagnostics = VecDiagnostics::default(); run_bytecode( catalog, &mut self.host, @@ -287,11 +290,11 @@ impl<'a, H: Host> MultiFormatter<'a, H> { self.vm.fuel, &mut self.vm.stack, sink, - Some(&mut diagnostics), + diagnostics, &mut self.vm.call_args, &mut self.vm.call_options, )?; - Ok(diagnostics.into_inner()) + Ok(()) } } @@ -397,7 +400,7 @@ mod tests { let mut mf = mf; let mut sink = TestStringSink::default(); assert_eq!( - mf.format_to(bad_handle, &vec![] as &Vec<(u32, Value)>, &mut sink) + mf.format_to(bad_handle, &vec![] as &Vec<(u32, Value)>, &mut sink, None) .unwrap_err(), FormatError::Trap(Trap::InvalidCatalogIndex) ); @@ -426,7 +429,9 @@ mod tests { args.insert("who", "world").expect("arg interned in cat2"); let mut sink = TestStringSink::default(); - let diagnostics = mf.format_to(handle, &args, &mut sink).unwrap(); + let mut diagnostics = vec![]; + mf.format_to(handle, &args, &mut sink, Some(&mut diagnostics)) + .unwrap(); assert!(diagnostics.is_empty()); assert_eq!(sink.out, "world"); } diff --git a/crates/message-format-runtime/src/lib.rs b/crates/message-format-runtime/src/lib.rs index a6b85d4..d16f36d 100644 --- a/crates/message-format-runtime/src/lib.rs +++ b/crates/message-format-runtime/src/lib.rs @@ -64,7 +64,7 @@ //! .expect("catalog defines `name`"); //! let mut out = String::new(); //! let mut sink = StringSink(&mut out); -//! let _errors = formatter.format_to(message, &args, &mut sink)?; +//! formatter.format_to(message, &args, &mut sink, None)?; //! # Ok(out) //! # } //! ``` @@ -149,7 +149,8 @@ //! args.insert("url", "https://example.com") //! .expect("catalog defines `url`"); //! let mut sink = CollectingSink::default(); -//! let errors = formatter.format_to(message, &args, &mut sink)?; +//! let mut errors = vec![]; +//! formatter.format_to(message, &args, &mut sink, Some(&mut errors))?; //! assert!(errors.is_empty()); //! # Ok(sink.events) //! # } diff --git a/crates/message-format-runtime/src/vm.rs b/crates/message-format-runtime/src/vm.rs index e46437f..2080faa 100644 --- a/crates/message-format-runtime/src/vm.rs +++ b/crates/message-format-runtime/src/vm.rs @@ -237,24 +237,18 @@ pub struct FormatOption<'a> { pub value: Cow<'a, str>, } -pub(crate) trait DiagnosticsSink { +/// Sink for runtime error diagnostics. +/// +/// Implement this trait to receive [`FormatError`] events. \ +/// Already implemented for `Vec`. +pub trait DiagnosticsSink { + /// Record an error into the diagnostics sink. fn record(&mut self, error: FormatError); } -#[derive(Default)] -pub(crate) struct VecDiagnostics { - errors: Vec, -} - -impl VecDiagnostics { - pub(crate) fn into_inner(self) -> Vec { - self.errors - } -} - -impl DiagnosticsSink for VecDiagnostics { +impl DiagnosticsSink for Vec { fn record(&mut self, error: FormatError) { - self.errors.push(error); + self.push(error); } } @@ -267,10 +261,21 @@ enum SelectorValue<'a> { Owned(Value), } -#[derive(Default)] +enum ExprStatePendingErrors { + /// Record the minimum amount of information required + Minimal { + /// Were there any errors? + /// + /// This is important for [`ExprState::should_skip_call`]. + any: bool, + }, + /// Record all errors + All(Vec), +} + struct ExprState { fallback_id: Option, - pending_errors: Vec, + pending_errors: ExprStatePendingErrors, } impl<'a> SelectorValue<'a> { @@ -333,8 +338,21 @@ impl<'a> SelectorValue<'a> { } impl ExprState { + fn new(diagnostics: &Option<&mut dyn DiagnosticsSink>) -> Self { + Self { + fallback_id: None, + pending_errors: match diagnostics { + Some(_) => ExprStatePendingErrors::All(Vec::new()), + None => ExprStatePendingErrors::Minimal { any: false }, + }, + } + } + fn record_error(&mut self, error: FormatError) { - self.pending_errors.push(error); + match &mut self.pending_errors { + ExprStatePendingErrors::Minimal { any } => *any = true, + ExprStatePendingErrors::All(errors) => errors.push(error), + } } fn set_fallback(&mut self, fallback_id: u32) { @@ -342,7 +360,10 @@ impl ExprState { } fn should_skip_call(&self) -> bool { - !self.pending_errors.is_empty() + match &self.pending_errors { + ExprStatePendingErrors::Minimal { any } => *any, + ExprStatePendingErrors::All(errors) => !errors.is_empty(), + } } fn clear_fallback(&mut self) { @@ -361,12 +382,23 @@ impl ExprState { push_expr_fallback(stack, catalog, self.fallback_id.take()) } - fn take_pending_errors(&mut self) -> Vec { - core::mem::take(&mut self.pending_errors) + fn take_pending_errors(&mut self) -> Option> { + match &mut self.pending_errors { + ExprStatePendingErrors::Minimal { any } => { + *any = false; + None + } + ExprStatePendingErrors::All(errors) => Some(core::mem::take(errors)), + } } fn clear_pending_errors(&mut self) { - self.pending_errors.clear(); + match &mut self.pending_errors { + ExprStatePendingErrors::Minimal { any } => { + *any = false; + } + ExprStatePendingErrors::All(errors) => errors.clear(), + } } } @@ -390,7 +422,7 @@ where let mut pc = entry_pc; stack.clear(); let mut selector: Option> = None; - let mut expr_state = ExprState::default(); + let mut expr_state = ExprState::new(&diagnostics); let mut remaining_fuel = fuel; loop { @@ -761,21 +793,21 @@ fn handle_call_instruction( // skip the call and use the expression fallback (e.g. `{$varname}`) per // TR35 ยง16. if expr_state.should_skip_call() { - let pending_errors = expr_state.take_pending_errors(); - if opcode == Opcode::CallSelect { + if let Some(pending_errors) = expr_state.take_pending_errors() { let mut pending_errors = pending_errors.into_iter(); - record_bad_selector(diagnostics, pending_errors.next()); + if opcode == Opcode::CallSelect { + record_bad_selector(diagnostics, pending_errors.next()); + } for error in pending_errors { record_diagnostic(diagnostics, error); } + } + if opcode == Opcode::CallSelect { expr_state.clear_fallback(); stack.push(Value::Null); - return Ok(()); - } - for error in pending_errors { - record_diagnostic(diagnostics, error); + } else { + expr_state.push_fallback(stack, catalog)?; } - expr_state.push_fallback(stack, catalog)?; return Ok(()); } @@ -1317,7 +1349,7 @@ mod tests { ) -> Result { let message = self.resolve(message_id)?; let mut sink = TestStringSink::default(); - let _diagnostics = self.format_to(message, args, &mut sink)?; + self.format_to(message, args, &mut sink, None)?; Ok(sink.out) } @@ -1328,7 +1360,9 @@ mod tests { sink: &mut dyn FormatSink, ) -> Result, FormatError> { let message = self.resolve(message_id)?; - self.format_to(message, args, sink) + let mut errors = vec![]; + self.format_to(message, args, sink, Some(&mut errors))?; + Ok(errors) } } @@ -1566,8 +1600,8 @@ mod tests { .format_by_id_for_test("main", &args) .expect("format"); let mut sink = TestStringSink::default(); - let _diagnostics = formatter - .format_to(handle, &args, &mut sink) + formatter + .format_to(handle, &args, &mut sink, None) .expect("format"); let resolved = sink.out; assert_eq!(direct, resolved); diff --git a/crates/message-format/src/formatter.rs b/crates/message-format/src/formatter.rs index 762dbad..dcddde5 100644 --- a/crates/message-format/src/formatter.rs +++ b/crates/message-format/src/formatter.rs @@ -104,7 +104,7 @@ impl<'a> MessageFormatter<'a> { out.clear(); let catalog = self.inner.catalog_for(message)?; let resolved = args.resolve(catalog); - let _diagnostics = self.inner.format_to(message, &resolved, out)?; + self.inner.format_to(message, &resolved, out, None)?; Ok(()) } diff --git a/wind_tunnel/benches/formatting.rs b/wind_tunnel/benches/formatting.rs index 87cef49..1d64fe4 100644 --- a/wind_tunnel/benches/formatting.rs +++ b/wind_tunnel/benches/formatting.rs @@ -53,7 +53,7 @@ trait RuntimeFormatExt { message_id: &str, args: &dyn Args, sink: &mut S, - ) -> Result, FormatError>; + ) -> Result<(), FormatError>; } impl RuntimeFormatExt for Formatter<'_, H> { @@ -63,7 +63,7 @@ impl RuntimeFormatExt for Formatter<'_, H> { args: &dyn Args, ) -> Result { let mut sink = BenchStringSink::default(); - let _diagnostics = self.format_to(message, args, &mut sink)?; + self.format_to(message, args, &mut sink, None)?; Ok(sink.out) } @@ -81,9 +81,10 @@ impl RuntimeFormatExt for Formatter<'_, H> { message_id: &str, args: &dyn Args, sink: &mut S, - ) -> Result, FormatError> { + ) -> Result<(), FormatError> { let message = self.resolve(message_id)?; - self.format_to(message, args, sink) + self.format_to(message, args, sink, None)?; + Ok(()) } } @@ -948,10 +949,10 @@ fn bench_formatting(c: &mut Criterion) { let mut sink = CountingSink::default(); b.iter(|| { sink.reset(); - let errors = formatter + formatter .format_to_by_id_for_bench("main", black_box(&markup_args), &mut sink) .expect("format_to"); - black_box((&errors, sink.events, sink.bytes)); + black_box((sink.events, sink.bytes)); }); }); @@ -962,10 +963,10 @@ fn bench_formatting(c: &mut Criterion) { let empty_args: Vec<(u32, Value)> = Vec::new(); b.iter(|| { sink.reset(); - let errors = formatter + formatter .format_to_by_id_for_bench("main", black_box(&empty_args), &mut sink) .expect("format_to"); - black_box((&errors, sink.events, sink.bytes)); + black_box((sink.events, sink.bytes)); }); }); @@ -979,10 +980,10 @@ fn bench_formatting(c: &mut Criterion) { let mut sink = CountingSink::default(); b.iter(|| { sink.reset(); - let errors = formatter + formatter .format_to_by_id_for_bench("main", black_box(&markup_option_args), &mut sink) .expect("format_to"); - black_box((&errors, sink.events, sink.bytes)); + black_box((sink.events, sink.bytes)); }); }); sink_group.finish(); diff --git a/wind_tunnel/src/bin/profile_select.rs b/wind_tunnel/src/bin/profile_select.rs index 4afe8fa..6881318 100644 --- a/wind_tunnel/src/bin/profile_select.rs +++ b/wind_tunnel/src/bin/profile_select.rs @@ -220,8 +220,8 @@ fn main() { for _ in 0..iters { out.clear(); let mut sink = OutputStringSink { out: &mut out }; - let _diagnostics = formatter - .format_to(message, &args, &mut sink) + formatter + .format_to(message, &args, &mut sink, None) .expect("format"); checksum = checksum.wrapping_add(black_box(out.len())); } @@ -239,8 +239,8 @@ fn main() { for _ in 0..iters { out.clear(); let mut sink = OutputStringSink { out: &mut out }; - let _diagnostics = formatter - .format_to(message, &args, &mut sink) + formatter + .format_to(message, &args, &mut sink, None) .expect("format"); checksum = checksum.wrapping_add(black_box(out.len())); } @@ -258,8 +258,8 @@ fn main() { for _ in 0..iters { out.clear(); let mut sink = OutputStringSink { out: &mut out }; - let _diagnostics = formatter - .format_to(message, &args, &mut sink) + formatter + .format_to(message, &args, &mut sink, None) .expect("format"); checksum = checksum.wrapping_add(black_box(out.len())); } @@ -280,8 +280,8 @@ fn main() { for _ in 0..iters { out.clear(); let mut sink = OutputStringSink { out: &mut out }; - let _diagnostics = formatter - .format_to(message, &args, &mut sink) + formatter + .format_to(message, &args, &mut sink, None) .expect("format"); checksum = checksum.wrapping_add(black_box(out.len())); }