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

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.)

Author: bors

src/liballoc/string.rs

@@ -1568,7 +1568,7 @@ impl String {
              Unbounded => {},
         };
 
-        unsafe {
+        let _ = unsafe {
             self.as_mut_vec()
         }.splice(range, replace_with.bytes());
     }

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]

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};
@@ -48,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;
@@ -73,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.)
@@ -135,24 +137,61 @@ 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)
-            {
+            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;
             }
 
-            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, _) => {
-                    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, 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.
+                    // 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.kind {
+                            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 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);
+                                }
+                            }
+                        }
+                    }
+                    has_emitted
                 }
                 ty::Opaque(def, _) => {
                     let mut has_emitted = false;
@@ -165,7 +204,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;
                             }
@@ -183,7 +222,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;
                             }
@@ -202,32 +241,25 @@ 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, len, err);
                     }
                     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, 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, err)
                     }
-                    // Otherwise, we don't lint, to avoid false positives.
-                    _ => false,
                 }
                 _ => false,
             }
@@ -240,12 +272,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(&note.as_str());

src/librustc_mir/transform/add_retag.rs

@@ -86,7 +86,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, box(place)),

src/libstd/panicking.rs

@@ -104,9 +104,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));
         }
     }
 }

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;
 }

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
 }

src/test/mir-opt/const_prop/ref_deref.rs

@@ -1,3 +1,5 @@
+#![allow(unused_must_use)]
+
 fn main() {
     *(&4);
 }

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 }

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; }