From 9d1c28fbecc58b8713eba2bbe20ea7b316d60af3 Mon Sep 17 00:00:00 2001 From: Federica Date: Mon, 3 Jun 2024 18:12:00 -0300 Subject: [PATCH 1/6] Improve arg handling --- cairo1-run/src/main.rs | 53 ++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index 82e4f7a866..5c959afb85 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -63,49 +63,30 @@ struct Args { #[derive(Debug, Clone, Default)] struct FuncArgs(Vec); +fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { + let mut array = vec![]; + while let Some(value) = iter.next() { + match value { + "]" => break, + _ => array.push(Felt252::from_dec_str(value).unwrap()), + } + } + FuncArg::Array(array) +} + fn process_args(value: &str) -> Result { if value.is_empty() { return Ok(FuncArgs::default()); } let mut args = Vec::new(); - let mut input = value.split(' '); + let mut input = value.split_inclusive(|c| c == '[' || c == ']' || c == ' ').skip_while(|c| c == &" "); + while let Some(value) = input.next() { - // First argument in an array - if value.starts_with('[') { - if value.ends_with(']') { - if value.len() == 2 { - args.push(FuncArg::Array(Vec::new())); - } else { - args.push(FuncArg::Array(vec![Felt252::from_dec_str( - value.strip_prefix('[').unwrap().strip_suffix(']').unwrap(), - ) - .unwrap()])); - } - } else { - let mut array_arg = - vec![Felt252::from_dec_str(value.strip_prefix('[').unwrap()).unwrap()]; - // Process following args in array - let mut array_end = false; - while !array_end { - if let Some(value) = input.next() { - // Last arg in array - if value.ends_with(']') { - array_arg.push( - Felt252::from_dec_str(value.strip_suffix(']').unwrap()).unwrap(), - ); - array_end = true; - } else { - array_arg.push(Felt252::from_dec_str(value).unwrap()) - } - } - } - // Finalize array - args.push(FuncArg::Array(array_arg)) - } - } else { - // Single argument - args.push(FuncArg::Single(Felt252::from_dec_str(value).unwrap())) + match value { + "[" => args.push(process_array(&mut input)), + _ => args.push(FuncArg::Single(Felt252::from_dec_str(value).unwrap())), } + } Ok(FuncArgs(args)) } From 1d7500f6178dd5cdf56a60cabfd41181cac9f08f Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 4 Jun 2024 11:14:23 -0300 Subject: [PATCH 2/6] Fix --- cairo1-run/src/main.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index 5c959afb85..3829cf908c 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -63,6 +63,8 @@ struct Args { #[derive(Debug, Clone, Default)] struct FuncArgs(Vec); +// Processes an iterator of format [s1, s2,.., sn, "]", ...], stopping at the first "]" string +// and returning the array [f1, f2,.., fn] where fi = Felt::from_dec_str(si) fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { let mut array = vec![]; while let Some(value) = iter.next() { @@ -74,13 +76,34 @@ fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { FuncArg::Array(array) } +// Parses a string of ascii whitespace separated values, containing either numbers or series of numbers wrapped in brackets +// Returns an array of felts and felt arrays fn process_args(value: &str) -> Result { if value.is_empty() { return Ok(FuncArgs::default()); } let mut args = Vec::new(); - let mut input = value.split_inclusive(|c| c == '[' || c == ']' || c == ' ').skip_while(|c| c == &" "); - + // Split input string into numbers and array delimiters + let mut input = value.split_ascii_whitespace().flat_map(|mut x| { + // We don't have a way to split and keep the separate delimiters so we do it manually + let mut res = vec![]; + if let Some(val) = x.strip_prefix('[') { + res.push("["); + x = val; + } + if let Some(val) = x.strip_suffix(']') { + if !val.is_empty() { + res.push(val) + } + res.push("]") + } else { + if !x.is_empty() { + res.push(x) + } + } + res + }); + // Process iterator of numbers & array delimiters while let Some(value) = input.next() { match value { "[" => args.push(process_array(&mut input)), From f51c1682b18d4c4eb3988180c131a51024277eb1 Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 4 Jun 2024 11:41:59 -0300 Subject: [PATCH 3/6] Clippy --- cairo1-run/src/main.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index 3829cf908c..7eb6bfe1c8 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -67,7 +67,7 @@ struct FuncArgs(Vec); // and returning the array [f1, f2,.., fn] where fi = Felt::from_dec_str(si) fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { let mut array = vec![]; - while let Some(value) = iter.next() { + for value in iter { match value { "]" => break, _ => array.push(Felt252::from_dec_str(value).unwrap()), @@ -79,9 +79,6 @@ fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { // Parses a string of ascii whitespace separated values, containing either numbers or series of numbers wrapped in brackets // Returns an array of felts and felt arrays fn process_args(value: &str) -> Result { - if value.is_empty() { - return Ok(FuncArgs::default()); - } let mut args = Vec::new(); // Split input string into numbers and array delimiters let mut input = value.split_ascii_whitespace().flat_map(|mut x| { @@ -96,10 +93,8 @@ fn process_args(value: &str) -> Result { res.push(val) } res.push("]") - } else { - if !x.is_empty() { - res.push(x) - } + } else if !x.is_empty() { + res.push(x) } res }); From 789786bc55b2a787f4e80e104586cfc3c9d3f729 Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 4 Jun 2024 11:58:24 -0300 Subject: [PATCH 4/6] Remove unwraps --- cairo1-run/src/main.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index 7eb6bfe1c8..c9ef562663 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -65,15 +65,18 @@ struct FuncArgs(Vec); // Processes an iterator of format [s1, s2,.., sn, "]", ...], stopping at the first "]" string // and returning the array [f1, f2,.., fn] where fi = Felt::from_dec_str(si) -fn process_array<'a>(iter:&mut impl Iterator) -> FuncArg { +fn process_array<'a>(iter: &mut impl Iterator) -> Result { let mut array = vec![]; for value in iter { match value { "]" => break, - _ => array.push(Felt252::from_dec_str(value).unwrap()), + _ => array.push( + Felt252::from_dec_str(value) + .map_err(|_| format!("\"{}\" is not a valid felt", value))?, + ), } } - FuncArg::Array(array) + Ok(FuncArg::Array(array)) } // Parses a string of ascii whitespace separated values, containing either numbers or series of numbers wrapped in brackets @@ -101,10 +104,12 @@ fn process_args(value: &str) -> Result { // Process iterator of numbers & array delimiters while let Some(value) = input.next() { match value { - "[" => args.push(process_array(&mut input)), - _ => args.push(FuncArg::Single(Felt252::from_dec_str(value).unwrap())), + "[" => args.push(process_array(&mut input)?), + _ => args.push(FuncArg::Single( + Felt252::from_dec_str(value) + .map_err(|_| format!("\"{}\" is not a valid felt", value))?, + )), } - } Ok(FuncArgs(args)) } From 7842a29acd470b4f4a748e68180b1e069460b3e7 Mon Sep 17 00:00:00 2001 From: Federica Date: Tue, 4 Jun 2024 12:13:56 -0300 Subject: [PATCH 5/6] Add changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5a02fa4df..f7c1452e62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ #### Upcoming Changes +* refactor + bugfix: Improve arg handling for cairo1-run [#1782](https://github.com/lambdaclass/cairo-vm/pull/1782) + * Now uses ascii whitespace as separator, preventing errors when using newlines in args file + * No longer gets stuck on improperly-formatted arrays + * Returns an informative clap error upon invalid felt strings instead of unwrapping + * refactor: Add boolean method Cairo1RunConfig::copy_to_output + Update Doc [#1778](https://github.com/lambdaclass/cairo-vm/pull/1778) * feat: Filter implicit arguments from return value in cairo1-run crate [#1775](https://github.com/lambdaclass/cairo-vm/pull/1775) From 00e8e404e518f64ece8152645493f8dd57d7508a Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Tue, 4 Jun 2024 15:44:56 -0300 Subject: [PATCH 6/6] Update comments --- cairo1-run/src/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index c9ef562663..f2f34c2064 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -63,8 +63,8 @@ struct Args { #[derive(Debug, Clone, Default)] struct FuncArgs(Vec); -// Processes an iterator of format [s1, s2,.., sn, "]", ...], stopping at the first "]" string -// and returning the array [f1, f2,.., fn] where fi = Felt::from_dec_str(si) +/// Processes an iterator of format [s1, s2,.., sn, "]", ...], stopping at the first "]" string +/// and returning the array [f1, f2,.., fn] where fi = Felt::from_dec_str(si) fn process_array<'a>(iter: &mut impl Iterator) -> Result { let mut array = vec![]; for value in iter { @@ -79,8 +79,8 @@ fn process_array<'a>(iter: &mut impl Iterator) -> Result Result { let mut args = Vec::new(); // Split input string into numbers and array delimiters