Auto merge of #43728 - zackmdavis:fnused, r=eddyb

bors · bors · commit 3f977baf34ed · 2017-08-09T04:03:49.000Z
#[must_use] for functions This implements [RFC 1940](rust-lang/rfcs#1940). The RFC and discussion thereof seem to suggest that tagging `PartialEq::eq` and friends as `#[must_use]` would automatically lint for unused comparisons, but it doesn't work out that way (at least the way I've implemented it): unused `.eq` method calls get linted, but not `==` expressions. (The lint operates on the HIR, which sees binary operations as their own thing, even if they ultimately just call `.eq` _&c._.) What do _you_ think?? Resolves #43302.
diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs
@@ -110,11 +110,13 @@ use self::Ordering::*;
 pub trait PartialEq<Rhs: ?Sized = Self> {
     /// This method tests for `self` and `other` values to be equal, and is used
     /// by `==`.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn eq(&self, other: &Rhs) -> bool;
 
     /// This method tests for `!=`.
     #[inline]
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn ne(&self, other: &Rhs) -> bool { !self.eq(other) }
 }
@@ -625,6 +627,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     /// let result = std::f64::NAN.partial_cmp(&1.0);
     /// assert_eq!(result, None);
     /// ```
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn partial_cmp(&self, other: &Rhs) -> Option<Ordering>;
 
@@ -640,6 +643,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     /// assert_eq!(result, false);
     /// ```
     #[inline]
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn lt(&self, other: &Rhs) -> bool {
         match self.partial_cmp(other) {
@@ -661,6 +665,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     /// assert_eq!(result, true);
     /// ```
     #[inline]
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn le(&self, other: &Rhs) -> bool {
         match self.partial_cmp(other) {
@@ -681,6 +686,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     /// assert_eq!(result, false);
     /// ```
     #[inline]
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn gt(&self, other: &Rhs) -> bool {
         match self.partial_cmp(other) {
@@ -702,6 +708,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     /// assert_eq!(result, true);
     /// ```
     #[inline]
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn ge(&self, other: &Rhs) -> bool {
         match self.partial_cmp(other) {
diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs
@@ -145,22 +145,38 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
         }
 
         let t = cx.tables.expr_ty(&expr);
-        let warned = match t.sty {
-            ty::TyTuple(ref tys, _) if tys.is_empty() => return,
-            ty::TyNever => return,
-            ty::TyBool => return,
-            ty::TyAdt(def, _) => check_must_use(cx, def.did, s.span),
+        let ty_warned = match t.sty {
+            ty::TyAdt(def, _) => check_must_use(cx, def.did, s.span, ""),
             _ => false,
         };
-        if !warned {
+
+        let mut fn_warned = false;
+        let maybe_def = match expr.node {
+            hir::ExprCall(ref callee, _) => {
+                match callee.node {
+                    hir::ExprPath(ref qpath) => Some(cx.tables.qpath_def(qpath, callee.id)),
+                    _ => None
+                }
+            },
+            hir::ExprMethodCall(..) => {
+                cx.tables.type_dependent_defs.get(&expr.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 ");
+        }
+
+        if !(ty_warned || fn_warned) {
             cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
         }
 
-        fn check_must_use(cx: &LateContext, def_id: DefId, sp: Span) -> bool {
+        fn check_must_use(cx: &LateContext, def_id: DefId, sp: Span, describe_path: &str) -> bool {
             for attr in cx.tcx.get_attrs(def_id).iter() {
                 if attr.check_name("must_use") {
-                    let mut msg = format!("unused `{}` which must be used",
-                                          cx.tcx.item_path_str(def_id));
+                    let mut msg = format!("unused {}`{}` which must be used",
+                                          describe_path, cx.tcx.item_path_str(def_id));
                     // check for #[must_use="..."]
                     if let Some(s) = attr.value_str() {
                         msg.push_str(": ");
diff --git a/src/test/ui/lint/fn_must_use.rs b/src/test/ui/lint/fn_must_use.rs
@@ -0,0 +1,33 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+
+struct MyStruct {
+    n: usize
+}
+
+impl MyStruct {
+    #[must_use]
+    fn need_to_use_this_method_value(&self) -> usize {
+        self.n
+    }
+}
+
+#[must_use="it's important"]
+fn need_to_use_this_value() -> bool {
+    false
+}
+
+fn main() {
+    need_to_use_this_value();
+
+    let m = MyStruct { n: 2 };
+    m.need_to_use_this_method_value();
+}
diff --git a/src/test/ui/lint/fn_must_use.stderr b/src/test/ui/lint/fn_must_use.stderr
@@ -0,0 +1,14 @@
+warning: unused return value of `need_to_use_this_value` which must be used: it's important
+  --> $DIR/fn_must_use.rs:29:5
+   |
+29 |     need_to_use_this_value();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(unused_must_use)] on by default
+
+warning: unused return value of `MyStruct::need_to_use_this_method_value` which must be used
+  --> $DIR/fn_must_use.rs:32:5
+   |
+32 |     m.need_to_use_this_method_value();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
