Fix linting false positive when block used as value #141987

ChAoSUnItY · 2025-06-03T21:16:45Z

rustbot · 2025-06-03T21:16:49Z

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tests/ui/lint/unused/block-as-value.rs

compiler/rustc_lint/src/unused.rs

lcnr · 2025-06-05T12:41:56Z

we also need this for unused parens:

#![allow(unused)]
#![warn(unused_parens)]
struct Foo;
fn main() {
    loop {
        if break (Foo) {}
    };
}

please add this as a test, also test if fn_call({ return }) {}. This should lint (i.e. recursing into function calls should update followed_by_block to false

same for try { { return } }. In general, anything that "wraps" its nested expr should set followed_by_block to false

lcnr · 2025-07-03T13:49:40Z

@rustbot author

rustbot · 2025-07-03T13:49:46Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

bors · 2025-07-28T09:38:34Z

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

Dylan-DPC · 2025-09-22T19:06:46Z

@ChAoSUnItY any updates on this? thanks

ChAoSUnItY · 2025-09-23T02:26:15Z

Sorry for late reply, I'm currently busy on other projects, and I am unable to find a proper solution towards the unused parens lint, but the unsued braces lint seems to be fine. Will it be better to let this Pull Request dedicates to only unused braces lint fix? So later we can open unused parens in another issue as original issue only mentioned unused braces lint error.

lcnr · 2025-09-23T05:43:55Z

Will it be better to let this Pull Request dedicates to only unused braces lint fix? So later we can open unused parens in another issue as original issue only mentioned unused braces lint error.

sure 🤔 why does using the approach for blocks using followed_by_block not work for parens?

ChAoSUnItY · 2025-09-23T17:33:09Z

There seems to be other logics or conditions that makes this not working. At least that's what I've encountered in June.

lcnr · 2025-09-24T08:00:25Z

alright, would be nice if you still had the changes/a commit from back then so that I could look it at.

I think even just fixing it for blocks is good for now 👍

rustbot · 2025-09-24T11:06:43Z

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

rustbot · 2025-09-24T11:06:45Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ChAoSUnItY · 2025-09-24T12:12:36Z

Weird, I seem to be unable undo accidental stdarch changes by following rustc dev guide?

folkertdev · 2025-09-24T12:59:29Z

Yeah submodules can be extremely annoying when rebasing. I generally resort to just dissolving the commit with git reset HEAD^ and then carefully using git add to add only the files I actually want to update. Then another ./x test or similar should update the stdarch repo to the correct commit again.

ChAoSUnItY · 2025-09-24T20:25:02Z

@rustbot ready

lcnr

this is a bit weird. Why does check_unused_delims_expr still take an explicit followed_by_block argument? I feel like this arg should be redundant by now

View changes since this review

lcnr · 2025-09-25T08:23:40Z

compiler/rustc_lint/src/unused.rs

+                // if ctx == UnusedDelimsCtx::FunctionArg {
+                //     println!("{:?}", self.followed_by_block.last().copied().unwrap_or(false))
+                // }


I forgot to remove 😅

lcnr · 2025-09-25T08:29:14Z

compiler/rustc_lint/src/unused.rs

+                //   - however, this does not lint if block is immediately followed by parent
+                //     expression's block, e.g. `if` and `match`, which may cause false positive.
+                //      ```
+                //      if return { return } {} else {}
+                //      ```
                // - `followed_by_block` is true and the internal expr may contain a `{`


these two bullet points should be unified. separately, why is your added comment indented?

I think it's

the current expression is may immediately be followed by a parents expression's block, e.g. from an if or match, and the internal expression may contain a {

is that right?

Yes, this should be the intended way to describe this scenario

lcnr · 2025-09-25T08:29:41Z

compiler/rustc_lint/src/unused.rs

                if let [stmt] = inner.stmts.as_slice()
                    && let ast::StmtKind::Expr(ref expr) = stmt.kind
                    && !Self::is_expr_delims_necessary(expr, ctx, followed_by_block)
+                    && ((ctx == UnusedDelimsCtx::FunctionArg || ctx == UnusedDelimsCtx::MethodArg)


why this check for ctx?

This is meant to check f({ return }) so it could be lint into f(return), as the callee, UnusedDelimLint::check_expr will directly enter UnusedBraces::check_unused_delims_expr, and I think there's no good and concise way to push a false into UnusedBraces::followed_by_block from UnusedDelimLint without massively refactoring the whole structural logic.

ah, that sucks :/ in a sense the followed_by_block fn arg should be sth like

enum FollowedByBlock { Yes, Inherit, No, }

and we only check the last self.followed_by_block entry if it's Inherit while the parens handler either maps Inherit to false, resultin gin false positives, or conservatively to true?

or hmm, wait, couldn't we have a

trait UnusedDelimLint { fn followed_by_block(&mut self) -> &mut Vec<bool>; }

and also set + unset stuff in UnusedDelimLint::check_expr?

This kind of modification makes things messy in my opinion, I have to admit that this is a good idea though.

Although I have noticed that unused_parens is working fine in this case, which is f((return)) will get lint into f(return), not sure if I missed some critical logics here?

function args always get checked with followed_by_block: false (see UnusedDelimLint::check_expr). This would be FollowedByBlock::No.

lcnr · 2025-09-26T09:12:27Z

I would really like for you to leave this in a nicer to read state than you found it 🤔 though that probably is a more significant change. Would be very happy if you could deduplicate the checks based on fn-arg is_followed_by_block vs self.is_followed_by_block, but request a review if that ends up being very difficult.

@rustbot author

rustbot assigned lcnr Jun 3, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 3, 2025

ChAoSUnItY changed the title ~~Fix linting false positive when block used as val~~ Fix linting false positive when block used as value Jun 3, 2025

This comment has been minimized.

Sign in to view

lcnr reviewed Jun 4, 2025

View reviewed changes

tests/ui/lint/unused/block-as-value.rs Outdated Show resolved Hide resolved

ChAoSUnItY force-pushed the fix/unused_braces branch from b7c2339 to 5fda6c1 Compare June 4, 2025 08:54

lcnr reviewed Jun 4, 2025

View reviewed changes

compiler/rustc_lint/src/unused.rs Outdated Show resolved Hide resolved

lcnr reviewed Jun 5, 2025

View reviewed changes

compiler/rustc_lint/src/unused.rs Outdated Show resolved Hide resolved

lcnr reviewed Jun 5, 2025

View reviewed changes

compiler/rustc_lint/src/unused.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2025

ChAoSUnItY force-pushed the fix/unused_braces branch from ae418c4 to 2b85785 Compare September 24, 2025 11:06

This comment has been minimized.

Sign in to view

Fix unused_braces linting

012f029

ChAoSUnItY force-pushed the fix/unused_braces branch from 2b85785 to 012f029 Compare September 24, 2025 19:19

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 24, 2025

lcnr reviewed Sep 25, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2025

Fix linting false positive when block used as value #141987

Are you sure you want to change the base?

Fix linting false positive when block used as value #141987

Uh oh!

Conversation

ChAoSUnItY commented Jun 3, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 3, 2025

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lcnr commented Jun 5, 2025

Uh oh!

lcnr commented Jul 3, 2025

Uh oh!

rustbot commented Jul 3, 2025

Uh oh!

bors commented Jul 28, 2025

Uh oh!

Dylan-DPC commented Sep 22, 2025

Uh oh!

ChAoSUnItY commented Sep 23, 2025

Uh oh!

lcnr commented Sep 23, 2025

Uh oh!

ChAoSUnItY commented Sep 23, 2025

Uh oh!

lcnr commented Sep 24, 2025

Uh oh!

rustbot commented Sep 24, 2025

Uh oh!

rustbot commented Sep 24, 2025

Uh oh!

This comment has been minimized.

ChAoSUnItY commented Sep 24, 2025

Uh oh!

folkertdev commented Sep 24, 2025

Uh oh!

ChAoSUnItY commented Sep 24, 2025

Uh oh!

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

ChAoSUnItY Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

ChAoSUnItY Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

ChAoSUnItY Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

ChAoSUnItY Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Sep 25, 2025

Choose a reason for hiding this comment

ChAoSUnItY commented Jun 3, 2025 •

edited by rustbot

Loading

lcnr left a comment •

edited by rustbot

Loading

ChAoSUnItY Sep 25, 2025 •

edited

Loading

lcnr Sep 25, 2025 •

edited

Loading

ChAoSUnItY Sep 25, 2025 •

edited

Loading