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

@@ -1571,7 +1571,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/ty/mod.rs

@@ -1948,7 +1948,7 @@ pub struct FieldDef {
 pub struct AdtDef {
     /// `DefId` of the struct, enum or union item.
     pub did: DefId,
-    /// Variants of the ADT. If this is a struct or enum, then there will be a single variant.
+    /// Variants of the ADT. If this is a struct or union, then there will be a single variant.
     pub variants: IndexVec<self::layout::VariantIdx, VariantDef>,
     /// Flags of the ADT (e.g. is this a struct? is this non-exhaustive?)
     flags: AdtFlags,

src/librustc_lint/unused.rs

@@ -1,8 +1,11 @@
 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::mir::interpret::{GlobalId, ConstValue};
 use rustc_data_structures::fx::FxHashMap;
 use lint::{LateContext, EarlyContext, LintContext, LintArray};
 use lint::{LintPass, EarlyLintPass, LateLintPass};
@@ -23,7 +26,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 +154,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 +237,43 @@ 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.assert_usize(cx.tcx) {
-                    // If the array is definitely non-empty, we can do `#[must_use]` checking.
-                    Some(n) if n != 0 => {
-                        let descr_pre = &format!(
-                            "{}array{} of ",
-                            descr_pre,
-                            plural_suffix,
-                        );
-                        check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, true)
+                ty::Array(ty, mut len) => {
+                    // Try to evaluate the length if it's unevaluated.
+                    // FIXME(59369): we should be able to remove this once we merge
+                    // https://github.com/rust-lang/rust/pull/59369.
+                    if let ConstValue::Unevaluated(def_id, substs) = len.val {
+                        let instance = ty::Instance::resolve(
+                            cx.tcx.global_tcx(),
+                            cx.param_env,
+                            def_id,
+                            substs,
+                        ).unwrap();
+                        let global_id = GlobalId {
+                            instance,
+                            promoted: None
+                        };
+                        if let Ok(ct) = cx.tcx.const_eval(cx.param_env.and(global_id)) {
+                            len = ct;
+                        }
+                    }
+
+                    match len.assert_usize(cx.tcx) {
+                        Some(0) => false, // Empty arrays won't contain any `#[must_use]` types.
+                        // 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,
             }

src/librustc_mir/transform/add_retag.rs

@@ -97,7 +97,7 @@ impl MirPass 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),

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

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