Skip to content

Commit

Permalink
Don't use regex in print oracle
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed Dec 6, 2024
1 parent a780761 commit df08cdd
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 58 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion compiler/noirc_printable_type/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
138 changes: 82 additions & 56 deletions compiler/noirc_printable_type/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -253,49 +253,6 @@ fn to_string<F: AcirField>(value: &PrintableValue<F>, 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<E>(
re: &Regex,
haystack: &str,
mut replacement: impl FnMut(&Captures) -> Result<String, E>,
) -> Result<String, E> {
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<F: AcirField> std::fmt::Display for PrintableValueDisplay<F> {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand All @@ -304,24 +261,56 @@ impl<F: AcirField> std::fmt::Display for PrintableValueDisplay<F> {
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<String>,
) -> 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.
Expand Down Expand Up @@ -421,3 +410,40 @@ pub fn decode_string_value<F: AcirField>(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::<FieldElement>::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::<FieldElement>::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::<FieldElement>::FmtString(template.to_string(), values);
assert_eq!(display.to_string(), expected);
}
}

0 comments on commit df08cdd

Please sign in to comment.