Skip to content
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

[Bug] LogData::topics_mut_unchecked and LogData::topics_mut are useless without DerefMut #775

Closed
wtdcode opened this issue Oct 17, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@wtdcode
Copy link
Contributor

wtdcode commented Oct 17, 2024

Component

primitives

What version of Alloy are you on?

alloy-primitives v0.8.8

Operating System

Linux

Describe the bug

main.rs

fn main() {
    let log = alloy::primitives::Log::empty();

    let topics = log.topics_mut();

    dbg!(&topics);
}

cargo.toml

[package]
name = "hello"
version = "0.1.0"
edition = "2021"

[dependencies]
alloy = "0.4.2"

This gives error:

error[E0596]: cannot borrow data in dereference of `Log` as mutable
 --> src/main.rs:4:18
  |
4 |     let topics = log.topics_mut();
  |                  ^^^ cannot borrow as mutable
  |
  = help: trait `DerefMut` is required to modify through a dereference, but it is not implemented for `Log`

For more information about this error, try `rustc --explain E0596`.
error: could not compile `hello` (bin "hello") due to 1 previous error

I have no idea when LogData::topics_mut_unchecked and LogData::topics_mut should be used without DerefMut implemented. It looks like it is by design not to implement DerefMut but it makes it impossible to call these two functions (and other functions taking &mut self).

@wtdcode wtdcode added the bug Something isn't working label Oct 17, 2024
@wtdcode wtdcode changed the title [Bug] LogData::topics_mut_unchecked and LogData::topics_mut are useless without DrefMut [Bug] LogData::topics_mut_unchecked and LogData::topics_mut are useless without DerefMut Oct 17, 2024
@wtdcode
Copy link
Contributor Author

wtdcode commented Oct 25, 2024

@DaniPopes @mattsse Hey guys, any advice on this? I can also draft a PR but I need to know the intended way to handle this:

  • Implement Deref for LogData or,
  • totally remove all methods taking &mut self ?

Or do I miss something?

@mattsse
Copy link
Member

mattsse commented Oct 25, 2024

let mut log = alloy_primitives::Log::empty();
let topics = log.data.topics_mut();

Log only has Deref for the wrapped data, but since data is pub anyway, we could also just impl derefmut as well imo

@wtdcode
Copy link
Contributor Author

wtdcode commented Oct 25, 2024

let mut log = alloy_primitives::Log::empty();
let topics = log.data.topics_mut();

Ohhhhh, you are correct! I didn't notice data is public XD. I will draft a PR for DerefMut.

@yash-atreya
Copy link
Member

Closed by #786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants