Auto merge of #48925 - zackmdavis:fn_must_stabilize, r=nikomatsakis

stabilize `#[must_use]` for functions and must-use comparison operators (RFC 1940)

r? @nikomatsakis

Author: bors

src/doc/unstable-book/src/language-features/fn-must-use.md

@@ -96,7 +96,7 @@
 #![feature(dropck_eyepatch)]
 #![feature(exact_size_is_empty)]
 #![feature(fmt_internals)]
-#![feature(fn_must_use)]
+#![cfg_attr(stage0, feature(fn_must_use))]
 #![feature(from_ref)]
 #![feature(fundamental)]
 #![feature(lang_items)]

src/liballoc/lib.rs

@@ -1282,6 +1282,7 @@ fn test_box_slice_clone() {
 }
 
 #[test]
+#[allow(unused_must_use)] // here, we care about the side effects of `.clone()`
 #[cfg_attr(target_os = "emscripten", ignore)]
 fn test_box_slice_clone_panics() {
     use std::sync::Arc;

src/liballoc/tests/slice.rs

@@ -76,7 +76,6 @@
 #![feature(doc_cfg)]
 #![feature(doc_spotlight)]
 #![feature(extern_types)]
-#![feature(fn_must_use)]
 #![feature(fundamental)]
 #![feature(intrinsics)]
 #![feature(iterator_flatten)]
@@ -114,6 +113,7 @@
 
 #![cfg_attr(stage0, feature(target_feature))]
 #![cfg_attr(stage0, feature(cfg_target_feature))]
+#![cfg_attr(stage0, feature(fn_must_use))]
 
 #[prelude_import]
 #[allow(unused)]

src/libcore/lib.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use rustc::hir::def::Def;
 use rustc::hir::def_id::DefId;
 use rustc::ty;
 use rustc::ty::adjustment;
@@ -72,54 +73,59 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
 
         let mut fn_warned = false;
         let mut op_warned = false;
-        if cx.tcx.features().fn_must_use {
-            let maybe_def = match expr.node {
-                hir::ExprCall(ref callee, _) => {
-                    match callee.node {
-                        hir::ExprPath(ref qpath) => {
-                            Some(cx.tables.qpath_def(qpath, callee.hir_id))
-                        },
-                        _ => None
-                    }
-                },
-                hir::ExprMethodCall(..) => {
-                    cx.tables.type_dependent_defs().get(expr.hir_id).cloned()
-                },
-                _ => None
-            };
-            if let Some(def) = maybe_def {
-                let def_id = def.def_id();
-                fn_warned = check_must_use(cx, def_id, s.span, "return value of ");
-            }
-            let must_use_op = match expr.node {
-                // Hardcoding operators here seemed more expedient than the
-                // refactoring that would be needed to look up the `#[must_use]`
-                // attribute which does exist on the comparison trait methods
-                hir::ExprBinary(bin_op, ..)  => {
-                    match bin_op.node {
-                        hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
-                            Some("comparison")
-                        },
-                        hir::BiAdd | hir::BiSub | hir::BiDiv | hir::BiMul | hir::BiRem => {
-                            Some("arithmetic operation")
-                        },
-                        hir::BiAnd | hir::BiOr => {
-                            Some("logical operation")
-                        },
-                        hir::BiBitXor | hir::BiBitAnd | hir::BiBitOr | hir::BiShl | hir::BiShr => {
-                            Some("bitwise operation")
-                        },
-                    }
-                },
-                hir::ExprUnary(..) => Some("unary operation"),
-                _ => None
-            };
-            if let Some(must_use_op) = must_use_op {
-                cx.span_lint(UNUSED_MUST_USE, expr.span,
-                    &format!("unused {} which must be used", must_use_op));
-                op_warned = true;
-            }
+        let maybe_def = match expr.node {
+            hir::ExprCall(ref callee, _) => {
+                match callee.node {
+                    hir::ExprPath(ref qpath) => {
+                        let def = cx.tables.qpath_def(qpath, callee.hir_id);
+                        if let Def::Fn(_) = def {
+                            Some(def)
+                        } else {  // `Def::Local` if it was a closure, for which we
+                            None  // do not currently support must-use linting
+                        }
+                    },
+                    _ => None
+                }
+            },
+            hir::ExprMethodCall(..) => {
+                cx.tables.type_dependent_defs().get(expr.hir_id).cloned()
+            },
+            _ => None
+        };
+        if let Some(def) = maybe_def {
+            let def_id = def.def_id();
+            fn_warned = check_must_use(cx, def_id, s.span, "return value of ");
         }
+        let must_use_op = match expr.node {
+            // Hardcoding operators here seemed more expedient than the
+            // refactoring that would be needed to look up the `#[must_use]`
+            // attribute which does exist on the comparison trait methods
+            hir::ExprBinary(bin_op, ..)  => {
+                match bin_op.node {
+                    hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
+                        Some("comparison")
+                    },
+                    hir::BiAdd | hir::BiSub | hir::BiDiv | hir::BiMul | hir::BiRem => {
+                        Some("arithmetic operation")
+                    },
+                    hir::BiAnd | hir::BiOr => {
+                        Some("logical operation")
+                    },
+                    hir::BiBitXor | hir::BiBitAnd | hir::BiBitOr | hir::BiShl | hir::BiShr => {
+                        Some("bitwise operation")
+                    },
+                }
+            },
+            hir::ExprUnary(..) => Some("unary operation"),
+            _ => None
+        };
+
+        if let Some(must_use_op) = must_use_op {
+            cx.span_lint(UNUSED_MUST_USE, expr.span,
+                         &format!("unused {} which must be used", must_use_op));
+            op_warned = true;
+        }
+
         if !(ty_warned || fn_warned || op_warned) {
             cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
         }

src/librustc_lint/unused.rs

@@ -988,6 +988,7 @@ impl CStr {
     /// behavior when `ptr` is used inside the `unsafe` block:
     ///
     /// ```no_run
+    /// # #![allow(unused_must_use)]
     /// use std::ffi::{CString};
     ///
     /// let ptr = CString::new("Hello").unwrap().as_ptr();
@@ -1003,6 +1004,7 @@ impl CStr {
     /// To fix the problem, bind the `CString` to a local variable:
     ///
     /// ```no_run
+    /// # #![allow(unused_must_use)]
     /// use std::ffi::{CString};
     ///
     /// let hello = CString::new("Hello").unwrap();

src/libstd/ffi/c_str.rs

@@ -518,6 +518,7 @@ mod tests {
         }
     }
 
+    #[allow(unused_must_use)]
     #[test]
     fn cloning() {
         let (tx1, rx1) = channel::<i32>();
@@ -540,6 +541,7 @@ mod tests {
         tx3.send(()).unwrap();
     }
 
+    #[allow(unused_must_use)]
     #[test]
     fn cloning2() {
         let (tx1, rx1) = channel::<i32>();

src/libstd/sync/mpsc/select.rs

@@ -369,9 +369,6 @@ declare_features! (
     // #[doc(include="some-file")]
     (active, external_doc, "1.22.0", Some(44732), None),
 
-    // allow `#[must_use]` on functions and comparison operators (RFC 1940)
-    (active, fn_must_use, "1.21.0", Some(43302), None),
-
     // Future-proofing enums/structs with #[non_exhaustive] attribute (RFC 2008)
     (active, non_exhaustive, "1.22.0", Some(44109), None),
 
@@ -591,6 +588,8 @@ declare_features! (
     (accepted, target_feature, "1.27.0", None, None),
     // Trait object syntax with `dyn` prefix
     (accepted, dyn_trait, "1.27.0", Some(44662), None),
+    // allow `#[must_use]` on functions; and, must-use operators (RFC 1940)
+    (accepted, fn_must_use, "1.27.0", Some(43302), None),
 );
 
 // If you change this, please modify src/doc/unstable-book as well. You must
@@ -1545,11 +1544,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                                         function may change over time, for now \
                                         a top-level `fn main()` is required");
                 }
-                if let Some(attr) = attr::find_by_name(&i.attrs[..], "must_use") {
-                    gate_feature_post!(&self, fn_must_use, attr.span,
-                                       "`#[must_use]` on functions is experimental",
-                                       GateStrength::Soft);
-                }
             }
 
             ast::ItemKind::Struct(..) => {
@@ -1581,7 +1575,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                                    "trait aliases are not yet fully implemented");
             }
 
-            ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, ref impl_items) => {
+            ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, _) => {
                 if polarity == ast::ImplPolarity::Negative {
                     gate_feature_post!(&self, optin_builtin_traits,
                                        i.span,
@@ -1594,16 +1588,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                                        i.span,
                                        "specialization is unstable");
                 }
-
-                for impl_item in impl_items {
-                    if let ast::ImplItemKind::Method(..) = impl_item.node {
-                        if let Some(attr) = attr::find_by_name(&impl_item.attrs[..], "must_use") {
-                            gate_feature_post!(&self, fn_must_use, attr.span,
-                                               "`#[must_use]` on methods is experimental",
-                                               GateStrength::Soft);
-                        }
-                    }
-                }
             }
 
             ast::ItemKind::Trait(ast::IsAuto::Yes, ..) => {