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

lint on manual Hash, *Ord, *Eq impls that access a *Cell #732

Closed
oli-obk opened this issue Mar 2, 2016 · 2 comments · Fixed by #4885
Closed

lint on manual Hash, *Ord, *Eq impls that access a *Cell #732

oli-obk opened this issue Mar 2, 2016 · 2 comments · Fixed by #4885
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 2, 2016

even read-only access can be bad, because between two calls to hash, the cell might have changed its value, even if there are only immutable references present.

cc #729

should be fairly straight forward.

  1. Find impls of the above traits
  2. check if the type has fields that have interior mutability
  3. check if the impl-method has an ExprField for one of the interior mutability fields

bonus points for also checking inherent methods that are called in the impl method for interior mutation.

@llogiq
Copy link
Contributor

llogiq commented May 8, 2016

we should also warn on std::sync::atomic types (as they also can be mutated from a & ref.

@oli-obk oli-obk added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. A-lint Area: New lints T-middle Type: Probably requires verifiying types labels May 10, 2017
@llogiq
Copy link
Contributor

llogiq commented Oct 10, 2019

I think that people might want those impls anyway. Especially Ord and Eq might be helpful for sorting or partitioning a collection (or a variety of other things) while having a &mut ref ensures no concurrent modification.

bors added a commit that referenced this issue Dec 23, 2019
new lint: mutable_key_type

This fixes #732 – well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members

changelog: add lint: `mutable_key_type`

r? @flip1995
bors added a commit that referenced this issue Dec 23, 2019
new lint: mutable_key_type

This fixes #732 – well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members

changelog: add lint [`mutable_key_type`]

r? @flip1995
bors added a commit that referenced this issue Dec 23, 2019
new lint: mutable_key_type

This fixes #732 – well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members

changelog: add `mutable_key_type` lint

r? @flip1995
bors added a commit that referenced this issue Dec 24, 2019
new lint: mutable_key_type

This fixes #732 – well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members

changelog: add `mutable_key_type` lint

r? @flip1995
bors added a commit that referenced this issue Dec 24, 2019
new lint: mutable_key_type

This fixes #732 - well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members

changelog: add `mutable_key_type` lint

r? @flip1995
@bors bors closed this as completed in 1837cbc Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants