-
Notifications
You must be signed in to change notification settings - Fork 154
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.
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
fix: remove static lifetime for name str parameter requirement for constant getter #1523
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 🚀 Please update the CHANGELOG file
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1523 +/- ##
==========================================
+ Coverage 94.76% 97.11% +2.34%
==========================================
Files 101 91 -10
Lines 38826 37248 -1578
==========================================
- Hits 36795 36172 -623
+ Misses 2031 1076 -955 ☔ View full report in Codecov by Sentry. |
5789f5e
to
a2a932e
Compare
a272b73
to
42db06f
Compare
Applying this patch should fix the linter: diff --git a/vm/src/hint_processor/builtin_hint_processor/math_utils.rs b/vm/src/hint_processor/builtin_hint_processor/math_utils.rs
index 68cb30fc1..bbe18b5d7 100644
--- a/vm/src/hint_processor/builtin_hint_processor/math_utils.rs
+++ b/vm/src/hint_processor/builtin_hint_processor/math_utils.rs
@@ -2085,7 +2085,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
- Err(HintError::MissingConstant(bx)) if *bx == ADDR_BOUND.to_string()
+ Err(HintError::MissingConstant(bx)) if &*bx == ADDR_BOUND
);
}
@@ -2309,7 +2309,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
- Err(HintError::MissingConstant(x)) if (*x) == "MAX_HIGH".to_string()
+ Err(HintError::MissingConstant(x)) if &*x == "MAX_HIGH"
);
}
diff --git a/vm/src/hint_processor/builtin_hint_processor/uint384.rs b/vm/src/hint_processor/builtin_hint_processor/uint384.rs
index 4e2bb4412..9f6646dd3 100644
--- a/vm/src/hint_processor/builtin_hint_processor/uint384.rs
+++ b/vm/src/hint_processor/builtin_hint_processor/uint384.rs
@@ -535,7 +535,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code::ADD_NO_UINT384_CHECK),
- Err(HintError::MissingConstant(bx)) if *bx == "SHIFT".to_string()
+ Err(HintError::MissingConstant(bx)) if &*bx == "SHIFT"
);
} |
remove static lifetime for name str parameter requirement for constant getter
Description
Hey!
I noticed
get_constant_from_var_name
has a different signature from other hint_utils helper functions, namely that it has a bound of static lifetime on the name parameter. I noticed it mainly comes from the error signatureHintError::MissingConstant
.I changed the signature of
MissingConstant
to match other errors: from Box<'static &str> to Box using the same util you use around the hint error codebase:var_name.to_string().into_boxed_str()
.In doing this, one no longer needs to have 'static strings for constant getting -> this will allow me in next PRs to have dynamic constant name getting, i.e., in garaga there are constant limbs of Prime P of the BN Curve.
And depending on
N_LIMBS
, we'll getP_i
.Let me know if this works!
Resolves: #1522
Description of the pull request changes and motivation.
Checklist