-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: let-and-return suggests invalid cast (#16135)
#16147
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
8a0f8d1 to
56be9c3
Compare
|
The test you have still works with the extra cast, it's just unnecessary. The type cast is needed to work around deficiencies in type inference: struct S;
trait T {}
impl T for S {}
fn f(x: *const S) -> *const dyn T {
Clone::clone(&x) as _
} |
dfdc327 to
ec26590
Compare
ec26590 to
ba90993
Compare
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.
You aren't including a test for the actual issue. Just copy the function from the issue since using as _ in that case messes up type inference.
| mod issue16135 { | ||
| struct S; | ||
| trait T {} | ||
| impl T for S {} | ||
|
|
||
| fn f(x: *const S) -> *const dyn T { | ||
| Clone::clone(&x) as _ | ||
| } | ||
| } |
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.
This doesn't test the lint. The function body should be:
let x = Clone::clone(&x);
xThere 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.
You're absolutely right. I'll try to get to this asap.
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, that doesn't work.
struct S;
trait T {}
impl T for S {}
fn f(x: *const S) -> *const dyn T {
Clone::clone(&x) as _
}Doesn't actually create an error, so the lint was valid.
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.
That not cause a lint warning is why you need to change the test. Right now the test contains what the code should be after rustfix runs, but the test needs to make sure we don't make a broken suggestion. Hence you need to change the function's body to what I said earlier so that it will actually trigger let_and_return.
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.
That, I understand, but I'd love to be able to reproduce the error from the original issue in the test. This test doesn't have that error.
|
Reminder, once the PR becomes ready for a review, use |
Simple PR that handles a special case where a raw pointer was invalidly cast via
as _.First PR, would appreciate feedback.
changelog: [
let_and_return]: fix lint suggestion for invalid cast to raw pointer.