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

Exclude self-by-value trait methods implemented on Box<T> from boxed_local #1478

Closed
dtolnay opened this issue Jan 27, 2017 · 4 comments
Closed
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have T-middle Type: Probably requires verifiying types

Comments

@dtolnay
Copy link
Member

dtolnay commented Jan 27, 2017

Simplest case:

trait Trait {
    fn f(self);
}

impl<T> Trait for Box<T> {
    fn f(self) {}
}
warning: local variable doesn't need to be boxed here, #[warn(boxed_local)] on by default
 --> src/main.rs:9:10
  |
9 |     fn f(self) {}
  |          ^^^^

More realistic case that we hit in dtolnay/syn#85:

trait LiftOnce<T, U> {
    type Output;
    fn lift<F>(self, f: F) -> Self::Output where F: FnOnce(T) -> U;
}

impl<T, U> LiftOnce<T, U> for Box<T> {
    type Output = Box<U>;
    fn lift<F>(self, f: F) -> Box<U> where F: FnOnce(T) -> U {
        Box::new(f(*self))
    }
}

I would consider this a false positive.

@sinkuu
Copy link
Contributor

sinkuu commented Jan 27, 2017

There is also false-positive with FnBox::call_box. Maybe all self: Box<_> should be excluded from boxed_local lint.

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing T-middle Type: Probably requires verifiying types labels Jan 27, 2017
@mcarton
Copy link
Member

mcarton commented Jan 27, 2017

@oli-obk E-easy? Do we have a method to check if a type implements Foo<…> for any now? (here Box<T> for any T).

@oli-obk
Copy link
Contributor

oli-obk commented Jan 28, 2017

I'd have just excluded all impl trait for Box methods.

@phansch
Copy link
Member

phansch commented Oct 16, 2018

Fixed via #3321 (feel free to re-open if it's still a problem)

@phansch phansch closed this as completed Oct 16, 2018
@rustbot rustbot added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

7 participants