hir-ty: walk container exprs for unused_must_use#22405
Open
MavenRain wants to merge 2 commits into
Open
Conversation
ExprValidator::check_unused_must_use previously matched only Expr::Call
and Expr::MethodCall directly on the expression of a Statement::Expr
with semicolon. This left several stmt-with-semi cases unwarned:
blocks whose tail is a #[must_use] call (`{ produces() };`),
unsafe-blocks (`unsafe { produces() };`), if/else arms, match arms,
and const blocks.
Refactors check_unused_must_use into a recursive collector that walks
into Expr::Block { tail, .. }, Expr::Unsafe { tail, .. },
Expr::If { then_branch, else_branch, .. }, Expr::Match { arms, .. },
and Expr::Const(inner) before applying the existing leaf check (callee
or method #[must_use] attribute, or #[must_use] ADT return type).
The diagnostic stays at the leaf call so the span points to the actual
must_use producer rather than the wrapping block.
Adds ide-diagnostics tests covering each new container case (block
tail, unsafe block tail, nested blocks, if/else branches, match arms,
const block, #[must_use] ADT through a block) and lock-in tests
confirming that the existing RustcLint plumbing already honors
#[allow(unused_must_use)], #[allow(unused)], and
#[deny(unused_must_use)] attribute resolution.
Follow-up to rust-lang#22239.
Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
rustbot
added
the
S-waiting-on-review
ChayimFriedman2
requested changes
May 19, 2026
| fn collect_unused_must_use( | ||
| &self, | ||
| expr: ExprId, | ||
| diags: &mut Vec<BodyValidationDiagnostic<'db>>, |
Contributor
There was a problem hiding this comment.
You can just push to self.diagnostics.
| // Walk through container expressions so that the value-producing leaf is | ||
| // checked even when wrapped in a block, `unsafe { .. }`, `if`/`match`, or | ||
| // a `const { .. }` block. | ||
| match &self.body[expr] { |
Contributor
There was a problem hiding this comment.
Or better, there is no need for a recursion here, given a block has up to one tail expression. Instead loop until you reach it.
MavenRain
added a commit
to MavenRain/rust-analyzer
that referenced
this pull request
May 20, 2026
Address ChayimFriedman2 review feedback on rust-lang#22405: * Push directly to self.diagnostics rather than threading a &mut Vec parameter through the recursion. Drops the check_unused_must_use / collect_unused_must_use split and the allocation in the thin wrapper. * Replace the recursive single-tail-chain walk (Block/Unsafe/Const) with a loop that reassigns expr until reaching a non-chain leaf. Branching containers (If/Match) still recurse since they have multiple arms. The borrow checker required hoisting the Statement::Expr pass out of the main loop in validate_block, since MatchCheckCtx holds &self.infcx and the must_use pass now needs &mut self. The two passes are independent (must_use on expression statements, non_exhaustive_let on let statements) so order is free. 19 unused_must_use tests + 981 hir-ty tests + 693 ide-diagnostics tests pass. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
This comment has been minimized.
This comment has been minimized.
Address ChayimFriedman2 review feedback on rust-lang#22405: * Push directly to self.diagnostics rather than threading a &mut Vec parameter through the recursion. Drops the check_unused_must_use / collect_unused_must_use split and the allocation in the thin wrapper. * Replace the recursive single-tail-chain walk (Block/Unsafe/Const) with a loop that reassigns expr until reaching a non-chain leaf. Branching containers (If/Match) still recurse since they have multiple arms. The borrow checker required hoisting the Statement::Expr pass out of the main loop in validate_block, since MatchCheckCtx holds &self.infcx and the must_use pass now needs &mut self. The two passes are independent (must_use on expression statements, non_exhaustive_let on let statements) so order is free. 19 unused_must_use tests + 981 hir-ty tests + 693 ide-diagnostics tests pass. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
MavenRain
force-pushed
the
feat-must-use-container-exprs
branch
from
2db699b to
ffe5641
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ExprValidator::check_unused_must_use previously matched only Expr::Call and Expr::MethodCall directly on the expression of a Statement::Expr with semicolon. This left several stmt-with-semi cases unwarned: blocks whose tail is a #[must_use] call (
{ produces() };), unsafe-blocks (unsafe { produces() };), if/else arms, match arms, and const blocks.Refactors check_unused_must_use into a recursive collector that walks into Expr::Block { tail, .. }, Expr::Unsafe { tail, .. }, Expr::If { then_branch, else_branch, .. }, Expr::Match { arms, .. }, and Expr::Const(inner) before applying the existing leaf check (callee or method #[must_use] attribute, or #[must_use] ADT return type). The diagnostic stays at the leaf call so the span points to the actual must_use producer rather than the wrapping block.
Adds ide-diagnostics tests covering each new container case (block tail, unsafe block tail, nested blocks, if/else branches, match arms, const block, #[must_use] ADT through a block) and lock-in tests confirming that the existing RustcLint plumbing already honors #[allow(unused_must_use)], #[allow(unused)], and #[deny(unused_must_use)] attribute resolution.
Follow-up to #22239.