From b55717f9b0023f58b7ca2fab77a0b33206076ac4 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 3 Nov 2018 19:23:05 +0000 Subject: [PATCH 1/6] Use general uninhabitedness checking --- src/librustc_lint/unused.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 7eab7d2100290..6383ca69fe5ca 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -60,19 +60,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { } let t = cx.tables.expr_ty(&expr); - // FIXME(varkor): replace with `t.is_unit() || t.conservative_is_uninhabited()`. - let type_permits_no_use = match t.sty { - ty::Tuple(ref tys) if tys.is_empty() => true, - ty::Never => true, - ty::Adt(def, _) => { - if def.variants.is_empty() { - true - } else { - check_must_use(cx, def.did, s.span, "") - } + let type_permits_lack_of_use = if t.is_unit() + || cx.tcx.is_ty_uninhabited_from(cx.tcx.hir.get_module_parent(expr.id), t) { + true + } else { + match t.sty { + ty::Adt(def, _) => check_must_use(cx, def.did, s.span, ""), + _ => false, } - _ => false, - }; + } let mut fn_warned = false; let mut op_warned = false; @@ -99,7 +95,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { if let Some(def) = maybe_def { let def_id = def.def_id(); fn_warned = check_must_use(cx, def_id, s.span, "return value of "); - } else if type_permits_no_use { + } else if type_permits_lack_of_use { // We don't warn about unused unit or uninhabited types. // (See https://github.com/rust-lang/rust/issues/43806 for details.) return; @@ -148,7 +144,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { op_warned = true; } - if !(type_permits_no_use || fn_warned || op_warned) { + if !(type_permits_lack_of_use || fn_warned || op_warned) { cx.span_lint(UNUSED_RESULTS, s.span, "unused result"); } From cb5520bc4865473971281ddb3e33a29a256109c8 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 3 Nov 2018 20:08:30 +0000 Subject: [PATCH 2/6] Recognise #[must_use] on traits, affecting impl Trait --- src/librustc_lint/unused.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 6383ca69fe5ca..f8030fa6ec7a8 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -66,9 +66,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { } else { match t.sty { ty::Adt(def, _) => check_must_use(cx, def.did, s.span, ""), + ty::Opaque(def, _) => { + let mut must_use = false; + for (predicate, _) in cx.tcx.predicates_of(def).predicates { + if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate { + let trait_ref = poly_trait_predicate.skip_binder().trait_ref; + if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ") { + must_use = true; + break; + } + } + } + must_use + } _ => false, } - } + }; let mut fn_warned = false; let mut op_warned = false; From 122886842e74467e24935793f79d7443eb509e35 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 3 Nov 2018 20:08:38 +0000 Subject: [PATCH 3/6] Test for #[must_use] on traits --- src/test/ui/lint/must_use-trait.rs | 22 ++++++++++++++++++++++ src/test/ui/lint/must_use-trait.stderr | 14 ++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 src/test/ui/lint/must_use-trait.rs create mode 100644 src/test/ui/lint/must_use-trait.stderr diff --git a/src/test/ui/lint/must_use-trait.rs b/src/test/ui/lint/must_use-trait.rs new file mode 100644 index 0000000000000..23df4fa6132d3 --- /dev/null +++ b/src/test/ui/lint/must_use-trait.rs @@ -0,0 +1,22 @@ +#![deny(unused_must_use)] + +#[must_use] +trait Critical {} + +trait NotSoCritical {} + +trait DecidedlyUnimportant {} + +struct Anon; + +impl Critical for Anon {} +impl NotSoCritical for Anon {} +impl DecidedlyUnimportant for Anon {} + +fn get_critical() -> impl NotSoCritical + Critical + DecidedlyUnimportant { + Anon {} +} + +fn main() { + get_critical(); //~ ERROR unused implementer of `Critical` that must be used +} diff --git a/src/test/ui/lint/must_use-trait.stderr b/src/test/ui/lint/must_use-trait.stderr new file mode 100644 index 0000000000000..94f5f4f40d2d5 --- /dev/null +++ b/src/test/ui/lint/must_use-trait.stderr @@ -0,0 +1,14 @@ +error: unused implementer of `Critical` that must be used + --> $DIR/must_use-trait.rs:21:5 + | +LL | get_critical(); //~ ERROR unused implementer of `Critical` that must be used + | ^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/must_use-trait.rs:1:9 + | +LL | #![deny(unused_must_use)] + | ^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 9178eb41d3e1ec2d96ff6bdbb19a0bccb7a45aff Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 3 Nov 2018 20:24:24 +0000 Subject: [PATCH 4/6] Handle trait objects --- src/librustc_lint/unused.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index f8030fa6ec7a8..3bb35b8e4e10e 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -65,13 +65,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { true } else { match t.sty { - ty::Adt(def, _) => check_must_use(cx, def.did, s.span, ""), + ty::Adt(def, _) => check_must_use(cx, def.did, s.span, "", ""), ty::Opaque(def, _) => { let mut must_use = false; for (predicate, _) in cx.tcx.predicates_of(def).predicates { if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate { let trait_ref = poly_trait_predicate.skip_binder().trait_ref; - if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ") { + if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ", "") { + must_use = true; + break; + } + } + } + must_use + } + ty::Dynamic(binder, _) => { + let mut must_use = false; + for predicate in binder.skip_binder().iter() { + if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate { + if check_must_use(cx, trait_ref.def_id, s.span, "", " trait object") { must_use = true; break; } @@ -107,7 +119,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { }; if let Some(def) = maybe_def { let def_id = def.def_id(); - fn_warned = check_must_use(cx, def_id, s.span, "return value of "); + fn_warned = check_must_use(cx, def_id, s.span, "return value of ", ""); } else if type_permits_lack_of_use { // We don't warn about unused unit or uninhabited types. // (See https://github.com/rust-lang/rust/issues/43806 for details.) @@ -161,11 +173,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { cx.span_lint(UNUSED_RESULTS, s.span, "unused result"); } - fn check_must_use(cx: &LateContext, def_id: DefId, sp: Span, describe_path: &str) -> bool { + fn check_must_use( + cx: &LateContext, + def_id: DefId, + sp: Span, + descr_pre_path: &str, + descr_post_path: &str, + ) -> bool { for attr in cx.tcx.get_attrs(def_id).iter() { if attr.check_name("must_use") { - let msg = format!("unused {}`{}` that must be used", - describe_path, cx.tcx.item_path_str(def_id)); + let msg = format!("unused {}`{}`{} that must be used", + descr_pre_path, descr_post_path, cx.tcx.item_path_str(def_id)); let mut err = cx.struct_span_lint(UNUSED_MUST_USE, sp, &msg); // check for #[must_use = "..."] if let Some(note) = attr.value_str() { From 0ab70fab1970ea665c0969f04adc66195703a7bc Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 3 Nov 2018 21:27:35 +0000 Subject: [PATCH 5/6] Fix typo in #[must_use] message --- src/librustc_lint/unused.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 3bb35b8e4e10e..906579a1ae054 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -183,7 +183,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { for attr in cx.tcx.get_attrs(def_id).iter() { if attr.check_name("must_use") { let msg = format!("unused {}`{}`{} that must be used", - descr_pre_path, descr_post_path, cx.tcx.item_path_str(def_id)); + descr_pre_path, cx.tcx.item_path_str(def_id), descr_post_path); let mut err = cx.struct_span_lint(UNUSED_MUST_USE, sp, &msg); // check for #[must_use = "..."] if let Some(note) = attr.value_str() { From 737dec0ec16b2606380ed73166b82e44be080ab5 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 19 Nov 2018 18:54:52 +0000 Subject: [PATCH 6/6] Fix change to predicates --- src/librustc_lint/unused.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 906579a1ae054..fab618d9c8ec8 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -68,7 +68,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { ty::Adt(def, _) => check_must_use(cx, def.did, s.span, "", ""), ty::Opaque(def, _) => { let mut must_use = false; - for (predicate, _) in cx.tcx.predicates_of(def).predicates { + for (predicate, _) in &cx.tcx.predicates_of(def).predicates { if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate { let trait_ref = poly_trait_predicate.skip_binder().trait_ref; if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ", "") {