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

Jump to bottom

new lint: mutable_key_type #4885

Merged
merged 1 commit into from Dec 24, 2019
Merged

new lint: mutable_key_type #4885

merged 1 commit into from Dec 24, 2019

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Dec 6, 2019 •
edited by phansch

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

@llogiq llogiq force-pushed the mut-key-types branch 3 times, most recently from 603d400 to 791b7e9 Compare December 6, 2019 20:36
@llogiq
Copy link
Contributor Author

llogiq commented Dec 11, 2019

r? @phansch

@flip1995
Copy link
Member

Sorry was absent for the past few days. Can you add tests for this lint. I don't really get, why every function/method signature gets checked, but only map/set types get linted.

IIUC only the check_local function can ever trigger the lint, except a function has a HashMap<InteriorMut, _> as an argument. Maybe it gets more clear with tests.

@llogiq
Copy link
Contributor Author

llogiq commented Dec 13, 2019

Fixed the lint and added a test. The lint will check for arguments, return types and non-wildcard locals.

@llogiq llogiq force-pushed the mut-key-types branch 2 times, most recently from 0f9a0cb to e6d638f Compare December 14, 2019 00:28
@llogiq
Copy link
Contributor Author

llogiq commented Dec 14, 2019

rebased.

@bors
Copy link
Collaborator

bors commented Dec 22, 2019

☔ The latest upstream changes (presumably #4930) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995
Copy link
Member

Am I missing something or would it be easier to just use the check_ty function of LateLintPass with hir_ty_to_ty instead of walking the signature of functions?

@llogiq
Copy link
Contributor Author

llogiq commented Dec 22, 2019

Probably. I took this route to ensure I know what places we lint.

@flip1995
Copy link
Member

That's a good argument for this way of writing the lint.

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 23, 2019

📌 Commit e6d638f has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Dec 23, 2019

⌛ Testing commit e6d638f with merge 8afd1dc...

bors added a commit that referenced this pull request Dec 23, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
8afd1dc 
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
Copy link
Collaborator

bors commented Dec 23, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

@bors retry

changlog was there. Maybe it was the second : in the changelog?

bors added a commit that referenced this pull request Dec 23, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
2cc8c39 
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
Copy link
Collaborator

bors commented Dec 23, 2019

⌛ Testing commit e6d638f with merge 2cc8c39...

@bors
Copy link
Collaborator

bors commented Dec 23, 2019

💔 Test failed - checks-travis

@llogiq
Copy link
Contributor Author

llogiq commented Dec 23, 2019

@bors retry

@bors
Copy link
Collaborator

bors commented Dec 23, 2019

⌛ Testing commit e6d638f with merge d5dbe1d...

bors added a commit that referenced this pull request Dec 23, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
d5dbe1d 
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 mentioned this pull request Dec 23, 2019
Merged
@bors
Copy link
Collaborator

bors commented Dec 23, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

Huh, just tested the changlog check locally and it worked with the commit message of d5dbe1d

@phansch
Copy link
Member

phansch commented Dec 24, 2019

@bors retry

@bors
Copy link
Collaborator

bors commented Dec 24, 2019

⌛ Testing commit e6d638f with merge 433d9e8...

bors added a commit that referenced this pull request Dec 24, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
433d9e8 
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
Copy link
Collaborator

bors commented Dec 24, 2019

💔 Test failed - checks-travis

@phansch
Copy link
Member

phansch commented Dec 24, 2019

UnicodeEncodeError: 'ascii' codec 🤔

@phansch
Copy link
Member

phansch commented Dec 24, 2019

@bors retry (maybe it was the – character?)

@bors
Copy link
Collaborator

bors commented Dec 24, 2019

⌛ Testing commit e6d638f with merge ec6d44d...

bors added a commit that referenced this pull request Dec 24, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
ec6d44d 
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
Copy link
Collaborator

bors commented Dec 24, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 24, 2019

📌 Commit 40435ac has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Dec 24, 2019

⌛ Testing commit 40435ac with merge 1837cbc...

bors added a commit that referenced this pull request Dec 24, 2019
@bors
Auto merge of #4885 - rust-lang:mut-key-types, r=flip1995
1837cbc 
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
Copy link
Collaborator

bors commented Dec 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 1837cbc to master...

@bors bors merged commit 40435ac into master Dec 24, 2019
This was referenced Dec 24, 2019
Merged
Closed
Merged
@flip1995 flip1995 deleted the mut-key-types branch December 24, 2019 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lint on manual Hash, *Ord, *Eq impls that access a *Cell
4 participants
@llogiq @flip1995 @bors @phansch