From a430846e40a5ea8b0dea4fe0ea3c146b70bc15c8 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 23 May 2019 22:56:23 +0100 Subject: [PATCH 1/2] Warn for #[must_use] in tuples --- src/librustc_lint/unused.rs | 89 ++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 34f7e04c16420..a1a61a9b1bf42 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -47,42 +47,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { return; } - let t = cx.tables.expr_ty(&expr); - let type_permits_lack_of_use = if t.is_unit() + let ty = cx.tables.expr_ty(&expr); + let type_permits_lack_of_use = if ty.is_unit() || cx.tcx.is_ty_uninhabited_from( - cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), t) + cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), ty) { true } 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 - } - 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; - } - } - } - must_use - } - _ => false, - } + check_must_use_ty(cx, ty, s.span) }; let mut fn_warned = false; @@ -108,7 +80,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { _ => None }; if let Some(def_id) = maybe_def_id { - fn_warned = check_must_use(cx, def_id, s.span, "return value of ", ""); + fn_warned = check_must_use_def(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.) @@ -162,10 +134,55 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { cx.span_lint(UNUSED_RESULTS, s.span, "unused result"); } - fn check_must_use( + // Returns whether an error has been emitted (and thus another does not need to be later). + fn check_must_use_ty( + cx: &LateContext<'_, '_>, + ty: ty::Ty<'_>, + span: Span, + ) -> bool { + match ty.sty { + ty::Adt(def, _) => check_must_use_def(cx, def.did, span, "", ""), + ty::Opaque(def, _) => { + let mut has_emitted = 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; + let def_id = trait_ref.def_id; + if check_must_use_def(cx, def_id, span, "implementer of ", "") { + has_emitted = true; + break; + } + } + } + has_emitted + } + ty::Dynamic(binder, _) => { + let mut has_emitted = false; + for predicate in binder.skip_binder().iter() { + if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate { + let def_id = trait_ref.def_id; + if check_must_use_def(cx, def_id, span, "", " trait object") { + has_emitted = true; + break; + } + } + } + has_emitted + } + ty::Tuple(ref tys) => { + tys.iter().map(|k| k.expect_ty()).any(|ty| { + check_must_use_ty(cx, ty, span) + }) + } + _ => false, + } + } + + // Returns whether an error has been emitted (and thus another does not need to be later). + fn check_must_use_def( cx: &LateContext<'_, '_>, def_id: DefId, - sp: Span, + span: Span, descr_pre_path: &str, descr_post_path: &str, ) -> bool { @@ -173,7 +190,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { if attr.check_name(sym::must_use) { let msg = format!("unused {}`{}`{} that must be used", descr_pre_path, cx.tcx.def_path_str(def_id), descr_post_path); - let mut err = cx.struct_span_lint(UNUSED_MUST_USE, sp, &msg); + let mut err = cx.struct_span_lint(UNUSED_MUST_USE, span, &msg); // check for #[must_use = "..."] if let Some(note) = attr.value_str() { err.note(¬e.as_str()); From 68c0ced41604913545cf4108e090e0c5d7272e26 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 23 May 2019 22:56:31 +0100 Subject: [PATCH 2/2] Add test for #[must_use] in tuples --- src/librustc_lint/unused.rs | 4 ++-- src/test/ui/lint/must_use-tuple.rs | 5 +++++ src/test/ui/lint/must_use-tuple.stderr | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/lint/must_use-tuple.rs create mode 100644 src/test/ui/lint/must_use-tuple.stderr diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index a1a61a9b1bf42..2567018134e35 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -1,7 +1,7 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::DefId; use rustc::lint; -use rustc::ty; +use rustc::ty::{self, Ty}; use rustc::ty::adjustment; use rustc_data_structures::fx::FxHashMap; use lint::{LateContext, EarlyContext, LintContext, LintArray}; @@ -137,7 +137,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { // Returns whether an error has been emitted (and thus another does not need to be later). fn check_must_use_ty( cx: &LateContext<'_, '_>, - ty: ty::Ty<'_>, + ty: Ty<'_>, span: Span, ) -> bool { match ty.sty { diff --git a/src/test/ui/lint/must_use-tuple.rs b/src/test/ui/lint/must_use-tuple.rs new file mode 100644 index 0000000000000..bc972d7ffc391 --- /dev/null +++ b/src/test/ui/lint/must_use-tuple.rs @@ -0,0 +1,5 @@ +#![deny(unused_must_use)] + +fn main() { + (Ok::<(), ()>(()),); //~ ERROR unused `std::result::Result` that must be used +} diff --git a/src/test/ui/lint/must_use-tuple.stderr b/src/test/ui/lint/must_use-tuple.stderr new file mode 100644 index 0000000000000..67d1ec01966f9 --- /dev/null +++ b/src/test/ui/lint/must_use-tuple.stderr @@ -0,0 +1,15 @@ +error: unused `std::result::Result` that must be used + --> $DIR/must_use-tuple.rs:4:5 + | +LL | (Ok::<(), ()>(()),); + | ^^^^^^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/must_use-tuple.rs:1:9 + | +LL | #![deny(unused_must_use)] + | ^^^^^^^^^^^^^^^ + = note: this `Result` may be an `Err` variant, which should be handled + +error: aborting due to previous error +