-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix [needless_return
] false negative
#13214
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
Conversation
…ws a value Fixes rust-lang#12907 changelog: Fix [`needless_return`] false negative when returned expression borrows a value
Looks good to me, thanks! Let's just wait for the assigned reviewer to take a look before approving. |
fn issue12907() -> String { | ||
return "".split("").next().unwrap().to_string(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this just needs a test for the mentioned RefCell
or any other type that fails to compile. But I think the significant drop check is adequate
There is the rustc_insigificant_dtor
attribute. (I didn't even know this existed before checking.) This is applied to certain types like Rc
, HashMap
, and array::IntoIter
. Not sure whether this attribute changes the behavior (i.e., compiling would succeed) but it'd be a good idea to check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this just needs a test for the mentioned
RefCell
or any other type that fails to compile. But I think the significant drop check is adequate
So should i delete the test then?
Not sure whether this attribute changes the behavior (i.e., compiling would succeed) but it'd be a good idea to check.
Yes, it compiles with the rustc_insigificant_dtor attribute and the lint is triggered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I meant the only change it needs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a very similar test to the one mentioned:
fn temporary_outlives_local() -> String {
let x = RefCell::<String>::default();
return x.borrow().clone();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that does the same thing 👍 missed that before
Thank you! @bors r+ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #12907
changelog: Fix [
needless_return
] false negative when returned expression borrows a value.