From 99906f43d622ed2ae7aac5983e1b344db538ef74 Mon Sep 17 00:00:00 2001 From: Bruce Mitchener Date: Wed, 16 Sep 2026 13:02:10 +0700 Subject: [PATCH] Simplify structured output metadata --- .../src/compiler/compile/tests.rs | 23 ++ crates/message-format/src/runtime/builtin.rs | 89 ++---- .../message-format/src/runtime/formatter.rs | 2 +- crates/message-format/src/runtime/value.rs | 32 +- crates/message-format/src/runtime/vm.rs | 290 +++++++----------- wind_tunnel/benches/formatting.rs | 38 +++ 6 files changed, 208 insertions(+), 266 deletions(-) diff --git a/crates/message-format/src/compiler/compile/tests.rs b/crates/message-format/src/compiler/compile/tests.rs index 76a63fd..b9b640d 100644 --- a/crates/message-format/src/compiler/compile/tests.rs +++ b/crates/message-format/src/compiler/compile/tests.rs @@ -2775,6 +2775,29 @@ fn optionless_string_reannotation_retains_inherited_universal_id() { assert_eq!(sink.ids, ["first"]); } +#[test] +fn explicit_universal_id_overrides_inherited_id() { + let source = ".local $x={foo :custom u:id=first} {{{$x :custom u:id=second}}}"; + let bytes = compile_str(source).expect("compiled"); + let catalog = Catalog::from_bytes(&bytes).expect("catalog"); + let host = HostFn(|_fn_id, args: &[Value], _opts| { + Ok(args + .first() + .cloned() + .unwrap_or_else(|| Value::Str("result".to_string()))) + }); + let mut formatter = Formatter::new(&catalog, host).expect("formatter"); + let message = formatter.resolve("main").expect("message"); + let mut sink = UniversalIdSink::default(); + + formatter + .format_to(message, &[], &mut sink, None) + .expect("formatted"); + + assert_eq!(sink.output, "foo"); + assert_eq!(sink.ids, ["second"]); +} + #[test] fn universal_id_does_not_change_dynamic_option_values_seen_by_custom_hosts() { let source = ".local $o={a :custom u:id=opt} {{{a :custom option=$o}}}"; diff --git a/crates/message-format/src/runtime/builtin.rs b/crates/message-format/src/runtime/builtin.rs index d9e612b..e771398 100644 --- a/crates/message-format/src/runtime/builtin.rs +++ b/crates/message-format/src/runtime/builtin.rs @@ -751,10 +751,7 @@ impl Host for BuiltinHost { kind: value.kind, value: Cow::Borrowed(value.text()), locale: Some(Cow::Owned(self.locale.to_string())), - id: value - .id - .as_deref() - .or_else(|| value.selection.as_ref()?.id.as_deref()), + id: None, direction: None, fields: &[], }); @@ -781,7 +778,7 @@ impl Host for BuiltinHost { kind: FormattedValueKind::Number, value: Cow::Borrowed(formatted.as_str()), locale: Some(Cow::Owned(self.locale.to_string())), - id: number.id.as_deref(), + id: None, direction: None, fields, }); @@ -823,7 +820,6 @@ fn plain_text<'a>(catalog: &'a Catalog, value: &'a Value) -> Cow<'a, str> { Value::Int(v) => Cow::Owned(v.to_string()), Value::Float(v) => Cow::Owned(v.to_string()), Value::Str(v) => Cow::Borrowed(v.as_str()), - Value::Identified { value, .. } => plain_text(catalog, value), Value::String(v) => Cow::Borrowed(v.text()), Value::StrRef(id) => catalog .pool_string_opt(*id) @@ -872,7 +868,7 @@ fn format_string( && dir.as_deref() == Some("\0inherit") { // Compiler-generated default isolation preserves explicit direction - // and identity already established by a stored resolved string. + // already established by a stored resolved string. let mut resolved = value.clone(); if resolved.direction == StringDirection::Unspecified { resolved.direction = StringDirection::Auto; @@ -886,22 +882,15 @@ fn format_string( Some("auto" | "\0inherit") => StringDirection::Auto, Some(_) => StringDirection::Unspecified, }; - let id = options - .get(BuiltinOptionKey::UId) - .map(|value| value.into_owned().into_boxed_str()); if let Value::Int(value) = value { let text = format_i64(*value); - let mut resolved = ResolvedString::from_integer(text.as_str(), *value, direction); - resolved.id = id; - return resolved; + return ResolvedString::from_integer(text.as_str(), *value, direction); } let text = plain_text(catalog, value); - let mut resolved = match text { + match text { Cow::Borrowed(text) => ResolvedString::from_borrowed(text, direction), Cow::Owned(text) => ResolvedString::from_owned(text, direction), - }; - resolved.id = id; - resolved + } } fn value_text<'a>(catalog: &'a Catalog, value: &'a Value) -> Option<&'a str> { @@ -937,23 +926,20 @@ fn resolve_number( on_error: &mut dyn FnMut(MessageFunctionError), ) -> Result { let value = numeric_source(value); - let (mut number, inherited_format, inherited_selection, inherited_select, inherited_id) = - match value { - Value::Number(number) => ( - number.value.clone(), - number.format, - number.selection, - number.has_explicit_select, - number.id.clone(), - ), - _ => ( - parse_number_value(value, catalog)?, - NumberFormatOptions::DEFAULT, - NumberSelection::None, - false, - None, - ), - }; + let (mut number, inherited_format, inherited_selection, inherited_select) = match value { + Value::Number(number) => ( + number.value.clone(), + number.format, + number.selection, + number.has_explicit_select, + ), + _ => ( + parse_number_value(value, catalog)?, + NumberFormatOptions::DEFAULT, + NumberSelection::None, + false, + ), + }; if integer_only { // Integer annotations intentionally discard precision inherited from a // preceding number annotation. The integer function itself emits no @@ -1001,12 +987,12 @@ fn resolve_number( selection => selection, } }; - let mut resolved = ResolvedNumber::new(number, format, selection, has_explicit_select); - resolved.id = options - .get(BuiltinOptionKey::UId) - .map(|value| value.into_owned().into_boxed_str()) - .or(inherited_id); - Ok(resolved) + Ok(ResolvedNumber::new( + number, + format, + selection, + has_explicit_select, + )) } fn resolve_percent( @@ -1014,21 +1000,12 @@ fn resolve_percent( catalog: &Catalog, options: &EffectiveOptions<'_>, ) -> Result<(ResolvedNumber, ResolvedNumber), FormatError> { - let retained_id = match value { - Value::Formatted(formatted) => formatted.id.clone(), - _ => None, - }; let value = numeric_source(value); - let (number, inherited_format, inherited_id) = match value { - Value::Number(number) => ( - number.value.clone(), - number.format, - retained_id.or_else(|| number.id.clone()), - ), + let (number, inherited_format) = match value { + Value::Number(number) => (number.value.clone(), number.format), _ => ( parse_number_value(value, catalog)?, NumberFormatOptions::DEFAULT, - retained_id, ), }; let text = match &number { @@ -1055,14 +1032,8 @@ fn resolve_percent( if format.maximum_fraction_digits.is_none() { format.maximum_fraction_digits = format.minimum_fraction_digits; } - let id = options - .get(BuiltinOptionKey::UId) - .map(|value| value.into_owned().into_boxed_str()) - .or(inherited_id); - let mut source = ResolvedNumber::new(number, format, NumberSelection::Plural, false); - source.id.clone_from(&id); - let mut selection = ResolvedNumber::new(scaled, format, NumberSelection::Plural, false); - selection.id = id; + let source = ResolvedNumber::new(number, format, NumberSelection::Plural, false); + let selection = ResolvedNumber::new(scaled, format, NumberSelection::Plural, false); Ok((source, selection)) } diff --git a/crates/message-format/src/runtime/formatter.rs b/crates/message-format/src/runtime/formatter.rs index d7f97bf..fea1344 100644 --- a/crates/message-format/src/runtime/formatter.rs +++ b/crates/message-format/src/runtime/formatter.rs @@ -16,7 +16,7 @@ use crate::runtime::{ #[derive(Default)] pub(crate) struct VmState { pub(crate) fuel: Option, - pub(crate) values: Vec, + pub(crate) values: Vec, pub(crate) stack: Vec, pub(crate) locals: Vec, pub(crate) call_args: Vec, diff --git a/crates/message-format/src/runtime/value.rs b/crates/message-format/src/runtime/value.rs index f2b3abf..8ecc9ba 100644 --- a/crates/message-format/src/runtime/value.rs +++ b/crates/message-format/src/runtime/value.rs @@ -31,13 +31,6 @@ pub enum Value { Float(f64), /// Owned UTF-8 string. Str(String), - /// A value carrying the universal `u:id` formatting annotation. - Identified { - /// Underlying function result. - value: Box, - /// User-provided identifier. - id: String, - }, /// String resolved by the `string` function, retaining direction metadata. String(ResolvedString), /// Reference to a catalog string-pool entry. @@ -83,7 +76,6 @@ pub struct ResolvedFormatted { pub(crate) formatted: String, pub(crate) kind: super::vm::FormattedValueKind, pub(crate) selection: Option, - pub(crate) id: Option>, } #[cfg(feature = "icu4x")] @@ -94,7 +86,6 @@ impl ResolvedFormatted { formatted, kind: super::vm::FormattedValueKind::String, selection: None, - id: None, } } @@ -104,7 +95,6 @@ impl ResolvedFormatted { formatted, kind: super::vm::FormattedValueKind::Number, selection: None, - id: None, } } @@ -114,7 +104,6 @@ impl ResolvedFormatted { formatted, kind: super::vm::FormattedValueKind::Number, selection: Some(selection), - id: None, } } @@ -127,11 +116,7 @@ impl ResolvedFormatted { impl Value { pub(crate) fn is_fallback(&self) -> bool { - match self { - Self::Fallback(_) | Self::FunctionFallback(_) => true, - Self::Identified { value, .. } => value.is_fallback(), - _ => false, - } + matches!(self, Self::Fallback(_) | Self::FunctionFallback(_)) } } @@ -142,8 +127,6 @@ pub struct ResolvedString { text: ResolvedStringText, /// Direction requested by the string function. pub(crate) direction: StringDirection, - /// User-provided `u:id`, when present. - pub(crate) id: Option>, } const INLINE_STRING_CAPACITY: usize = 24; @@ -199,11 +182,7 @@ impl ResolvedString { } else { ResolvedStringText::Heap(text.into()) }; - Self { - text, - direction, - id: None, - } + Self { text, direction } } pub(crate) fn from_owned(text: String, direction: StringDirection) -> Self { @@ -213,7 +192,6 @@ impl ResolvedString { Self { text: ResolvedStringText::Heap(text.into_boxed_str()), direction, - id: None, } } @@ -228,7 +206,6 @@ impl ResolvedString { bytes, }, direction, - id: None, } } @@ -265,7 +242,7 @@ impl ResolvedString { impl PartialEq for ResolvedString { fn eq(&self, other: &Self) -> bool { - self.direction == other.direction && self.id == other.id && self.text() == other.text() + self.direction == other.direction && self.text() == other.text() } } @@ -303,8 +280,6 @@ pub struct ResolvedNumber { pub(crate) selection: NumberSelection, /// Whether a `select` option was explicitly resolved for this value. pub(crate) has_explicit_select: bool, - /// User-provided `u:id`, when present. - pub(crate) id: Option>, } /// Exact numeric payload retained by [`ResolvedNumber`]. @@ -393,7 +368,6 @@ impl ResolvedNumber { format, selection, has_explicit_select, - id: None, } } diff --git a/crates/message-format/src/runtime/vm.rs b/crates/message-format/src/runtime/vm.rs index 23e0498..4f2a553 100644 --- a/crates/message-format/src/runtime/vm.rs +++ b/crates/message-format/src/runtime/vm.rs @@ -24,18 +24,32 @@ use crate::runtime::{ value::{Args, ResolvedString, StrId, StringDirection, Value}, }; -fn store_value(values: &mut Vec, value: Value) -> usize { - let id = values.len(); - values.push(value); - id +#[derive(Debug)] +pub(crate) struct StoredValue { + value: Value, + id: Option>, +} + +fn store_value(values: &mut Vec, value: Value) -> usize { + store_value_with_id(values, value, None) } -fn stored_value(values: &[Value], id: usize) -> Result<&Value, FormatError> { +fn store_value_with_id(values: &mut Vec, value: Value, id: Option>) -> usize { + let index = values.len(); + values.push(StoredValue { value, id }); + index +} + +fn stored(values: &[StoredValue], id: usize) -> Result<&StoredValue, FormatError> { values .get(id) .ok_or(FormatError::Trap(Trap::InvalidValueIndex)) } +fn stored_value(values: &[StoredValue], id: usize) -> Result<&Value, FormatError> { + Ok(&stored(values, id)?.value) +} + /// Resolved message handle for repeated formatting. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct MessageHandle { @@ -118,7 +132,7 @@ impl<'a> Iterator for FunctionOptionsIter<'a> { fn next(&mut self) -> Option { for (key, value) in self.inner.by_ref() { if !value.is_fallback() { - return Some((*key, unwrapped_value(value))); + return Some((*key, value)); } } None @@ -561,72 +575,38 @@ impl FormatSink for String { fn markup_close(&mut self, _name: &str, _options: &[FormatOption<'_>]) {} } -struct SinkForward<'a, S: FormatSink + ?Sized>(&'a mut S); - -impl FormatSink for SinkForward<'_, S> { - fn wants_structured_output(&self) -> bool { - self.0.wants_structured_output() - } - fn literal(&mut self, s: &str) { - self.0.literal(s); - } - - fn expression(&mut self, s: &str) { - self.0.expression(s); - } - - fn markup_open(&mut self, name: &str, options: &[FormatOption<'_>]) { - self.0.markup_open(name, options); - } - - fn markup_close(&mut self, name: &str, options: &[FormatOption<'_>]) { - self.0.markup_close(name, options); - } - - fn formatted_value(&mut self, value: &FormattedValue<'_>) { - self.0.formatted_value(value); - } +struct SinkForward<'a, S: FormatSink + ?Sized> { + sink: &'a mut S, + id: Option<&'a str>, +} - fn bidi_isolation(&mut self, value: &str) { - self.0.bidi_isolation(value); +impl<'a, S: FormatSink + ?Sized> SinkForward<'a, S> { + fn new(sink: &'a mut S) -> Self { + Self { sink, id: None } } - fn fallback(&mut self, source: &str, rendered: &str) { - self.0.fallback(source, rendered); + fn identified(sink: &'a mut S, id: &'a str) -> Self { + Self { sink, id: Some(id) } } - - fn markup( - &mut self, - kind: MarkupKind, - name: &str, - id: Option<&str>, - options: &[FormatOption<'_>], - ) { - self.0.markup(kind, name, id, options); - } -} - -struct IdentifiedSink<'a, S: FormatSink + ?Sized> { - sink: &'a mut S, - id: &'a str, } -impl FormatSink for IdentifiedSink<'_, S> { +impl FormatSink for SinkForward<'_, S> { fn wants_structured_output(&self) -> bool { self.sink.wants_structured_output() } - - fn literal(&mut self, value: &str) { - self.sink.literal(value); + fn literal(&mut self, s: &str) { + self.sink.literal(s); } fn expression(&mut self, value: &str) { - if self.sink.wants_structured_output() { + if let Some(id) = self.id + && self.sink.wants_structured_output() + { self.sink.formatted_value(&FormattedValue { kind: FormattedValueKind::String, value: Cow::Borrowed(value), locale: None, - id: Some(self.id), + id: Some(id), direction: None, fields: &[], }); @@ -644,14 +624,18 @@ impl FormatSink for IdentifiedSink<'_, S> { } fn formatted_value(&mut self, value: &FormattedValue<'_>) { - self.sink.formatted_value(&FormattedValue { - kind: value.kind, - value: Cow::Borrowed(value.value.as_ref()), - locale: value.locale.as_deref().map(Cow::Borrowed), - id: Some(self.id), - direction: value.direction, - fields: value.fields, - }); + if let Some(id) = self.id { + self.sink.formatted_value(&FormattedValue { + kind: value.kind, + value: Cow::Borrowed(value.value.as_ref()), + locale: value.locale.as_deref().map(Cow::Borrowed), + id: Some(id), + direction: value.direction, + fields: value.fields, + }); + } else { + self.sink.formatted_value(value); + } } fn bidi_isolation(&mut self, value: &str) { @@ -792,7 +776,7 @@ impl<'a> SelectorValue<'a> { fn case_match( &self, - values: &[Value], + values: &[StoredValue], locals: &[usize], case_str_id: u32, catalog: &Catalog, @@ -832,13 +816,13 @@ impl<'a> SelectorValue<'a> { }) } - fn fast_str_id(&self, values: &[Value]) -> Option { + fn fast_str_id(&self, values: &[StoredValue]) -> Option { match self { Self::Borrowed { str_id, .. } => *str_id, Self::ExactText(_) => None, Self::Local(_) => None, Self::InvalidBorrowed => None, - Self::Stored(value_id) => match values.get(*value_id) { + Self::Stored(value_id) => match values.get(*value_id).map(|stored| &stored.value) { Some(Value::StrRef(id)) => Some(*id), _ => None, }, @@ -945,7 +929,7 @@ impl ExprState { fn finish_declaration( &mut self, value_id: usize, - values: &mut Vec, + values: &mut Vec, catalog: &Catalog, diagnostics: &mut Option<&mut dyn DiagnosticsSink>, ) -> Result { @@ -991,7 +975,7 @@ pub(crate) fn run_bytecode( entry_pc: u32, args: &dyn Args, fuel: Option, - values: &mut Vec, + values: &mut Vec, stack: &mut Vec, locals: &mut Vec, sink: &mut S, @@ -1087,7 +1071,8 @@ where &mut diagnostics, &mut expr_state, )?; - stack.push(store_value(values, resolved)); + let id = stored(values, value)?.id.clone(); + stack.push(store_value_with_id(values, resolved, id)); } Opcode::OutLit | Opcode::OutSlice @@ -1212,7 +1197,7 @@ fn handle_output_instruction( sink: &mut S, host: &mut H, index: &H::CatalogIndex, - values: &[Value], + values: &[StoredValue], stack: &mut Vec, catalog: &Catalog, args: &dyn Args, @@ -1240,7 +1225,15 @@ where } Opcode::OutVal => { let value = stack.pop().ok_or(FormatError::StackUnderflow)?; - emit_output_value(sink, host, index, catalog, stored_value(values, value)?); + let value = stored(values, value)?; + emit_output_value( + sink, + host, + index, + catalog, + &value.value, + value.id.as_deref(), + ); } Opcode::OutArg => { let key_id = read_u32(catalog.code(), base + 1)?; @@ -1254,7 +1247,7 @@ where fn handle_select_instruction<'a>( args: &'a dyn Args, - values: &[Value], + values: &[StoredValue], stack: &mut Vec, locals: &[usize], selector: &mut Option>, @@ -1394,7 +1387,7 @@ fn resolve_optionless_string( return expr_state.take_fallback(catalog); } - let mut resolved = if let Value::String(value) = value { + let resolved = if let Value::String(value) = value { value.clone() } else if let Value::Int(value) = value { let text = format_i64(*value); @@ -1406,9 +1399,6 @@ fn resolve_optionless_string( None => ResolvedString::plain_borrowed(""), } }; - if resolved.id.is_none() { - resolved.id = value_id(value).map(|id| id.to_string().into_boxed_str()); - } expr_state.clear_fallback(); Ok(Value::String(resolved)) } @@ -1431,7 +1421,7 @@ fn load_option_arg_value( } fn decode_call_args( - values: &[Value], + values: &[StoredValue], stack: &mut Vec, arg_count: usize, call_args: &mut Vec, @@ -1447,7 +1437,7 @@ fn decode_call_args( } fn decode_option_pairs( - values: &[Value], + values: &[StoredValue], stack: &mut Vec, catalog: &Catalog, optc: usize, @@ -1491,7 +1481,7 @@ fn resolve_markup_option_key(key: Value, _catalog: &Catalog) -> Result( host: &mut H, index: &H::CatalogIndex, - values: &mut Vec, + values: &mut Vec, stack: &mut Vec, catalog: &Catalog, opcode: Opcode, @@ -1515,7 +1505,17 @@ fn handle_call_opcode( call_options, resolve_call_option_key, )?; - let result = if arg_count == 1 { + let inherited_id = if structured_output && arg_count > 0 { + let first_arg = stack + .len() + .checked_sub(arg_count) + .and_then(|index| stack.get(index)) + .ok_or(FormatError::StackUnderflow)?; + stored(values, *first_arg)?.id.clone() + } else { + None + }; + let (result, id) = if arg_count == 1 { let arg = stack.pop().ok_or(FormatError::StackUnderflow)?; handle_call_instruction( host, @@ -1528,6 +1528,7 @@ fn handle_call_opcode( diagnostics, expr_state, structured_output, + inherited_id.as_deref(), )? } else { decode_call_args(values, stack, arg_count, call_args)?; @@ -1542,16 +1543,17 @@ fn handle_call_opcode( diagnostics, expr_state, structured_output, + inherited_id.as_deref(), )? }; - stack.push(store_value(values, result)); + stack.push(store_value_with_id(values, result, id)); Ok(()) } fn handle_project_select( host: &mut H, index: &H::CatalogIndex, - values: &mut Vec, + values: &mut Vec, stack: &mut Vec, catalog: &Catalog, base: usize, @@ -1560,7 +1562,7 @@ fn handle_project_select( ) -> Result<(), FormatError> { let fn_id = read_u16(code, base + 1)?; let value_id = stack.pop().ok_or(FormatError::StackUnderflow)?; - let value = unwrapped_value(stored_value(values, value_id)?); + let value = stored_value(values, value_id)?; let prechecked_invalid_number = { #[cfg(feature = "icu4x")] { @@ -1605,7 +1607,7 @@ fn handle_project_select( fn handle_markup_instruction( sink: &mut S, - values: &[Value], + values: &[StoredValue], stack: &mut Vec, catalog: &Catalog, opcode: Opcode, @@ -1651,7 +1653,8 @@ fn handle_call_instruction( diagnostics: &mut Option<&mut dyn DiagnosticsSink>, expr_state: &mut ExprState, structured_output: bool, -) -> Result { + inherited_id: Option<&str>, +) -> Result<(Value, Option>), FormatError> { // If a missing variable was loaded as an operand for this function call, // skip the call and use the expression fallback (e.g. `{$varname}`) per // TR35 ยง16. @@ -1680,70 +1683,35 @@ fn handle_call_instruction( } if opcode == Opcode::CallSelect { expr_state.clear_fallback(); - Ok(Value::Null) + Ok((Value::Null, None)) } else { - expr_state.take_fallback(catalog) + Ok((expr_state.take_fallback(catalog)?, None)) } } else { - let inherited_id = structured_output - .then(|| { - call_args - .first() - .and_then(value_id) - .map(ToString::to_string) - }) - .flatten(); - let unwrapped_args; - let host_args = if call_args - .iter() - .any(|value| matches!(value, Value::Identified { .. })) - { - unwrapped_args = call_args - .iter() - .map(|value| unwrapped_value(value).clone()) - .collect::>(); - unwrapped_args.as_slice() - } else { - call_args - }; let result = handle_resolved_call( host, index, opcode, fn_id, - host_args, + call_args, call_options, catalog, diagnostics, expr_state, )?; - if opcode == Opcode::CallFunc && structured_output { - Ok(attach_call_id( + let id = if opcode == Opcode::CallFunc + && structured_output + && !matches!( result, - resolved_call_id(fn_id, call_options, catalog).or(inherited_id), - )) + Value::Null | Value::Fallback(_) | Value::FunctionFallback(_) + ) { + resolved_call_id(fn_id, call_options, catalog) + .map(String::into_boxed_str) + .or_else(|| inherited_id.map(Into::into)) } else { - Ok(result) - } - } -} - -fn unwrapped_value(mut value: &Value) -> &Value { - while let Value::Identified { value: inner, .. } = value { - value = inner; - } - value -} - -fn value_id(value: &Value) -> Option<&str> { - match value { - Value::Identified { id, .. } => Some(id), - Value::String(value) => value.id.as_deref(), - #[cfg(feature = "icu4x")] - Value::Number(value) => value.id.as_deref(), - #[cfg(feature = "icu4x")] - Value::Formatted(value) => value.id.as_deref(), - _ => None, + None + }; + Ok((result, id)) } } @@ -1770,30 +1738,6 @@ fn resolved_call_id(fn_id: u16, options: &[(u32, Value)], catalog: &Catalog) -> }) } -fn attach_call_id(mut value: Value, id: Option) -> Value { - let Some(id) = id else { - return value; - }; - match &mut value { - Value::String(value) => value.id = Some(id.into_boxed_str()), - #[cfg(feature = "icu4x")] - Value::Number(value) => value.id = Some(id.into_boxed_str()), - #[cfg(feature = "icu4x")] - Value::Formatted(value) => value.id = Some(id.into_boxed_str()), - Value::Identified { - id: existing_id, .. - } => *existing_id = id, - Value::Null | Value::Fallback(_) | Value::FunctionFallback(_) => return value, - _ => { - return Value::Identified { - value: Box::new(value), - id, - }; - } - } - value -} - fn handle_resolved_call( host: &mut H, index: &H::CatalogIndex, @@ -1972,7 +1916,6 @@ impl<'a> ValueView<'a> { Value::Int(v) => Some(Self::Int(*v)), Value::Float(v) => Some(Self::Float(*v)), Value::Str(v) => Some(Self::Text(v)), - Value::Identified { value, .. } => Self::from_value(value, catalog), Value::String(v) => Some(Self::ExactText(v.text())), Value::StrRef(id) => catalog.pool_string_opt(*id).map(Self::Text), Value::Fallback(id) => catalog.pool_string_opt(*id).map(Self::Fallback), @@ -2214,7 +2157,7 @@ fn emit_value_ref(sink: &mut S, catalog: &Catalog, value kind: value.kind, value: Cow::Borrowed(value.text()), locale: None, - id: value.id.as_deref(), + id: None, direction: None, fields: &[], }); @@ -2226,7 +2169,7 @@ fn emit_value_ref(sink: &mut S, catalog: &Catalog, value kind: FormattedValueKind::Number, value: Cow::Owned(number.text()), locale: None, - id: number.id.as_deref(), + id: None, direction: None, fields: &[], }); @@ -2262,7 +2205,7 @@ pub(crate) fn emit_resolved_string<'a, S: FormatSink + ?Sized>( kind: FormattedValueKind::String, value: Cow::Borrowed(value.text()), locale, - id: value.id.as_deref(), + id: None, direction, fields: &[], }); @@ -2280,21 +2223,14 @@ fn emit_output_value( index: &H::CatalogIndex, catalog: &Catalog, value: &Value, + id: Option<&str>, ) { - if let Value::Identified { value, id } = value { - let mut identified = IdentifiedSink { - sink, - id: id.as_str(), - }; - let mut forwarded = SinkForward(&mut identified); - if !host.format_default_to(catalog, index, value, &mut forwarded) { - emit_value_ref(&mut identified, catalog, value); - } - return; - } - let mut forwarded = SinkForward(sink); + let mut forwarded = match id { + Some(id) => SinkForward::identified(sink, id), + None => SinkForward::new(sink), + }; if !host.format_default_to(catalog, index, value, &mut forwarded) { - emit_value_ref(sink, catalog, value); + emit_value_ref(&mut forwarded, catalog, value); } } @@ -2308,7 +2244,7 @@ fn emit_arg_direct_or_fallback( diagnostics: &mut Option<&mut dyn DiagnosticsSink>, ) { if let Some(value) = args.get_ref(key_id) { - emit_output_value(sink, host, index, catalog, value); + emit_output_value(sink, host, index, catalog, value, None); return; } diff --git a/wind_tunnel/benches/formatting.rs b/wind_tunnel/benches/formatting.rs index 1f28c26..1c4bdcb 100644 --- a/wind_tunnel/benches/formatting.rs +++ b/wind_tunnel/benches/formatting.rs @@ -628,6 +628,7 @@ fn build_icu_datetime_catalog() -> Catalog { struct CountingSink { events: usize, bytes: usize, + structured: bool, } impl CountingSink { @@ -638,6 +639,10 @@ impl CountingSink { } impl FormatSink for CountingSink { + fn wants_structured_output(&self) -> bool { + self.structured + } + fn literal(&mut self, value: &str) { self.events += 1; self.bytes += value.len(); @@ -663,6 +668,11 @@ impl FormatSink for CountingSink { self.bytes += option.key.len() + option.value.len(); } } + + fn formatted_value(&mut self, value: &message_format::runtime::FormattedValue<'_>) { + self.events += 1; + self.bytes += value.value.len() + value.id.map_or(0, str::len); + } } fn bench_formatting(c: &mut Criterion) { @@ -1457,6 +1467,34 @@ fn bench_runtime_compiled_declarations(c: &mut Criterion) { }); }); + let identified_catalog = + compile_catalog(".local $n = {21 :number u:id=item} {{a={$n} b={$n} c={$n}}}"); + let identified_host = BuiltinHost::new(&locale("en-US")).expect("host"); + let mut identified_formatter = + Formatter::new(&identified_catalog, identified_host).expect("formatter"); + let identified_message = identified_formatter + .resolve("main") + .expect("resolved message"); + let identified_args: Vec<(u32, Value)> = Vec::new(); + let mut identified_sink = CountingSink { + structured: true, + ..CountingSink::default() + }; + group.bench_function("structured_number_id_reused_three_places", |b| { + b.iter(|| { + identified_sink.reset(); + identified_formatter + .format_to( + identified_message, + black_box(&identified_args), + &mut identified_sink, + None, + ) + .expect("format"); + black_box((identified_sink.events, identified_sink.bytes)); + }); + }); + let chain_catalog = compile_catalog(".input {$x :string} .local $n = {$x :number} {{value={$n}}}"); let chain_entries = [("x", Value::Int(21))];