From 5137bb3ceb67f14e33e34a5198c7f9db7bbbd836 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Wed, 7 Jan 2026 19:31:52 +0100 Subject: [PATCH] Handle `Result` and `ControlFlow` as `T` wrt `#[must_use]` There is a proposal to change the behaviour of rustc's `must_use` lint to consider `Result` and `ControlFlow` as `T` when `U` is uninhabited. See . This might make the user adding extra `#[must_use]` attributes to functions returning `Result` or `ControlFlow`, which would trigger the `double_must_use` lint in Clippy without the current change. --- clippy_lints/src/functions/must_use.rs | 8 +++++++ clippy_utils/src/ty/mod.rs | 14 +++++++++++-- tests/ui/double_must_use.rs | 28 +++++++++++++++++++++++++ tests/ui/double_must_use.stderr | 26 ++++++++++++++++++----- tests/ui/drop_non_drop.rs | 9 ++++++++ tests/ui/drop_non_drop.stderr | 22 ++++++++++++++----- tests/ui/let_underscore_must_use.rs | 10 +++++++++ tests/ui/let_underscore_must_use.stderr | 10 ++++++++- tests/ui/must_use_candidates.fixed | 12 +++++++++++ tests/ui/must_use_candidates.rs | 10 +++++++++ tests/ui/must_use_candidates.stderr | 28 ++++++++++++++++++++++++- 11 files changed, 163 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index 68532de0368f..2eefe4e09929 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -1,3 +1,4 @@ +use clippy_utils::res::MaybeDef as _; use hir::FnSig; use rustc_errors::Applicability; use rustc_hir::def::Res; @@ -222,6 +223,13 @@ fn check_must_use_candidate<'tcx>( format!("#[must_use] \n{indent}"), Applicability::MachineApplicable, ); + if let Some(msg) = match return_ty(cx, item_id).opt_diag_name(cx) { + Some(sym::ControlFlow) => Some("`ControlFlow` as `C` when `B` is uninhabited"), + Some(sym::Result) => Some("`Result` as `T` when `E` is uninhabited"), + _ => None, + } { + diag.note(format!("a future version of Rust will treat {msg} wrt `#[must_use]`")); + } }); } diff --git a/clippy_utils/src/ty/mod.rs b/clippy_utils/src/ty/mod.rs index a90d64e972c1..34d6f983daae 100644 --- a/clippy_utils/src/ty/mod.rs +++ b/clippy_utils/src/ty/mod.rs @@ -305,10 +305,20 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { } } -// Returns whether the type has #[must_use] attribute +// Returns whether the `ty` has `#[must_use]` attribute. If `ty` is a `Result`/`ControlFlow` +// whose `Err`/`Break` payload is an uninhabited type, the `Ok`/`Continue` payload type +// will be used instead. See . pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { match ty.kind() { - ty::Adt(adt, _) => find_attr!(cx.tcx.get_all_attrs(adt.did()), AttributeKind::MustUse { .. }), + ty::Adt(adt, args) => match cx.tcx.get_diagnostic_name(adt.did()) { + Some(sym::Result) if args.type_at(1).is_privately_uninhabited(cx.tcx, cx.typing_env()) => { + is_must_use_ty(cx, args.type_at(0)) + }, + Some(sym::ControlFlow) if args.type_at(0).is_privately_uninhabited(cx.tcx, cx.typing_env()) => { + is_must_use_ty(cx, args.type_at(1)) + }, + _ => find_attr!(cx.tcx.get_all_attrs(adt.did()), AttributeKind::MustUse { .. }), + }, ty::Foreign(did) => find_attr!(cx.tcx.get_all_attrs(*did), AttributeKind::MustUse { .. }), ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => { // for the Array case we don't need to care for the len == 0 case diff --git a/tests/ui/double_must_use.rs b/tests/ui/double_must_use.rs index 3d4aaa9baa49..71995d9466d9 100644 --- a/tests/ui/double_must_use.rs +++ b/tests/ui/double_must_use.rs @@ -1,5 +1,8 @@ #![warn(clippy::double_must_use)] #![allow(clippy::result_unit_err)] +#![feature(never_type)] + +use std::ops::ControlFlow; #[must_use] pub fn must_use_result() -> Result<(), ()> { @@ -40,6 +43,31 @@ async fn async_must_use_result() -> Result<(), ()> { Ok(()) } +#[must_use] +pub fn must_use_result_with_uninhabited() -> Result<(), !> { + unimplemented!(); +} + +#[must_use] +pub struct T; + +#[must_use] +pub fn must_use_result_with_uninhabited_2() -> Result { + //~^ double_must_use + unimplemented!(); +} + +#[must_use] +pub fn must_use_controlflow_with_uninhabited() -> ControlFlow { + unimplemented!(); +} + +#[must_use] +pub fn must_use_controlflow_with_uninhabited_2() -> ControlFlow { + //~^ double_must_use + unimplemented!(); +} + fn main() { must_use_result(); must_use_tuple(); diff --git a/tests/ui/double_must_use.stderr b/tests/ui/double_must_use.stderr index 555dd8902cac..50b1640c47f5 100644 --- a/tests/ui/double_must_use.stderr +++ b/tests/ui/double_must_use.stderr @@ -1,5 +1,5 @@ error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` - --> tests/ui/double_must_use.rs:5:1 + --> tests/ui/double_must_use.rs:8:1 | LL | pub fn must_use_result() -> Result<(), ()> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | pub fn must_use_result() -> Result<(), ()> { = help: to override `-D warnings` add `#[allow(clippy::double_must_use)]` error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` - --> tests/ui/double_must_use.rs:12:1 + --> tests/ui/double_must_use.rs:15:1 | LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) { = help: either add some descriptive message or remove the attribute error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` - --> tests/ui/double_must_use.rs:19:1 + --> tests/ui/double_must_use.rs:22:1 | LL | pub fn must_use_array() -> [Result<(), ()>; 1] { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,12 +25,28 @@ LL | pub fn must_use_array() -> [Result<(), ()>; 1] { = help: either add some descriptive message or remove the attribute error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` - --> tests/ui/double_must_use.rs:37:1 + --> tests/ui/double_must_use.rs:40:1 | LL | async fn async_must_use_result() -> Result<(), ()> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: either add some descriptive message or remove the attribute -error: aborting due to 4 previous errors +error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` + --> tests/ui/double_must_use.rs:55:1 + | +LL | pub fn must_use_result_with_uninhabited_2() -> Result { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: either add some descriptive message or remove the attribute + +error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]` + --> tests/ui/double_must_use.rs:66:1 + | +LL | pub fn must_use_controlflow_with_uninhabited_2() -> ControlFlow { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: either add some descriptive message or remove the attribute + +error: aborting due to 6 previous errors diff --git a/tests/ui/drop_non_drop.rs b/tests/ui/drop_non_drop.rs index 0345e8670ab8..e6433de5163d 100644 --- a/tests/ui/drop_non_drop.rs +++ b/tests/ui/drop_non_drop.rs @@ -6,6 +6,11 @@ fn make_result(t: T) -> Result { Ok(t) } +// The return type should behave as `T` as the `Err` variant is uninhabited +fn make_result_uninhabited_err(t: T) -> Result { + Ok(t) +} + #[must_use] fn must_use(t: T) -> T { t @@ -41,4 +46,8 @@ fn main() { // Don't lint drop(Baz(Bar)); + + // Lint + drop(make_result_uninhabited_err(Foo)); + //~^ drop_non_drop } diff --git a/tests/ui/drop_non_drop.stderr b/tests/ui/drop_non_drop.stderr index b431c62c92c5..567a820990c6 100644 --- a/tests/ui/drop_non_drop.stderr +++ b/tests/ui/drop_non_drop.stderr @@ -1,11 +1,11 @@ error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes - --> tests/ui/drop_non_drop.rs:22:5 + --> tests/ui/drop_non_drop.rs:27:5 | LL | drop(Foo); | ^^^^^^^^^ | note: argument has type `main::Foo` - --> tests/ui/drop_non_drop.rs:22:10 + --> tests/ui/drop_non_drop.rs:27:10 | LL | drop(Foo); | ^^^ @@ -13,16 +13,28 @@ LL | drop(Foo); = help: to override `-D warnings` add `#[allow(clippy::drop_non_drop)]` error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes - --> tests/ui/drop_non_drop.rs:39:5 + --> tests/ui/drop_non_drop.rs:44:5 | LL | drop(Baz(Foo)); | ^^^^^^^^^^^^^^ | note: argument has type `main::Baz` - --> tests/ui/drop_non_drop.rs:39:10 + --> tests/ui/drop_non_drop.rs:44:10 | LL | drop(Baz(Foo)); | ^^^^^^^^ -error: aborting due to 2 previous errors +error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes + --> tests/ui/drop_non_drop.rs:51:5 + | +LL | drop(make_result_uninhabited_err(Foo)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `std::result::Result` + --> tests/ui/drop_non_drop.rs:51:10 + | +LL | drop(make_result_uninhabited_err(Foo)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors diff --git a/tests/ui/let_underscore_must_use.rs b/tests/ui/let_underscore_must_use.rs index 5cf31ec63c66..1ef43c343fb8 100644 --- a/tests/ui/let_underscore_must_use.rs +++ b/tests/ui/let_underscore_must_use.rs @@ -109,4 +109,14 @@ fn main() { #[allow(clippy::let_underscore_must_use)] let _ = a; + + // No lint because this type should behave as `()` + let _ = Result::<_, std::convert::Infallible>::Ok(()); + + #[must_use] + struct T; + + // Lint because this type should behave as `T` + let _ = Result::<_, std::convert::Infallible>::Ok(T); + //~^ let_underscore_must_use } diff --git a/tests/ui/let_underscore_must_use.stderr b/tests/ui/let_underscore_must_use.stderr index 130ea11646fd..23e929b5bf89 100644 --- a/tests/ui/let_underscore_must_use.stderr +++ b/tests/ui/let_underscore_must_use.stderr @@ -96,5 +96,13 @@ LL | let _ = a; | = help: consider explicitly using expression value -error: aborting due to 12 previous errors +error: non-binding `let` on an expression with `#[must_use]` type + --> tests/ui/let_underscore_must_use.rs:120:5 + | +LL | let _ = Result::<_, std::convert::Infallible>::Ok(T); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: aborting due to 13 previous errors diff --git a/tests/ui/must_use_candidates.fixed b/tests/ui/must_use_candidates.fixed index 1e8589cf39d6..c277d92d7c7b 100644 --- a/tests/ui/must_use_candidates.fixed +++ b/tests/ui/must_use_candidates.fixed @@ -107,3 +107,15 @@ pub extern "C" fn unmangled(i: bool) -> bool { fn main() { assert_eq!(1, pure(1)); } + +//~v must_use_candidate +#[must_use] +pub fn result_uninhabited() -> Result { + todo!() +} + +//~v must_use_candidate +#[must_use] +pub fn controlflow_uninhabited() -> std::ops::ControlFlow { + todo!() +} diff --git a/tests/ui/must_use_candidates.rs b/tests/ui/must_use_candidates.rs index 71d546718ae7..224506dacfe4 100644 --- a/tests/ui/must_use_candidates.rs +++ b/tests/ui/must_use_candidates.rs @@ -102,3 +102,13 @@ pub extern "C" fn unmangled(i: bool) -> bool { fn main() { assert_eq!(1, pure(1)); } + +//~v must_use_candidate +pub fn result_uninhabited() -> Result { + todo!() +} + +//~v must_use_candidate +pub fn controlflow_uninhabited() -> std::ops::ControlFlow { + todo!() +} diff --git a/tests/ui/must_use_candidates.stderr b/tests/ui/must_use_candidates.stderr index 5ddbd0260629..f2ec9b265c45 100644 --- a/tests/ui/must_use_candidates.stderr +++ b/tests/ui/must_use_candidates.stderr @@ -60,5 +60,31 @@ LL + #[must_use] LL | pub fn arcd(_x: Arc) -> bool { | -error: aborting due to 5 previous errors +error: this function could have a `#[must_use]` attribute + --> tests/ui/must_use_candidates.rs:108:8 + | +LL | pub fn result_uninhabited() -> Result { + | ^^^^^^^^^^^^^^^^^^ + | + = note: a future version of Rust will treat `Result` as `T` when `E` is uninhabited wrt `#[must_use]` +help: add the attribute + | +LL + #[must_use] +LL | pub fn result_uninhabited() -> Result { + | + +error: this function could have a `#[must_use]` attribute + --> tests/ui/must_use_candidates.rs:113:8 + | +LL | pub fn controlflow_uninhabited() -> std::ops::ControlFlow { + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: a future version of Rust will treat `ControlFlow` as `C` when `B` is uninhabited wrt `#[must_use]` +help: add the attribute + | +LL + #[must_use] +LL | pub fn controlflow_uninhabited() -> std::ops::ControlFlow { + | + +error: aborting due to 7 previous errors