Skip to content

⚡ Bolt: [performance improvement] Refactor pattern module manual argument extraction #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
logbie wants to merge 1 commit into from
Closed

⚡ Bolt: [performance improvement] Refactor pattern module manual argument extraction #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/stdlib/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,21 @@ generate_expect!(
|t: &Rc<chrono::NaiveTime>| Rc::clone(t)
);

generate_expect!(
/// Extracts a compiled pattern from a WFL Value.
///
/// # Arguments
/// * `value` - The WFL Value to extract from
///
/// # Returns
/// Returns an `Rc<CompiledPattern>` clone if the value is a Pattern variant.
expect_pattern,
Pattern,
Rc<crate::pattern::CompiledPattern>,
"a compiled pattern",
|p: &Rc<crate::pattern::CompiledPattern>| Rc::clone(p)
);

generate_expect!(
/// Extracts a DateTime value from a WFL Value, returning it as a reference-counted NaiveDateTime.
///
Expand Down
178 changes: 27 additions & 151 deletions src/stdlib/pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::helpers::{check_arg_count, expect_pattern, expect_text};
use crate::interpreter::environment::Environment;
use crate::interpreter::error::RuntimeError;
use crate::interpreter::value::Value;
Expand All @@ -19,35 +20,11 @@ pub fn register(env: &mut Environment) {
/// Native function: pattern_matches(text, pattern) -> boolean
/// Tests if text matches the given compiled pattern
pub fn pattern_matches_native(args: Vec<Value>) -> Result<Value, RuntimeError> {
if args.len() != 2 {
return Err(RuntimeError::new(
"pattern_matches requires exactly 2 arguments (text, pattern)".to_string(),
0,
0,
));
}

let text_str = match &args[0] {
Value::Text(s) => s.as_ref(),
_ => {
return Err(RuntimeError::new(
"First argument to pattern_matches must be text".to_string(),
0,
0,
));
}
};
check_arg_count("pattern_matches", &args, 2)?;

let compiled_pattern = match &args[1] {
Value::Pattern(p) => p,
_ => {
return Err(RuntimeError::new(
"Second argument to pattern_matches must be a compiled pattern".to_string(),
0,
0,
));
}
};
let text = expect_text(&args[0])?;
let compiled_pattern = expect_pattern(&args[1])?;
let text_str = text.as_ref();

let matches = compiled_pattern.matches(text_str);
Ok(Value::Bool(matches))
Expand All @@ -56,35 +33,11 @@ pub fn pattern_matches_native(args: Vec<Value>) -> Result<Value, RuntimeError> {
/// Native function: pattern_find(text, pattern) -> object or null
/// Finds the first match of pattern in text
pub fn pattern_find_native(args: Vec<Value>) -> Result<Value, RuntimeError> {
if args.len() != 2 {
return Err(RuntimeError::new(
"pattern_find requires exactly 2 arguments (text, pattern)".to_string(),
0,
0,
));
}
check_arg_count("pattern_find", &args, 2)?;

let text_str = match &args[0] {
Value::Text(s) => s.as_ref(),
_ => {
return Err(RuntimeError::new(
"First argument to pattern_find must be text".to_string(),
0,
0,
));
}
};

let compiled_pattern = match &args[1] {
Value::Pattern(p) => p,
_ => {
return Err(RuntimeError::new(
"Second argument to pattern_find must be a compiled pattern".to_string(),
0,
0,
));
}
};
let text = expect_text(&args[0])?;
let compiled_pattern = expect_pattern(&args[1])?;
let text_str = text.as_ref();

match compiled_pattern.find(text_str) {
Some(match_result) => {
Expand Down Expand Up @@ -120,35 +73,11 @@ pub fn pattern_find_native(args: Vec<Value>) -> Result<Value, RuntimeError> {
/// Native function: pattern_find_all(text, pattern) -> list
/// Finds all matches of pattern in text
pub fn pattern_find_all_native(args: Vec<Value>) -> Result<Value, RuntimeError> {
if args.len() != 2 {
return Err(RuntimeError::new(
"pattern_find_all requires exactly 2 arguments (text, pattern)".to_string(),
0,
0,
));
}

let text_str = match &args[0] {
Value::Text(s) => s.as_ref(),
_ => {
return Err(RuntimeError::new(
"First argument to pattern_find_all must be text".to_string(),
0,
0,
));
}
};
check_arg_count("pattern_find_all", &args, 2)?;

let compiled_pattern = match &args[1] {
Value::Pattern(p) => p,
_ => {
return Err(RuntimeError::new(
"Second argument to pattern_find_all must be a compiled pattern".to_string(),
0,
0,
));
}
};
let text = expect_text(&args[0])?;
let compiled_pattern = expect_pattern(&args[1])?;
let text_str = text.as_ref();

let matches = compiled_pattern.find_all(text_str);
let mut result_list = Vec::new();
Expand Down Expand Up @@ -189,46 +118,15 @@ pub fn native_pattern_replace(
line: usize,
column: usize,
) -> Result<Value, RuntimeError> {
if args.len() != 3 {
return Err(RuntimeError::new(
"pattern_replace requires exactly 3 arguments".to_string(),
line,
column,
));
}

let text = match &args[0] {
Value::Text(t) => t.as_ref(),
_ => {
return Err(RuntimeError::new(
"First argument must be text".to_string(),
line,
column,
));
}
};
check_arg_count("pattern_replace", &args, 3)
.map_err(|e| RuntimeError::new(e.message, line, column))?;

let _pattern = match &args[1] {
Value::Pattern(p) => p.as_ref(),
_ => {
return Err(RuntimeError::new(
"Second argument must be a pattern".to_string(),
line,
column,
));
}
};

let _replacement = match &args[2] {
Value::Text(t) => t.as_ref(),
_ => {
return Err(RuntimeError::new(
"Third argument must be text".to_string(),
line,
column,
));
}
};
let text = expect_text(&args[0]).map_err(|e| RuntimeError::new(e.message, line, column))?;
let _pattern =
expect_pattern(&args[1]).map_err(|e| RuntimeError::new(e.message, line, column))?;
let _replacement =
expect_text(&args[2]).map_err(|e| RuntimeError::new(e.message, line, column))?;
let text = text.as_ref();

// TODO: Update to use new pattern system for replacement
Ok(Value::Text(Arc::from(text)))
Expand All @@ -240,35 +138,13 @@ pub fn native_pattern_split(
line: usize,
column: usize,
) -> Result<Value, RuntimeError> {
if args.len() != 2 {
return Err(RuntimeError::new(
"pattern_split requires exactly 2 arguments".to_string(),
line,
column,
));
}
check_arg_count("pattern_split", &args, 2)
.map_err(|e| RuntimeError::new(e.message, line, column))?;

let text = match &args[0] {
Value::Text(t) => t.as_ref(),
_ => {
return Err(RuntimeError::new(
"First argument must be text".to_string(),
line,
column,
));
}
};

let pattern = match &args[1] {
Value::Pattern(p) => p,
_ => {
return Err(RuntimeError::new(
"Second argument must be a pattern".to_string(),
line,
column,
));
}
};
let text_arc = expect_text(&args[0]).map_err(|e| RuntimeError::new(e.message, line, column))?;
let pattern =
expect_pattern(&args[1]).map_err(|e| RuntimeError::new(e.message, line, column))?;
let text = text_arc.as_ref();

// Find all matches of the pattern in the text
let matches = pattern.find_all(text);
Expand Down
8 changes: 4 additions & 4 deletions src/stdlib/pattern_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ mod tests {
result
.unwrap_err()
.to_string()
.contains("exactly 2 arguments")
.contains("expects 2 arguments")
);
}

Expand All @@ -33,7 +33,7 @@ mod tests {
result
.unwrap_err()
.to_string()
.contains("exactly 2 arguments")
.contains("expects 2 arguments")
);
}

Expand All @@ -45,7 +45,7 @@ mod tests {
result
.unwrap_err()
.to_string()
.contains("exactly 2 arguments")
.contains("expects 2 arguments")
);
}

Expand All @@ -57,6 +57,6 @@ mod tests {
];
let result = pattern_matches_native(args);
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("First argument"));
assert!(result.unwrap_err().to_string().contains("Expected text"));
}
}
Loading