Skip to content

/rust-clippy

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 idea: drop bounds #3773

Closed
notriddle opened this Issue Feb 16, 2019 · 2 comments

Comments

Assignees
No one assigned
Labels
A-unnecessary L-lint good first issue
Projects
None yet
Milestone
No milestone
3 participants
@notriddle @CodesInChaos @oli-obk
@notriddle
Copy link
Contributor

notriddle commented Feb 16, 2019 •
edited

Code in this form should trigger a warning: bounds by drop does not accomplish anything, since t.drop() cannot be directly called, and non-Drop types may have drop glue anyway.

fn foo<T: Drop>(t: T) { ... }
@CodesInChaos

This comment has been minimized.

Sign in to view
Copy link

CodesInChaos commented Feb 16, 2019

Same for:

trait MyTrait: Drop {}

@oli-obk oli-obk added good first issue L-lint A-unnecessary labels Feb 16, 2019

@notriddle

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

notriddle commented Feb 16, 2019

Cool, I've started implementing it then!

@notriddle notriddle referenced this issue Feb 17, 2019

Merged

Add a lint to warn on `T: Drop` bounds #3776

bors added a commit that referenced this issue Feb 19, 2019

@bors
Auto merge of #3776 - notriddle:drop-bounds, r=oli-obk 
Add a lint to warn on `T: Drop` bounds

**What it does:** Checks for generics with `std::ops::Drop` as bounds.

**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.

**Known problems:** None.

**Example:**
```rust
fn foo<T: Drop>() {}
```

Fixes #3773
Loading status checks…
075c212

@bors bors closed this in #3776 Feb 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.