From 74b097cfae1d08fe4ef135d92931c7dcceddf9e3 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 1 Jul 2019 01:56:11 +0100 Subject: [PATCH 01/17] Extend `#[must_use]` lint to user-defined ADTs --- src/librustc_lint/unused.rs | 51 +++++++++++---- src/test/ui/lint/must_use-adt.rs | 77 +++++++++++++++++++++++ src/test/ui/lint/must_use-adt.stderr | 92 ++++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/lint/must_use-adt.rs create mode 100644 src/test/ui/lint/must_use-adt.stderr diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 3b3995832cb4c..2d170278dfae0 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -1,7 +1,9 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::DefId; +use rustc::hir::HirVec; use rustc::lint; use rustc::ty::{self, Ty}; +use rustc::ty::subst::Subst; use rustc::ty::adjustment; use rustc_data_structures::fx::FxHashMap; use lint::{LateContext, EarlyContext, LintContext, LintArray}; @@ -151,8 +153,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { let descr_pre = &format!("{}boxed ", descr_pre); check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, plural_len) } - ty::Adt(def, _) => { - check_must_use_def(cx, def.did, span, descr_pre, descr_post) + ty::Adt(def, subst) => { + // Check the type itself for `#[must_use]` annotations. + let mut has_emitted = check_must_use_def( + cx, def.did, span, descr_pre, descr_post); + // Check any fields of the type for `#[must_use]` annotations. + // We ignore ADTs with more than one variant for simplicity and to avoid + // false positives. + // Unions are also ignored (though in theory, we could lint if every field of + // a union was `#[must_use]`). + if def.variants.len() == 1 && !def.is_union() { + let fields = match &expr.node { + hir::ExprKind::Struct(_, fields, _) => { + fields.iter().map(|f| &*f.expr).collect() + } + hir::ExprKind::Call(_, args) => args.iter().collect(), + _ => HirVec::new(), + }; + + for variant in &def.variants { + for (i, field) in variant.fields.iter().enumerate() { + let descr_post + = &format!(" in field `{}`", field.ident.as_str()); + let ty = cx.tcx.type_of(field.did).subst(cx.tcx, subst); + let (expr, span) = if let Some(&field) = fields.get(i) { + (field, field.span) + } else { + (expr, span) + }; + has_emitted |= check_must_use_ty( + cx, ty, expr, span, descr_pre, descr_post, plural); + } + } + } + has_emitted } ty::Opaque(def, _) => { let mut has_emitted = false; @@ -202,17 +236,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() { let descr_post = &format!(" in tuple element {}", i); let span = *spans.get(i).unwrap_or(&span); - if check_must_use_ty( - cx, - ty, - expr, - span, - descr_pre, - descr_post, - plural_len - ) { - has_emitted = true; - } + has_emitted |= check_must_use_ty( + cx, ty, expr, span, descr_pre, descr_post, plural_len); } has_emitted } diff --git a/src/test/ui/lint/must_use-adt.rs b/src/test/ui/lint/must_use-adt.rs new file mode 100644 index 0000000000000..a958abfda5b19 --- /dev/null +++ b/src/test/ui/lint/must_use-adt.rs @@ -0,0 +1,77 @@ +#![deny(unused_must_use)] + +#[must_use] +struct S; + +#[must_use] +trait A {} + +struct B; + +impl A for B {} + +struct T(S); + +struct U { + x: (), + y: T, +} + +struct V { + a: S, +} + +struct W { + w: [(u8, Box); 2], + x: u32, + y: (B, B), + z: (S, S), + e: [(u8, Box); 2], + f: S, +} + +fn get_v() -> V { + V { a: S } +} + +struct Z([(u8, Box); 2]); + +fn get_wrapped_arr() -> Z { + Z([(0, Box::new(B)), (0, Box::new(B))]) +} + +fn get_tuple_arr() -> ([(u8, Box); 2],) { + ([(0, Box::new(B)), (0, Box::new(B))],) +} + +struct R { + r: T +} + +struct List(T, Option>); + +fn main() { + S; //~ ERROR unused `S` that must be used + T(S); //~ ERROR unused `S` in field `0` that must be used + U { x: (), y: T(S) }; //~ ERROR unused `S` in field `0` that must be used + get_v(); //~ ERROR unused `S` in field `a` that must be used + V { a: S }; //~ ERROR unused `S` in field `a` that must be used + W { + w: [(0, Box::new(B)), (0, Box::new(B))], + //~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be used + x: 0, + y: (B, B), + z: (S, S), + //~^ unused `S` in tuple element 0 that must be used + //~^^ unused `S` in tuple element 1 that must be used + e: [(0, Box::new(B)), (0, Box::new(B))], + //~^ unused array of boxed `A` trait objects in tuple element 1 that must be used + f: S, //~ ERROR unused `S` in field `f` that must be used + }; + get_wrapped_arr(); + //~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be use + get_tuple_arr(); + //~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be used + R { r: S }; //~ ERROR unused `S` in field `r` that must be used + List(S, Some(Box::new(List(S, None)))); //~ ERROR unused `S` in field `0` that must be used +} diff --git a/src/test/ui/lint/must_use-adt.stderr b/src/test/ui/lint/must_use-adt.stderr new file mode 100644 index 0000000000000..334696807e8ec --- /dev/null +++ b/src/test/ui/lint/must_use-adt.stderr @@ -0,0 +1,92 @@ +error: unused `S` that must be used + --> $DIR/must_use-adt.rs:54:5 + | +LL | S; + | ^^ + | +note: lint level defined here + --> $DIR/must_use-adt.rs:1:9 + | +LL | #![deny(unused_must_use)] + | ^^^^^^^^^^^^^^^ + +error: unused `S` in field `0` that must be used + --> $DIR/must_use-adt.rs:55:7 + | +LL | T(S); + | ^ + +error: unused `S` in field `0` that must be used + --> $DIR/must_use-adt.rs:56:21 + | +LL | U { x: (), y: T(S) }; + | ^ + +error: unused `S` in field `a` that must be used + --> $DIR/must_use-adt.rs:57:5 + | +LL | get_v(); + | ^^^^^^^^ + +error: unused `S` in field `a` that must be used + --> $DIR/must_use-adt.rs:58:12 + | +LL | V { a: S }; + | ^ + +error: unused array of boxed `A` trait objects in tuple element 1 that must be used + --> $DIR/must_use-adt.rs:60:12 + | +LL | w: [(0, Box::new(B)), (0, Box::new(B))], + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unused `S` in tuple element 0 that must be used + --> $DIR/must_use-adt.rs:64:13 + | +LL | z: (S, S), + | ^ + +error: unused `S` in tuple element 1 that must be used + --> $DIR/must_use-adt.rs:64:16 + | +LL | z: (S, S), + | ^ + +error: unused array of boxed `A` trait objects in tuple element 1 that must be used + --> $DIR/must_use-adt.rs:67:12 + | +LL | e: [(0, Box::new(B)), (0, Box::new(B))], + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unused `S` in field `f` that must be used + --> $DIR/must_use-adt.rs:69:12 + | +LL | f: S, + | ^ + +error: unused array of boxed `A` trait objects in tuple element 1 that must be used + --> $DIR/must_use-adt.rs:71:5 + | +LL | get_wrapped_arr(); + | ^^^^^^^^^^^^^^^^^^ + +error: unused array of boxed `A` trait objects in tuple element 1 that must be used + --> $DIR/must_use-adt.rs:73:5 + | +LL | get_tuple_arr(); + | ^^^^^^^^^^^^^^^^ + +error: unused `S` in field `r` that must be used + --> $DIR/must_use-adt.rs:75:12 + | +LL | R { r: S }; + | ^ + +error: unused `S` in field `0` that must be used + --> $DIR/must_use-adt.rs:76:10 + | +LL | List(S, Some(Box::new(List(S, None)))); + | ^ + +error: aborting due to 14 previous errors + From eaf8bf1ff1c380611960ad34a672c9e45db1dc07 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 1 Jul 2019 01:56:55 +0100 Subject: [PATCH 02/17] Ignore unused result in `replace_range` --- src/liballoc/string.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index 639124e26cc20..96700a825c95d 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -1568,7 +1568,7 @@ impl String { Unbounded => {}, }; - unsafe { + let _ = unsafe { self.as_mut_vec() }.splice(range, replace_with.bytes()); } From 49422dfcf5e3f09bbe099c6814d5c4bd75846460 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 22 Jul 2019 21:30:54 +0100 Subject: [PATCH 03/17] Address review comments --- src/librustc_lint/unused.rs | 8 ++++---- src/libstd/panicking.rs | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 2d170278dfae0..deabb6e882aed 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -242,8 +242,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { has_emitted } ty::Array(ty, len) => match len.try_eval_usize(cx.tcx, cx.param_env) { - // If the array is definitely non-empty, we can do `#[must_use]` checking. - Some(n) if n != 0 => { + // Empty arrays won't contain any `#[must_use]` types. + Some(0) => false, + // If the array may be non-empty, we do `#[must_use]` checking. + _ => { let descr_pre = &format!( "{}array{} of ", descr_pre, @@ -251,8 +253,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { ); check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, n as usize + 1) } - // Otherwise, we don't lint, to avoid false positives. - _ => false, } _ => false, } diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 638ce1679b8e9..2d275a4b9b275 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -104,9 +104,7 @@ pub fn set_hook(hook: Box) + 'static + Sync + Send>) { HOOK_LOCK.write_unlock(); if let Hook::Custom(ptr) = old_hook { - #[allow(unused_must_use)] { - Box::from_raw(ptr); - } + mem::drop(Box::from_raw(ptr)); } } } From 02fc4697ec36a91bba036f03c9c36ccd27ca99ef Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 22 Jul 2019 21:31:33 +0100 Subject: [PATCH 04/17] Make UNUSED_MUST_USE deny-by-default --- 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 deabb6e882aed..eded76ff788fb 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -25,7 +25,7 @@ use log::debug; declare_lint! { pub UNUSED_MUST_USE, - Warn, + Deny, "unused result of a type flagged as `#[must_use]`", report_in_external_macro: true } From 13c2571185412556e1371cf7e44cc974d6fe7fd3 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 22 Jul 2019 23:05:32 +0100 Subject: [PATCH 05/17] Fix ignored value --- src/librustc_mir/transform/add_retag.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs index b56a1b263fd6a..169f89ed03018 100644 --- a/src/librustc_mir/transform/add_retag.rs +++ b/src/librustc_mir/transform/add_retag.rs @@ -86,7 +86,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag { .filter(needs_retag) .collect::>(); // Emit their retags. - basic_blocks[START_BLOCK].statements.splice(0..0, + let _ = basic_blocks[START_BLOCK].statements.splice(0..0, places.into_iter().map(|place| Statement { source_info, kind: StatementKind::Retag(RetagKind::FnEntry, box(place)), From deb400010e8896626b137876061afdca3a97ebc9 Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 24 Jul 2019 01:12:29 +0100 Subject: [PATCH 06/17] Fix failing tests --- src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs | 2 +- src/test/ui/issues/auxiliary/issue-2723-a.rs | 2 +- src/test/ui/issues/auxiliary/issue-9906.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs b/src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs index 4cd001ecc9e60..0f48f05185851 100644 --- a/src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs +++ b/src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs @@ -4,7 +4,7 @@ use std::sync::mpsc::{Receiver, channel}; pub fn foo(x: T) -> Receiver { let (tx, rx) = channel(); thread::spawn(move|| { - tx.send(x.clone()); + let _ = tx.send(x.clone()); }); rx } diff --git a/src/test/ui/issues/auxiliary/issue-2723-a.rs b/src/test/ui/issues/auxiliary/issue-2723-a.rs index 661b46d829dfe..edb93548475d9 100644 --- a/src/test/ui/issues/auxiliary/issue-2723-a.rs +++ b/src/test/ui/issues/auxiliary/issue-2723-a.rs @@ -1,3 +1,3 @@ pub unsafe fn f(xs: Vec ) { - xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::>(); + let _ = xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::>(); } diff --git a/src/test/ui/issues/auxiliary/issue-9906.rs b/src/test/ui/issues/auxiliary/issue-9906.rs index 8a3eea790a26a..9a45224086478 100644 --- a/src/test/ui/issues/auxiliary/issue-9906.rs +++ b/src/test/ui/issues/auxiliary/issue-9906.rs @@ -10,6 +10,6 @@ mod other { } pub fn foo(){ - 1+1; + let _ = 1 + 1; } } From 82af81a7c4a3489f15e14fa21abf47580f13ac6a Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 28 Jul 2019 22:16:24 +0100 Subject: [PATCH 07/17] Fix more failing tests --- src/test/mir-opt/const_prop/ref_deref.rs | 2 ++ src/test/run-fail/binop-fail-3.rs | 2 +- src/test/run-fail/binop-panic.rs | 2 +- src/test/run-fail/generator-resume-after-panic.rs | 2 +- src/test/run-fail/issue-28934.rs | 2 +- src/test/run-fail/panic-set-unset-handler.rs | 2 +- src/test/run-fail/panic-take-handler-nop.rs | 2 +- 7 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/test/mir-opt/const_prop/ref_deref.rs b/src/test/mir-opt/const_prop/ref_deref.rs index 2d04822c0e789..3398c1c908684 100644 --- a/src/test/mir-opt/const_prop/ref_deref.rs +++ b/src/test/mir-opt/const_prop/ref_deref.rs @@ -1,3 +1,5 @@ +#![allow(unused_must_use)] + fn main() { *(&4); } diff --git a/src/test/run-fail/binop-fail-3.rs b/src/test/run-fail/binop-fail-3.rs index a7696fffda0c2..7bd12997ecb68 100644 --- a/src/test/run-fail/binop-fail-3.rs +++ b/src/test/run-fail/binop-fail-3.rs @@ -5,5 +5,5 @@ fn foo() -> ! { #[allow(resolve_trait_on_defaulted_unit)] fn main() { - foo() == foo(); // these types wind up being defaulted to () + let _ = foo() == foo(); // these types wind up being defaulted to () } diff --git a/src/test/run-fail/binop-panic.rs b/src/test/run-fail/binop-panic.rs index dba5cecc67e11..a8a0ff5250d2b 100644 --- a/src/test/run-fail/binop-panic.rs +++ b/src/test/run-fail/binop-panic.rs @@ -4,5 +4,5 @@ fn my_err(s: String) -> ! { panic!("quux"); } fn main() { - 3_usize == my_err("bye".to_string()); + let _ = 3_usize == my_err("bye".to_string()); } diff --git a/src/test/run-fail/generator-resume-after-panic.rs b/src/test/run-fail/generator-resume-after-panic.rs index 910b4903bf6a3..5a65cc3ce2f1e 100644 --- a/src/test/run-fail/generator-resume-after-panic.rs +++ b/src/test/run-fail/generator-resume-after-panic.rs @@ -15,7 +15,7 @@ fn main() { panic!(); yield; }; - panic::catch_unwind(panic::AssertUnwindSafe(|| { + let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| { let x = Pin::new(&mut g).resume(); })); Pin::new(&mut g).resume(); diff --git a/src/test/run-fail/issue-28934.rs b/src/test/run-fail/issue-28934.rs index 5915372b6920f..b2b786662131b 100644 --- a/src/test/run-fail/issue-28934.rs +++ b/src/test/run-fail/issue-28934.rs @@ -19,5 +19,5 @@ impl<'i, 't> Parser<'i, 't> { fn main() { let x = 0u8; - Parser(&x, &x).parse_nested_block(|input| input.expect_exhausted()).unwrap(); + let _ = Parser(&x, &x).parse_nested_block(|input| input.expect_exhausted()).unwrap(); } diff --git a/src/test/run-fail/panic-set-unset-handler.rs b/src/test/run-fail/panic-set-unset-handler.rs index f8809c2f3889e..e05f170515752 100644 --- a/src/test/run-fail/panic-set-unset-handler.rs +++ b/src/test/run-fail/panic-set-unset-handler.rs @@ -6,6 +6,6 @@ fn main() { panic::set_hook(Box::new(|i| { eprint!("greetings from the panic handler"); })); - panic::take_hook(); + let _ = panic::take_hook(); panic!("foobar"); } diff --git a/src/test/run-fail/panic-take-handler-nop.rs b/src/test/run-fail/panic-take-handler-nop.rs index bb191a38f8473..3a2bc00a39cd0 100644 --- a/src/test/run-fail/panic-take-handler-nop.rs +++ b/src/test/run-fail/panic-take-handler-nop.rs @@ -3,6 +3,6 @@ use std::panic; fn main() { - panic::take_hook(); + let _ = panic::take_hook(); panic!("foobar"); } From 36fb8099f3c57d3a945e7bfdb6a1f3b4fd73ab41 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 29 Jul 2019 21:17:39 +0100 Subject: [PATCH 08/17] Fix incremental tests --- src/test/incremental/change_crate_order/main.rs | 2 +- src/test/incremental/warnings-reemitted.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/incremental/change_crate_order/main.rs b/src/test/incremental/change_crate_order/main.rs index c167cf6e035d6..37f60061ec15b 100644 --- a/src/test/incremental/change_crate_order/main.rs +++ b/src/test/incremental/change_crate_order/main.rs @@ -20,5 +20,5 @@ use b::B; //? #[rustc_clean(label="typeck_tables_of", cfg="rpass2")] pub fn main() { - A + B; + let _ = A + B; } diff --git a/src/test/incremental/warnings-reemitted.rs b/src/test/incremental/warnings-reemitted.rs index a1d11f8aa5bbe..6e237d635d75a 100644 --- a/src/test/incremental/warnings-reemitted.rs +++ b/src/test/incremental/warnings-reemitted.rs @@ -6,5 +6,5 @@ #![warn(const_err)] fn main() { - 255u8 + 1; //~ WARNING this expression will panic at run-time + let _ = 255u8 + 1; //~ WARNING this expression will panic at run-time } From 80016b2e46764d0c7771a6e75c550941eedc1544 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 29 Jul 2019 23:04:44 +0100 Subject: [PATCH 09/17] Fix pretty tests --- src/test/pretty/block-disambig.rs | 2 +- src/test/pretty/unary-op-disambig.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/pretty/block-disambig.rs b/src/test/pretty/block-disambig.rs index ac9b84a5d7e37..015fa3b41f04d 100644 --- a/src/test/pretty/block-disambig.rs +++ b/src/test/pretty/block-disambig.rs @@ -7,7 +7,7 @@ use std::cell::Cell; -fn test1() { let val = &0; { } *val; } +fn test1() { let val = &0; { } let _ = *val; } fn test2() -> isize { let val = &0; { } *val } diff --git a/src/test/pretty/unary-op-disambig.rs b/src/test/pretty/unary-op-disambig.rs index 0c57e0a337175..f3f16ee5530b1 100644 --- a/src/test/pretty/unary-op-disambig.rs +++ b/src/test/pretty/unary-op-disambig.rs @@ -16,4 +16,4 @@ fn alt_semi() -> isize { match true { true => { f() } _ => { } }; -1 } fn alt_no_semi() -> isize { (match true { true => { 0 } _ => { 1 } }) - 1 } -fn stmt() { { f() }; -1; } +fn stmt() { { f() }; let _ = -1; } From 82d6c2ba3158e47daecc0c1b16c07a9b2423b718 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 30 Jul 2019 00:44:58 +0100 Subject: [PATCH 10/17] Fix liballoc tests --- src/liballoc/tests/vec.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/liballoc/tests/vec.rs b/src/liballoc/tests/vec.rs index 98d013dfa2b57..f6c0748ac1c4d 100644 --- a/src/liballoc/tests/vec.rs +++ b/src/liballoc/tests/vec.rs @@ -583,7 +583,7 @@ fn test_drain_inclusive_out_of_bounds() { fn test_splice() { let mut v = vec![1, 2, 3, 4, 5]; let a = [10, 11, 12]; - v.splice(2..4, a.iter().cloned()); + let _ = v.splice(2..4, a.iter().cloned()); assert_eq!(v, &[1, 2, 10, 11, 12, 5]); v.splice(1..3, Some(20)); assert_eq!(v, &[1, 20, 11, 12, 5]); @@ -606,7 +606,7 @@ fn test_splice_inclusive_range() { fn test_splice_out_of_bounds() { let mut v = vec![1, 2, 3, 4, 5]; let a = [10, 11, 12]; - v.splice(5..6, a.iter().cloned()); + let _ = v.splice(5..6, a.iter().cloned()); } #[test] @@ -614,7 +614,7 @@ fn test_splice_out_of_bounds() { fn test_splice_inclusive_out_of_bounds() { let mut v = vec![1, 2, 3, 4, 5]; let a = [10, 11, 12]; - v.splice(5..=5, a.iter().cloned()); + let _ = v.splice(5..=5, a.iter().cloned()); } #[test] From 3d685ab8cdb1cee031de9c4aa26705641daeaddf Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 30 Jul 2019 10:40:25 +0100 Subject: [PATCH 11/17] Fix save-analysis tests --- src/test/run-make-fulldeps/save-analysis-fail/foo.rs | 4 +++- src/test/run-make-fulldeps/save-analysis-fail/krate2.rs | 6 +++--- src/test/run-make-fulldeps/save-analysis/foo.rs | 4 +++- src/test/run-make-fulldeps/save-analysis/krate2.rs | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/test/run-make-fulldeps/save-analysis-fail/foo.rs b/src/test/run-make-fulldeps/save-analysis-fail/foo.rs index e042210ac79b0..b62b1f99469bd 100644 --- a/src/test/run-make-fulldeps/save-analysis-fail/foo.rs +++ b/src/test/run-make-fulldeps/save-analysis-fail/foo.rs @@ -1,7 +1,9 @@ -#![ crate_name = "test" ] +#![crate_name = "test"] #![feature(box_syntax)] #![feature(rustc_private)] +#![allow(unused_must_use)] + extern crate graphviz; // A simple rust project diff --git a/src/test/run-make-fulldeps/save-analysis-fail/krate2.rs b/src/test/run-make-fulldeps/save-analysis-fail/krate2.rs index 7d787e0c9871f..fb81fd301a38d 100644 --- a/src/test/run-make-fulldeps/save-analysis-fail/krate2.rs +++ b/src/test/run-make-fulldeps/save-analysis-fail/krate2.rs @@ -1,8 +1,8 @@ -#![ crate_name = "krate2" ] -#![ crate_type = "lib" ] +#![crate_name = "krate2"] +#![crate_type = "lib"] use std::io::Write; pub fn hello() { - std::io::stdout().write_all(b"hello world!\n"); + let _ = std::io::stdout().write_all(b"hello world!\n"); } diff --git a/src/test/run-make-fulldeps/save-analysis/foo.rs b/src/test/run-make-fulldeps/save-analysis/foo.rs index bc0209dc5832a..fb3647fe2f75b 100644 --- a/src/test/run-make-fulldeps/save-analysis/foo.rs +++ b/src/test/run-make-fulldeps/save-analysis/foo.rs @@ -1,9 +1,11 @@ -#![ crate_name = "test" ] +#![crate_name = "test"] #![feature(box_syntax)] #![feature(rustc_private)] #![feature(associated_type_defaults)] #![feature(external_doc)] +#![allow(unused_must_use)] + extern crate graphviz; // A simple rust project diff --git a/src/test/run-make-fulldeps/save-analysis/krate2.rs b/src/test/run-make-fulldeps/save-analysis/krate2.rs index 7d787e0c9871f..3933b20806dfa 100644 --- a/src/test/run-make-fulldeps/save-analysis/krate2.rs +++ b/src/test/run-make-fulldeps/save-analysis/krate2.rs @@ -4,5 +4,5 @@ use std::io::Write; pub fn hello() { - std::io::stdout().write_all(b"hello world!\n"); + let _ = std::io::stdout().write_all(b"hello world!\n"); } From 0ec7a6218691c3049a9eb36a988c3fcca3c4f773 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 7 Sep 2019 13:32:55 +0100 Subject: [PATCH 12/17] Address review comments --- src/test/pretty/block-disambig.rs | 3 ++- src/test/pretty/unary-op-disambig.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/pretty/block-disambig.rs b/src/test/pretty/block-disambig.rs index 015fa3b41f04d..9dd3d4fa5a484 100644 --- a/src/test/pretty/block-disambig.rs +++ b/src/test/pretty/block-disambig.rs @@ -7,7 +7,8 @@ use std::cell::Cell; -fn test1() { let val = &0; { } let _ = *val; } +#[allow(unused_must_use)] +fn test1() { let val = &0; { } *val; } fn test2() -> isize { let val = &0; { } *val } diff --git a/src/test/pretty/unary-op-disambig.rs b/src/test/pretty/unary-op-disambig.rs index f3f16ee5530b1..bfcae7028f932 100644 --- a/src/test/pretty/unary-op-disambig.rs +++ b/src/test/pretty/unary-op-disambig.rs @@ -16,4 +16,5 @@ fn alt_semi() -> isize { match true { true => { f() } _ => { } }; -1 } fn alt_no_semi() -> isize { (match true { true => { 0 } _ => { 1 } }) - 1 } -fn stmt() { { f() }; let _ = -1; } +#[allow(unused_must_use)] +fn stmt() { { f() }; -1; } From 49a552b5a15c230e7c426bc480c7fbf115d2f0b6 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 8 Oct 2019 23:23:39 +0100 Subject: [PATCH 13/17] Fix some rebase errors --- src/librustc_lint/unused.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index eded76ff788fb..67325adbe564a 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -163,7 +163,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { // Unions are also ignored (though in theory, we could lint if every field of // a union was `#[must_use]`). if def.variants.len() == 1 && !def.is_union() { - let fields = match &expr.node { + let fields = match &expr.kind { hir::ExprKind::Struct(_, fields, _) => { fields.iter().map(|f| &*f.expr).collect() } @@ -182,7 +182,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { (expr, span) }; has_emitted |= check_must_use_ty( - cx, ty, expr, span, descr_pre, descr_post, plural); + cx, ty, expr, span, descr_pre, descr_post, plural_len); } } } @@ -251,7 +251,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { descr_pre, plural_suffix, ); - check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, n as usize + 1) + // `2` is just a stand-in for a number greater than 1, for correct plurals + // in diagnostics. + check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, 2) } } _ => false, From 7569fca927e4b45b268cf5a5ae632142c9a374fd Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 8 Oct 2019 23:24:23 +0100 Subject: [PATCH 14/17] Make `UNUSED_MUST_USE` warn-by-default --- 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 67325adbe564a..2f21874c1b947 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -25,7 +25,7 @@ use log::debug; declare_lint! { pub UNUSED_MUST_USE, - Deny, + Warn, "unused result of a type flagged as `#[must_use]`", report_in_external_macro: true } From 5c1ee1741fa0347736c1da89e35a50b6157f9a02 Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 9 Oct 2019 00:09:25 +0100 Subject: [PATCH 15/17] Update new tests --- src/test/ui/issues/issue-48132.rs | 2 +- src/test/ui/type-alias-impl-trait/issue-58951.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/issues/issue-48132.rs b/src/test/ui/issues/issue-48132.rs index f564aefe78ced..bdfdfa9e08de9 100644 --- a/src/test/ui/issues/issue-48132.rs +++ b/src/test/ui/issues/issue-48132.rs @@ -27,5 +27,5 @@ where I: Iterator, } fn main() { - outer(std::iter::once(&1).cloned()); + let _ = outer(std::iter::once(&1).cloned()); } diff --git a/src/test/ui/type-alias-impl-trait/issue-58951.rs b/src/test/ui/type-alias-impl-trait/issue-58951.rs index 3416c6745bb0d..b1e30f1d7b2e6 100644 --- a/src/test/ui/type-alias-impl-trait/issue-58951.rs +++ b/src/test/ui/type-alias-impl-trait/issue-58951.rs @@ -7,7 +7,7 @@ type A = impl Iterator; fn def_a() -> A { 0..1 } pub fn use_a() { - def_a().map(|x| x); + let _ = def_a().map(|x| x); } fn main() {} From 57a04498d1c95de29e74029774999b098bea1c0f Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 9 Oct 2019 00:09:47 +0100 Subject: [PATCH 16/17] Make recursive lint into an error for crater --- src/librustc_lint/unused.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 2f21874c1b947..ff645811623a9 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -50,7 +50,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { } let ty = cx.tables.expr_ty(&expr); - let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "", "", 1); + let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "", "", 1, false); let mut fn_warned = false; let mut op_warned = false; @@ -75,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { _ => None }; if let Some(def_id) = maybe_def_id { - fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", ""); + fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "", false); } 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.) @@ -137,7 +137,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { span: Span, descr_pre: &str, descr_post: &str, - plural_len: usize, + len: usize, + err: bool, // HACK: Report an error rather than a lint, for crater testing. ) -> bool { if ty.is_unit() || cx.tcx.is_ty_uninhabited_from( cx.tcx.hir().get_module_parent(expr.hir_id), ty) @@ -145,18 +146,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { return true; } - let plural_suffix = pluralise!(plural_len); + let plural_suffix = pluralise!(len); match ty.kind { ty::Adt(..) if ty.is_box() => { let boxed_ty = ty.boxed_ty(); let descr_pre = &format!("{}boxed ", descr_pre); - check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, plural_len) + check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, len, err) } ty::Adt(def, subst) => { // Check the type itself for `#[must_use]` annotations. let mut has_emitted = check_must_use_def( - cx, def.did, span, descr_pre, descr_post); + cx, def.did, span, descr_pre, descr_post, err); // Check any fields of the type for `#[must_use]` annotations. // We ignore ADTs with more than one variant for simplicity and to avoid // false positives. @@ -182,7 +183,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { (expr, span) }; has_emitted |= check_must_use_ty( - cx, ty, expr, span, descr_pre, descr_post, plural_len); + cx, ty, expr, span, descr_pre, descr_post, len, true); } } } @@ -199,7 +200,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { descr_pre, plural_suffix, ); - if check_must_use_def(cx, def_id, span, descr_pre, descr_post) { + if check_must_use_def(cx, def_id, span, descr_pre, descr_post, err) { has_emitted = true; break; } @@ -217,7 +218,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { plural_suffix, descr_post, ); - if check_must_use_def(cx, def_id, span, descr_pre, descr_post) { + if check_must_use_def(cx, def_id, span, descr_pre, descr_post, err) { has_emitted = true; break; } @@ -237,7 +238,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { let descr_post = &format!(" in tuple element {}", i); let span = *spans.get(i).unwrap_or(&span); has_emitted |= check_must_use_ty( - cx, ty, expr, span, descr_pre, descr_post, plural_len); + cx, ty, expr, span, descr_pre, descr_post, len, err); } has_emitted } @@ -253,7 +254,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { ); // `2` is just a stand-in for a number greater than 1, for correct plurals // in diagnostics. - check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, 2) + check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, 2, err) } } _ => false, @@ -267,12 +268,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { span: Span, descr_pre_path: &str, descr_post_path: &str, + force_err: bool, // HACK: Report an error rather than a lint, for crater testing. ) -> bool { for attr in cx.tcx.get_attrs(def_id).iter() { 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, span, &msg); + let mut err = if !force_err { + cx.struct_span_lint(UNUSED_MUST_USE, span, &msg) + } else { + cx.sess().struct_span_err(span, &msg) + }; // check for #[must_use = "..."] if let Some(note) = attr.value_str() { err.note(¬e.as_str()); From 8bcf64ad80e32f3d337e14bcb98a7188c811fe1d Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 9 Oct 2019 00:32:02 +0100 Subject: [PATCH 17/17] Ignore private `must_use` fields --- src/librustc_lint/unused.rs | 30 +++++++++++-------- .../private-nested-unused_must_use.rs | 26 ++++++++++++++++ .../ui/lint/mod-nested-unused_must_use.rs | 12 ++++++++ .../ui/lint/mod-nested-unused_must_use.stderr | 8 +++++ 4 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/lint/auxiliary/private-nested-unused_must_use.rs create mode 100644 src/test/ui/lint/mod-nested-unused_must_use.rs create mode 100644 src/test/ui/lint/mod-nested-unused_must_use.stderr diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index ff645811623a9..8dd23b0041be0 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -140,9 +140,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { len: usize, err: bool, // HACK: Report an error rather than a lint, for crater testing. ) -> bool { - if ty.is_unit() || cx.tcx.is_ty_uninhabited_from( - cx.tcx.hir().get_module_parent(expr.hir_id), ty) - { + let module = cx.tcx.hir().get_module_parent(expr.hir_id); + + if ty.is_unit() || cx.tcx.is_ty_uninhabited_from(module, ty) { return true; } @@ -174,16 +174,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { for variant in &def.variants { for (i, field) in variant.fields.iter().enumerate() { - let descr_post - = &format!(" in field `{}`", field.ident.as_str()); - let ty = cx.tcx.type_of(field.did).subst(cx.tcx, subst); - let (expr, span) = if let Some(&field) = fields.get(i) { - (field, field.span) - } else { - (expr, span) - }; - has_emitted |= check_must_use_ty( - cx, ty, expr, span, descr_pre, descr_post, len, true); + let is_visible = def.is_enum() || + field.vis.is_accessible_from(module, cx.tcx); + if is_visible { + let descr_post + = &format!(" in field `{}`", field.ident.as_str()); + let ty = cx.tcx.type_of(field.did).subst(cx.tcx, subst); + let (expr, span) = if let Some(&field) = fields.get(i) { + (field, field.span) + } else { + (expr, span) + }; + has_emitted |= check_must_use_ty( + cx, ty, expr, span, descr_pre, descr_post, len, true); + } } } } diff --git a/src/test/ui/lint/auxiliary/private-nested-unused_must_use.rs b/src/test/ui/lint/auxiliary/private-nested-unused_must_use.rs new file mode 100644 index 0000000000000..0d3c57fc2db1f --- /dev/null +++ b/src/test/ui/lint/auxiliary/private-nested-unused_must_use.rs @@ -0,0 +1,26 @@ +#[must_use] +pub struct S; + +pub struct T; + +pub struct B { + s: S, + pub t: T, +} + +pub struct C { + pub s: S, + pub t: T, +} + +impl B { + pub fn new() -> B { + B { s: S, t: T } + } +} + +impl C { + pub fn new() -> C { + C { s: S, t: T } + } +} diff --git a/src/test/ui/lint/mod-nested-unused_must_use.rs b/src/test/ui/lint/mod-nested-unused_must_use.rs new file mode 100644 index 0000000000000..19eb79b85910e --- /dev/null +++ b/src/test/ui/lint/mod-nested-unused_must_use.rs @@ -0,0 +1,12 @@ +// aux-build:private-nested-unused_must_use.rs + +#![deny(unused_must_use)] + +extern crate private_nested_unused_must_use; + +use self::private_nested_unused_must_use::{B, C}; + +fn main() { + B::new(); // ok: ignores private `must_use` type + C::new(); //~ ERROR unused `private_nested_unused_must_use::S` in field `s` that must be used +} diff --git a/src/test/ui/lint/mod-nested-unused_must_use.stderr b/src/test/ui/lint/mod-nested-unused_must_use.stderr new file mode 100644 index 0000000000000..903434b23c3b2 --- /dev/null +++ b/src/test/ui/lint/mod-nested-unused_must_use.stderr @@ -0,0 +1,8 @@ +error: unused `private_nested_unused_must_use::S` in field `s` that must be used + --> $DIR/mod-nested-unused_must_use.rs:11:5 + | +LL | C::new(); + | ^^^^^^^^^ + +error: aborting due to previous error +