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.

Sign up for GitHub

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

Jump to bottom

find_or_insert_with should return a &mut pointer #6394

Closed
metajack opened this issue May 10, 2013 · 6 comments
Closed

find_or_insert_with should return a &mut pointer #6394

metajack opened this issue May 10, 2013 · 6 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@metajack
Copy link
Contributor

Trying to satisfy the borrow checker in some cases leads to a little bit clumsy HashMap code. For example, if you want to find_or_insert but need to modify the value.

Here's an example where I had to work around this by using contains_key and then find_mut and insert directly:

https://gist.github.com/metajack/5555485#file-gistfile2-diff-L28-L45

I suggest adding find_or_insert_mut and find_or_insert_with_mut. It might be better to use find_mut_or_insert and find_mut_or_insert_with as names perhaps.
@metajack
Copy link
Contributor Author

nominating for production ready

@nikomatsakis
Copy link
Contributor

Just to be clear, these methods would be the same as find_or_insert_with, but that they return a mutable pointer? It may be that this could just replace find_or_insert_with altogether; you can always re-borrow the returned &mut pointer as & if that's what you want

@metajack
Copy link
Contributor Author

If that's the case, then find_mut should disappear as well, and find should return &mut, etc?

@thestinger
Copy link
Contributor

@metajack: find needs to exist because it takes &self

@graydon
Copy link
Contributor

graydon commented May 16, 2013

accepted for backwards-compatible milestone

@nikomatsakis nikomatsakis mentioned this issue Jun 1, 2013
Closed
@lilyball
Copy link
Contributor

lilyball commented Jun 2, 2013

My latest proposed patch in #6815 fixes this.

@metajack metajack mentioned this issue Sep 9, 2013
Closed
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 6, 2020
@bors
Auto merge of rust-lang#6394 - nico-abram:unsafe_sizeof_count_copies,…
7f22b1c 
… r=ebroto

Add lint size_of_in_element_count

Fixes rust-lang#6381
changelog: Add lint to check for using size_of::<T> or size_of_val::<T> in the count parameter to ptr::copy or ptr::copy_nonoverlapping, which take a count of Ts (And not a count of bytes)

- \[X] Followed [lint naming conventions][lint_naming]
- \[X] Added passing UI tests (including committed `.stderr` file)
- \[ ] `cargo test` passes locally
- \[X] Executed `cargo dev update_lints`
- \[X] Added lint documentation
- \[X] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

Running `cargo test` locally fails with this error:

```
running 1 test
test fmt ... FAILED

failures:

---- fmt stdout ----
status: exit code: 1
stdout:
stderr: error: unable to unlink old fallback exe
error: caused by: Access is denied. (os error 5)

thread 'fmt' panicked at 'Formatting check failed. Run `cargo dev fmt` to update formatting.', tests\fmt.rs:32:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    fmt

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
```

But I did run `cargo dev fmt`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@lilyball @graydon @metajack @nikomatsakis @thestinger