Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,11 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
expr: &hir::Expr<'_>,
span: Span,
) -> Option<MustUsePath> {
if ty.is_unit()
|| !ty.is_inhabited_from(
cx.tcx,
cx.tcx.parent_module(expr.hir_id).to_def_id(),
cx.typing_env(),
)
{
if ty.is_unit() {
return Some(MustUsePath::Suppressed);
}
let parent_mod_did = cx.tcx.parent_module(expr.hir_id).to_def_id();
if !ty.is_inhabited_from(cx.tcx, parent_mod_did, cx.typing_env()) {
return Some(MustUsePath::Suppressed);
}

Expand All @@ -293,6 +291,30 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
is_ty_must_use(cx, pinned_ty, expr, span)
.map(|inner| MustUsePath::Pinned(Box::new(inner)))
}
// Suppress warnings on `Result<(), UninhabitedType>` (e.g. `Result<(), !>`).
ty::Adt(def, args)
if cx.tcx.is_diagnostic_item(sym::Result, def.did())
&& args.type_at(0).is_unit()
&& !args.type_at(1).is_inhabited_from(
cx.tcx,
parent_mod_did,
cx.typing_env(),
) =>
{
Some(MustUsePath::Suppressed)
}
// Suppress warnings on `ControlFlow<Uninhabited, ()>` (e.g. `ControlFlow<!, ()>`).
ty::Adt(def, args)
if cx.tcx.is_diagnostic_item(sym::ControlFlow, def.did())
&& args.type_at(1).is_unit()
&& !args.type_at(0).is_inhabited_from(
cx.tcx,
parent_mod_did,
cx.typing_env(),
Comment on lines +309 to +313
Copy link
Member

Choose a reason for hiding this comment

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

This feels weird to me. ControlFlow is all about not giving a particular "human intent" to one or the other (just getting the ? polarity you want), so I would expect ControlFlow<(), !> and ControlFlow<!, ()> to either both lint or both not lint.

(Not supporting Result<!, ()> I understand since we generally tell people not to use () there in the first place, but for ControlFlow both ways are plausible.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@scottmcm I don't have any strong opinions about ControlFlow here. My own impression was that it'd be confusing to do that, but I'd love to hear feedback from other users of ControlFlow.

Copy link
Member Author

Choose a reason for hiding this comment

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

) =>
{
Some(MustUsePath::Suppressed)
}
ty::Adt(def, _) => is_def_must_use(cx, def.did(), span),
ty::Alias(ty::Opaque | ty::Projection, ty::AliasTy { def_id: def, .. }) => {
elaborate(cx.tcx, cx.tcx.explicit_item_self_bounds(def).iter_identity_copied())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub enum MyUninhabited {}

#[non_exhaustive]
pub enum MyUninhabitedNonexhaustive {}
101 changes: 101 additions & 0 deletions tests/ui/lint/unused/must_use-result-unit-uninhabited.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
//@ edition: 2024
//@ aux-crate:dep=must_use_result_unit_uninhabited_extern_crate.rs

#![deny(unused_must_use)]
#![feature(never_type)]

use core::ops::{ControlFlow, ControlFlow::Continue};
use dep::{MyUninhabited, MyUninhabitedNonexhaustive};

fn result_unit_unit() -> Result<(), ()> {
Ok(())
}

fn result_unit_infallible() -> Result<(), core::convert::Infallible> {
Ok(())
}

fn result_unit_never() -> Result<(), !> {
Ok(())
}

fn result_unit_myuninhabited() -> Result<(), MyUninhabited> {
Ok(())
}

fn result_unit_myuninhabited_nonexhaustive() -> Result<(), MyUninhabitedNonexhaustive> {
Ok(())
}

trait AssocType {
type Error;
}

struct S1;
impl AssocType for S1 {
type Error = !;
}

struct S2;
impl AssocType for S2 {
type Error = ();
}

fn result_unit_assoctype<AT: AssocType>(_: AT) -> Result<(), AT::Error> {
Ok(())
}

trait UsesAssocType {
type Error;
fn method_use_assoc_type(&self) -> Result<(), Self::Error>;
}

impl UsesAssocType for S1 {
type Error = !;
fn method_use_assoc_type(&self) -> Result<(), Self::Error> {
Ok(())
}
}

impl UsesAssocType for S2 {
type Error = ();
fn method_use_assoc_type(&self) -> Result<(), Self::Error> {
Err(())
}
}

fn controlflow_unit() -> ControlFlow<()> {
Continue(())
}

fn controlflow_infallible_unit() -> ControlFlow<core::convert::Infallible, ()> {
Continue(())
}

fn controlflow_never() -> ControlFlow<!> {
Continue(())
}

fn main() {
result_unit_unit(); //~ ERROR: unused `Result` that must be used
result_unit_infallible();
result_unit_never();
result_unit_myuninhabited();
result_unit_myuninhabited_nonexhaustive(); //~ ERROR: unused `Result` that must be used
result_unit_assoctype(S1);
result_unit_assoctype(S2); //~ ERROR: unused `Result` that must be used
S1.method_use_assoc_type();
S2.method_use_assoc_type(); //~ ERROR: unused `Result` that must be used

controlflow_unit(); //~ ERROR: unused `ControlFlow` that must be used
controlflow_infallible_unit();
controlflow_never();
}

trait AssocTypeBeforeMonomorphisation {
type Error;
fn generate(&self) -> Result<(), Self::Error>;
fn process(&self) {
self.generate(); //~ ERROR: unused `Result` that must be used
}
}
78 changes: 78 additions & 0 deletions tests/ui/lint/unused/must_use-result-unit-uninhabited.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
error: unused `Result` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:80:5
|
LL | result_unit_unit();
| ^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
note: the lint level is defined here
--> $DIR/must_use-result-unit-uninhabited.rs:4:9
|
LL | #![deny(unused_must_use)]
| ^^^^^^^^^^^^^^^
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = result_unit_unit();
| +++++++

error: unused `Result` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:84:5
|
LL | result_unit_myuninhabited_nonexhaustive();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = result_unit_myuninhabited_nonexhaustive();
| +++++++

error: unused `Result` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:86:5
|
LL | result_unit_assoctype(S2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = result_unit_assoctype(S2);
| +++++++

error: unused `Result` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:88:5
|
LL | S2.method_use_assoc_type();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = S2.method_use_assoc_type();
| +++++++

error: unused `ControlFlow` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:90:5
|
LL | controlflow_unit();
| ^^^^^^^^^^^^^^^^^^
|
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = controlflow_unit();
| +++++++

error: unused `Result` that must be used
--> $DIR/must_use-result-unit-uninhabited.rs:99:9
|
LL | self.generate();
| ^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = self.generate();
| +++++++

error: aborting due to 6 previous errors

Loading