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

Don't lint let_unit_value when needed for type inferenece #8563

Merged
merged 3 commits into from
Apr 15, 2022

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Mar 17, 2022

fixes: #1502

Pinging @dtolnay. I think this is enough to fix the issue. Do you have a good list crates to test this on?

changelog: Don't lint let_unit_value when needed for type inference

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 17, 2022
Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know which crates suffered this problem, but code-wise this looks good to me.

tests/ui/let_unit.rs Outdated Show resolved Hide resolved
@Jarcho Jarcho force-pushed the let_unit_1502 branch 5 times, most recently from 6bf3e41 to ff0e654 Compare March 18, 2022 03:30
@bors
Copy link
Contributor

bors commented Mar 30, 2022

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

@llogiq
Copy link
Contributor

llogiq commented Apr 2, 2022

Sorry for letting this wait for so long, but I lack insight into actual cases in the wild, so I'm not sure if they are all caught

However, as even if this misses some cases it is still a good improvement, r=me with a rebase.

@Jarcho
Copy link
Contributor Author

Jarcho commented Apr 5, 2022

This will now handle block expression and branches when checking if type inference is needed.

The only thing I can think of that would also be a false positive would be a something like:

trait Foo {
    type T;
    fn get_t(&self) -> Self::T;
    fn use_t(&self, _: T);
}
struct Bar;
impl Foo for Bar {
    type T = ();
    ...
}

fn f(bar: Bar) {
    let x = bar.get_t();
    bar.use_t(x);
}

But I'm pretty sure that already doesn't lint.

@Jarcho Jarcho force-pushed the let_unit_1502 branch 2 times, most recently from a81ac09 to 4ec407c Compare April 5, 2022 02:18
@bors
Copy link
Contributor

bors commented Apr 11, 2022

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

@llogiq
Copy link
Contributor

llogiq commented Apr 14, 2022

Ok, I might be offline for a while, but r+ when rebased. Thanks and again sorry for the wait.

@bors delegate+

@bors
Copy link
Contributor

bors commented Apr 14, 2022

✌️ @Jarcho can now approve this pull request

@Jarcho
Copy link
Contributor Author

Jarcho commented Apr 15, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 15, 2022

📌 Commit 70f7c62 has been approved by Jarcho

@bors
Copy link
Contributor

bors commented Apr 15, 2022

⌛ Testing commit 70f7c62 with merge 0bc93b6...

@bors
Copy link
Contributor

bors commented Apr 15, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 0bc93b6 to master...

tgross35 added a commit to tgross35/linux that referenced this pull request Jul 16, 2023
Currently, 'rustc_common_flags' lives in the top-level 'Makefile'.
This means that we need to touch this whenever adjustments to Rust's CLI
configuration are desired, such as changing what lints Clippy should
emit.

This patch moves these flags to a separate file within 'rust/' so that
necessary changes can be made without affecting 'Makefile'. It copies
the behavior currently used with 'bindgen_parameters', which accepts
comments.

Additionally, 'let_unit_value` is no longer specifically named as it is
now part of the 'style' group [1].

[1]: rust-lang/rust-clippy#8563

Signed-off-by: Trevor Gross <tmgross@umich.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Let_unit_value false positive when return type is type parameter
4 participants