chore: bump to rust edition 2024 #10802
Conversation
| @@ -82,7 +82,6 @@ jobs: | |||
| - uses: actions/checkout@v4 | |||
| - uses: dtolnay/rust-toolchain@nightly | |||
| with: | |||
| toolchain: nightly-2024-02-03 | |||
There was a problem hiding this comment.
Found out we were pinning to this specific nightly, this seems unintended / resolved
| @@ -46,7 +46,7 @@ pub struct HexIdProvider { | |||
|
|
|||
| impl HexIdProvider { | |||
| /// Generates a random hex encoded Id | |||
| pub fn gen(&self) -> String { | |||
There was a problem hiding this comment.
gen is now a reserved keyword, found generate better than r#gen
| @@ -20,7 +20,7 @@ impl Cheatcode for setEnvCall { | |||
| } else if value.contains('\0') { | |||
| Err(fmt_err!("environment variable value can't contain NUL character `\\0`")) | |||
| } else { | |||
| env::set_var(key, value); | |||
| unsafe { env::set_var(key, value); } | |||
There was a problem hiding this comment.
unsafe is now required for things that touch the environment
| fuzz_state.clone(), | ||
| fuzz_fixtures.clone(), |
There was a problem hiding this comment.
Due to lifetime / ownership issues clone is not required here - not ideal!
There was a problem hiding this comment.
you mean, is required?
isn't this giga expensive?
There was a problem hiding this comment.
Sorry - yes, is required
The inner fields are Arc'ed so clones should be relatively cheap but still worth evaluating how to refactor this without clones
| let settings = cache.profiles.get(&profile).expect("must be present"); | ||
|
|
||
| settings | ||
| cache.profiles.get(&profile).expect("must be present") |
There was a problem hiding this comment.
clippy lint rule now enforces returning the result of a let binding from a block preferring direct returns where possible
zerosnacks
requested review from
DaniPopes,
klkvr,
mattsse,
grandizzy and
yash-atreya
as code owners
| fuzz_state.clone(), | ||
| fuzz_fixtures.clone(), |
There was a problem hiding this comment.
you mean, is required?
isn't this giga expensive?
| @@ -85,7 +89,7 @@ pub fn invariant_strat( | |||
| /// * If `senders` is empty, then it's either a random address (10%) or from the dictionary (90%). | |||
| /// * If `senders` is not empty, a random address is chosen from the list of senders. | |||
| fn select_random_sender( | |||
| fuzz_state: &EvmFuzzState, | |||
| fuzz_state: EvmFuzzState, | |||
There was a problem hiding this comment.
shouldn't this borrow?
see Line 102
There was a problem hiding this comment.
this should borrow , the fix is potentially use<> like everywhere else you did it? obviously a refactor should require you to clone where you could pass by reference before
| @@ -287,27 +286,27 @@ impl NodeArgs { | |||
| } | |||
|
|
|||
| fn account_generator(&self) -> AccountGenerator { | |||
| let mut gen = AccountGenerator::new(self.accounts as usize) | |||
| let mut generate = AccountGenerator::new(self.accounts as usize) | |||
| @@ -232,7 +232,7 @@ Private Keys | |||
| let _ = write!(s, "\n({idx}) 0x{hex}"); | |||
| } | |||
|
|
|||
| if let Some(ref gen) = self.account_generator { | |||
| if let Some(ref generate) = self.account_generator { | |||
| if let Some(ref gen) = self.account_generator { | ||
| let phrase = gen.get_phrase().to_string(); | ||
| let derivation_path = gen.get_derivation_path().to_string(); | ||
| if let Some(ref generate) = self.account_generator { |
| @@ -85,7 +89,7 @@ pub fn invariant_strat( | |||
| /// * If `senders` is empty, then it's either a random address (10%) or from the dictionary (90%). | |||
| /// * If `senders` is not empty, a random address is chosen from the list of senders. | |||
| fn select_random_sender( | |||
| fuzz_state: &EvmFuzzState, | |||
| fuzz_state: EvmFuzzState, | |||
There was a problem hiding this comment.
this should borrow , the fix is potentially use<> like everywhere else you did it? obviously a refactor should require you to clone where you could pass by reference before
Motivation
Closes: #10734
Solution
Initial commit was done as a trial run with Claude, worked quite well!
Some of the added
.clone()is concerning, will look for alternative solution to deal with ownership. We are mostly dealing withArc's and the other fields are likely light to clone.Expecting this PR to fail its lint check, doing that in a follow-up here: #10803 as the diff is very large in terms of number of files touched
PR Checklist