rust-ethereum / evm

Pure Rust implementation of Ethereum Virtual Machine
Apache License 2.0
1.17k stars 360 forks source link

Chore: silence clippy closure in match warning #267

Closed clayrab closed 4 months ago

clayrab commented 9 months ago

Clippy is complaining about closures in match conditions.

https://rust-lang.github.io/rust-clippy/master/index.html#/blocks_in_conditions

I think the claim that this style "makes it hard to read" is subjective. I personally find these to be easily readable and I think it would be okay to just silence this warning.

These are the 2 offending blocks:

in misc.rs:

    match stack.perform_pop0_push1(|| {
        let size = U256::from(code.len());
        Ok((u256_to_h256(size), ()))
    }) {
        Ok(()) => Control::Continue,
        Err(e) => Control::Exit(Err(e)),
    }

in system.rs

    match machine.stack.perform_pop1_push0(|target| {
        let balance = handler.balance(address);

        handler.transfer(Transfer {
            source: address,
            target: (*target).into(),
            value: balance,
        })?;

        handler.mark_delete(address);
        handler.reset_balance(address);

        Ok(((), ()))
    }) {
        Ok(()) => Control::Exit(ExitSucceed::Suicided.into()),
        Err(e) => Control::Exit(Err(e)),
    }

I think find clippy is a bit heavy-handed, we've silenced quite a few things on litheumcore:

cargo clippy -- -A clippy::new_without_default -A clippy::too_many_arguments -A clippy::borrowed_box -A clippy::redundant_pattern_matching -A clippy::needless_bool -A clippy::or_fun_call -A clippy::len_without_is_empty -A clippy::identity_op -A clippy::collapsible_else_if -A clippy::unnecessary_fold -A clippy::map_flatten

clayrab commented 9 months ago

This is suprisingly difficult...

clippy.toml doesn't seem to support blocks_in_conditions:

  error: error reading Clippy's configuration file: unknown field `blocks_in_conditions`, expected one of

Doing it with a [lints] section in Cargo.toml is supposed to work after rust 1.74 but it just seems to fail silently when I do this:

[lints.clippy]
asdf = "deny"

So I'm suspecting that this also is just failing silently, since clippy.toml doesn't seem to support it:

[lints.clippy]
blocks_in_conditions = "deny"

I'm still looking for a better way to do this...

clayrab commented 9 months ago

Managed to get it going with a bit of help from @thaodt.

clippy.toml isn't the right place for this.

What do you think about this .cargo/config.toml solution?