Auto merge of #62262 - varkor:must_use-adt-components-ii, r=<try>

bors · bors · commit 5ac1de79c89d · 2019-09-09T20:12:14.000Z
Extend `#[must_use]` to nested structures Extends the `#[must_use]` lint to apply when `#[must_use]` types are nested within `struct`s (or one-variant `enum`s), making the lint much more generally useful. This is in line with #61100 extending the lint to tuples. Fixes #39524. cc @rust-lang/lang and @rust-lang/compiler for discussion in case this is a controversial change. In particular, we might want to consider allowing annotations on fields containing `#[must_use]` types in user-defined types (e.g. `#[allow(unused_must_use)]`) to opt out of this behaviour, if there are cases where we this this is likely to have frequent false positives. (This is based on top of #62235.)
diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs
@@ -1571,7 +1571,7 @@ impl String {
              Unbounded => {},
         };
 
-        unsafe {
+        let _ = unsafe {
             self.as_mut_vec()
         }.splice(range, replace_with.bytes());
     }
diff --git 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,15 +606,15 @@ 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]
 #[should_panic]
 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]
diff --git 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};
@@ -23,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
 }
@@ -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)
                 }
-                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,24 +236,23 @@ 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) {
-                            has_emitted = true;
-                        }
+                        has_emitted |= check_must_use_ty(
+                            cx, ty, expr, span, descr_pre, descr_post, plural);
                     }
                     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,
                             plural_suffix,
                         );
                         check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, true)
                     }
-                    // Otherwise, we don't lint, to avoid false positives.
-                    _ => false,
                 }
                 _ => false,
             }
diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs
@@ -93,7 +93,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
                     .filter(needs_retag)
                     .collect::<Vec<_>>();
             // 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, place),
diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs
@@ -103,9 +103,7 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + '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));
         }
     }
 }
diff --git 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
@@ -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
 }
diff --git 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/pretty/block-disambig.rs b/src/test/pretty/block-disambig.rs
@@ -7,6 +7,7 @@
 
 use std::cell::Cell;
 
+#[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
@@ -16,4 +16,5 @@ fn alt_semi() -> isize { match true { true => { f() } _ => { } }; -1 }
 
 fn alt_no_semi() -> isize { (match true { true => { 0 } _ => { 1 } }) - 1 }
 
+#[allow(unused_must_use)]
 fn stmt() { { f() }; -1; }
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -3,6 +3,6 @@
 use std::panic;
 
 fn main() {
-    panic::take_hook();
+    let _ = panic::take_hook();
     panic!("foobar");
 }
diff --git 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
@@ -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
@@ -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
@@ -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");
 }
diff --git 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<T:'static + Send + Clone>(x: T) -> Receiver<T> {
     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
@@ -1,3 +1,3 @@
 pub unsafe fn f(xs: Vec<isize> ) {
-    xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::<Vec<()>>();
+    let _ = xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::<Vec<()>>();
 }
diff --git 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;
     }
 }
diff --git a/src/test/ui/lint/must_use-adt.rs 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<dyn A>); 2],
+    x: u32,
+    y: (B, B),
+    z: (S, S),
+    e: [(u8, Box<dyn A>); 2],
+    f: S,
+}
+
+fn get_v() -> V {
+    V { a: S }
+}
+
+struct Z([(u8, Box<dyn A>); 2]);
+
+fn get_wrapped_arr() -> Z {
+    Z([(0, Box::new(B)), (0, Box::new(B))])
+}
+
+fn get_tuple_arr() -> ([(u8, Box<dyn A>); 2],) {
+    ([(0, Box::new(B)), (0, Box::new(B))],)
+}
+
+struct R<T> {
+    r: T
+}
+
+struct List<T>(T, Option<Box<Self>>);
+
+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
diff --git a/src/tools/cargo b/src/tools/cargo

Original file line number	Diff line number	Diff line change
`@@ -1571,7 +1571,7 @@ impl String {`
`1571`	`1571`	`Unbounded => {},`
`1572`	`1572`	`};`
`1573`	`1573`
`1574`		`- unsafe {`
	`1574`	`+ let _ = unsafe {`
`1575`	`1575`	`self.as_mut_vec()`
`1576`	`1576`	`}.splice(range, replace_with.bytes());`
`1577`	`1577`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,9 +103,7 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) {`
`103`	`103`	`HOOK_LOCK.write_unlock();`
`104`	`104`
`105`	`105`	`if let Hook::Custom(ptr) = old_hook {`
`106`		`- #[allow(unused_must_use)] {`
`107`		`- Box::from_raw(ptr);`
`108`		`- }`
	`106`	`+ mem::drop(Box::from_raw(ptr));`
`109`	`107`	`}`
`110`	`108`	`}`
`111`	`109`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,5 +20,5 @@ use b::B;`
`20`	`20`
`21`	`21`	`//? #[rustc_clean(label="typeck_tables_of", cfg="rpass2")]`
`22`	`22`	`pub fn main() {`
`23`		`- A + B;`
	`23`	`+ let _ = A + B;`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,5 +6,5 @@`
`6`	`6`	`#![warn(const_err)]`
`7`	`7`
`8`	`8`	`fn main() {`
`9`		`- 255u8 + 1; //~ WARNING this expression will panic at run-time`
	`9`	`+ let _ = 255u8 + 1; //~ WARNING this expression will panic at run-time`
`10`	`10`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+#![allow(unused_must_use)]`
	`2`	`+`
`1`	`3`	`fn main() {`
`2`	`4`	`*(&4);`
`3`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,4 +16,5 @@ fn alt_semi() -> isize { match true { true => { f() } _ => { } }; -1 }`
`16`	`16`
`17`	`17`	`fn alt_no_semi() -> isize { (match true { true => { 0 } _ => { 1 } }) - 1 }`
`18`	`18`
	`19`	`+#[allow(unused_must_use)]`
`19`	`20`	`fn stmt() { { f() }; -1; }`