From df08cdd1c9f49499197bfa0a31efed30c1f40263 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 09:57:29 -0300 Subject: [PATCH] Don't use regex in print oracle --- Cargo.lock | 1 - compiler/noirc_printable_type/Cargo.toml | 1 - compiler/noirc_printable_type/src/lib.rs | 138 ++++++++++++++--------- 3 files changed, 82 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 14066a0e37e..c43e17955d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3195,7 +3195,6 @@ dependencies = [ "acvm", "iter-extended", "jsonrpc", - "regex", "serde", "serde_json", "thiserror", diff --git a/compiler/noirc_printable_type/Cargo.toml b/compiler/noirc_printable_type/Cargo.toml index 8bb56703e8a..8d0574aad64 100644 --- a/compiler/noirc_printable_type/Cargo.toml +++ b/compiler/noirc_printable_type/Cargo.toml @@ -14,7 +14,6 @@ workspace = true [dependencies] acvm.workspace = true iter-extended.workspace = true -regex = "1.9.1" serde.workspace = true serde_json.workspace = true thiserror.workspace = true diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 0c6fbe3fbac..d46b37c4ea2 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -2,7 +2,7 @@ use std::{collections::BTreeMap, str}; use acvm::{acir::AcirField, brillig_vm::brillig::ForeignCallParam}; use iter_extended::vecmap; -use regex::{Captures, Regex}; + use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -253,49 +253,6 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op Some(output) } -/// Assumes `re` is a regex that matches interpolation sequences like `{...}`. -/// Replaces all matches by invoking `replacement`, but only if a match isn't preceded by an odd -/// number of '{'. -/// For example, if we have `{{x}}` then there's one `{` before the interpolation, and we can -/// see it's actually not an interpolation. -/// However, if we have `{{{x}}}` there are two `{` before the interpolation, so the third -/// `{` actually starts an interpolation. -/// With four (`{{{{x}}}}`) it's again even and it's not an interpolation, etc. -fn replace_all_interpolations( - re: &Regex, - haystack: &str, - mut replacement: impl FnMut(&Captures) -> Result, -) -> Result { - let mut new = String::with_capacity(haystack.len()); - let mut last_match = 0; - for caps in re.captures_iter(haystack) { - let m = caps.get(0).unwrap(); - - // Count how many '{' we have right before this interpolation. If the number if - // even (or zero) it's an interpolation, otherwise it's not. - let mut curly_braces_count = 0; - let mut index = m.start(); - while index > 0 { - if haystack.as_bytes().get(index - 1).unwrap() == &b'{' { - curly_braces_count += 1; - } else { - break; - } - index -= 1; - } - - if curly_braces_count % 2 == 1 { - continue; - } - - new.push_str(&haystack[last_match..m.start()]); - new.push_str(&replacement(&caps)?); - last_match = m.end(); - } - new.push_str(&haystack[last_match..]); - Ok(new) -} - impl std::fmt::Display for PrintableValueDisplay { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -304,24 +261,56 @@ impl std::fmt::Display for PrintableValueDisplay { write!(fmt, "{output_string}") } Self::FmtString(template, values) => { - let mut display_iter = values.iter(); - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; + let mut values_iter = values.iter(); + write_template_replacing_interpolations(template, fmt, || { + values_iter.next().and_then(|(value, typ)| to_string(value, typ)) + }) + } + } + } +} - // First replace all `{...}` interpolations with the corresponding values, making sure - // to skip ones that are also surrounded by curly braces (like `{{...}}`) - let formatted_str = replace_all_interpolations(&re, template, |_: &Captures| { - let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; - to_string(value, typ).ok_or(std::fmt::Error) - })?; +fn write_template_replacing_interpolations( + template: &str, + fmt: &mut std::fmt::Formatter<'_>, + mut replacement: impl FnMut() -> Option, +) -> std::fmt::Result { + let mut last_index = 0; // How far we've written from the template + let mut char_indices = template.char_indices().peekable(); + while let Some((char_index, char)) = char_indices.next() { + // Keep going forward until we find a '{' + if char != '{' { + continue; + } - // Now that we replaced all interpolations, we need to unescape the double curly braces - // that were added to avoid interpolations. - let formatted_str = formatted_str.replace("{{", "{").replace("}}", "}"); + // We'll either have to write an interpolation or '{{' if it's an escape, + // so let's write what we've seen so far in the template. + write!(fmt, "{}", &template[last_index..char_index])?; - write!(fmt, "{formatted_str}") + // If it's '{{', write '{' and keep going + if char_indices.peek().map(|(_, char)| char) == Some(&'{') { + write!(fmt, "{{")?; + (last_index, _) = char_indices.next().unwrap(); + continue; + } + + // Write the interpolation + if let Some(string) = replacement() { + write!(fmt, "{}", string)?; + } else { + return Err(std::fmt::Error); + } + + // Whatever was inside '{...}' doesn't matter, so skip until we find '}' + while let Some((_, char)) = char_indices.next() { + if char == '}' { + last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); + break; } } } + + write!(fmt, "{}", &template[last_index..]) } /// This trims any leading zeroes. @@ -421,3 +410,40 @@ pub fn decode_string_value(field_elements: &[F]) -> String { let final_string = str::from_utf8(&string_as_slice).unwrap(); final_string.to_owned() } + +#[cfg(test)] +mod tests { + use acvm::FieldElement; + + use crate::{PrintableType, PrintableValue, PrintableValueDisplay}; + + #[test] + fn printable_value_display_to_string_without_interpolations() { + let template = "hello"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), vec![]); + assert_eq!(display.to_string(), template); + } + + #[test] + fn printable_value_display_to_string_with_curly_escapes() { + let template = "hello {{world}} {{{{double_escape}}}}"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), vec![]); + assert_eq!(display.to_string(), template); + } + + #[test] + fn printable_value_display_to_string_with_interpolations() { + let template = "hello {one} {{no}} {two} {{not_again}} {three} world"; + let values = vec![ + (PrintableValue::String("ONE".to_string()), PrintableType::String { length: 3 }), + (PrintableValue::String("TWO".to_string()), PrintableType::String { length: 3 }), + (PrintableValue::String("THREE".to_string()), PrintableType::String { length: 5 }), + ]; + let expected = "hello ONE {{no}} TWO {{not_again}} THREE world"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), values); + assert_eq!(display.to_string(), expected); + } +}