From ab91a6b4df04a60cf19682b1f016848d3e4784ad Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Thu, 11 Oct 2018 23:10:37 -0700 Subject: [PATCH] `#[must_use]` for associated functions is supposed to actually work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the comments of (closed, defunct) pull request #54884, Mazdak "Centril" Farrokhzad noted that must-use annotations didn't work on an associated function (what other communities might call a "static method"). Subsequent logging revealed that in this case we have a `Def::Method`, whereas the lint pass was only matching on `Def::Fn`. (One could argue that those def-names are thereby misleading—must-use for self-ful methods have always worked—but documenting or reworking that can be left to another day.) --- src/liballoc/rc.rs | 4 ++-- src/liballoc/sync.rs | 4 ++-- src/librustc_lint/unused.rs | 9 +++++---- src/test/run-pass/resolve-pseudo-shadowing.rs | 2 +- src/test/ui/fn_must_use.rs | 8 ++++++++ src/test/ui/fn_must_use.stderr | 20 ++++++++++++------- 6 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 915b8e7787e99..40bb2faa3623b 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -867,7 +867,7 @@ impl Clone for Rc { /// /// let five = Rc::new(5); /// - /// Rc::clone(&five); + /// let _ = Rc::clone(&five); /// ``` #[inline] fn clone(&self) -> Rc { @@ -1304,7 +1304,7 @@ impl Clone for Weak { /// /// let weak_five = Rc::downgrade(&Rc::new(5)); /// - /// Weak::clone(&weak_five); + /// let _ = Weak::clone(&weak_five); /// ``` #[inline] fn clone(&self) -> Weak { diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 9e245fbd7bbe5..35935861fb182 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -713,7 +713,7 @@ impl Clone for Arc { /// /// let five = Arc::new(5); /// - /// Arc::clone(&five); + /// let _ = Arc::clone(&five); /// ``` #[inline] fn clone(&self) -> Arc { @@ -1135,7 +1135,7 @@ impl Clone for Weak { /// /// let weak_five = Arc::downgrade(&Arc::new(5)); /// - /// Weak::clone(&weak_five); + /// let _ = Weak::clone(&weak_five); /// ``` #[inline] fn clone(&self) -> Weak { diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index ec3c310c63c4f..7d178d2096720 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -80,10 +80,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { match callee.node { hir::ExprKind::Path(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 + match def { + Def::Fn(_) | Def::Method(_) => Some(def), + // `Def::Local` if it was a closure, for which we + // do not currently support must-use linting + _ => None } }, _ => None diff --git a/src/test/run-pass/resolve-pseudo-shadowing.rs b/src/test/run-pass/resolve-pseudo-shadowing.rs index 071279ae7d818..bf6e686bb7bb4 100644 --- a/src/test/run-pass/resolve-pseudo-shadowing.rs +++ b/src/test/run-pass/resolve-pseudo-shadowing.rs @@ -12,7 +12,7 @@ fn check(_c: Clone) { fn check2() { - <() as std::clone::Clone>::clone(&()); + let _ = <() as std::clone::Clone>::clone(&()); } check2(); } diff --git a/src/test/ui/fn_must_use.rs b/src/test/ui/fn_must_use.rs index def23046db21d..e3e20bc89b462 100644 --- a/src/test/ui/fn_must_use.rs +++ b/src/test/ui/fn_must_use.rs @@ -22,6 +22,11 @@ impl MyStruct { fn need_to_use_this_method_value(&self) -> usize { self.n } + + #[must_use] + fn need_to_use_this_associated_function_value() -> isize { + -1 + } } trait EvenNature { @@ -66,6 +71,9 @@ fn main() { m.is_even(); // trait method! //~^ WARN unused return value + MyStruct::need_to_use_this_associated_function_value(); + //~^ WARN unused return value + m.replace(3); // won't warn (annotation needs to be in trait definition) // comparison methods are `must_use` diff --git a/src/test/ui/fn_must_use.stderr b/src/test/ui/fn_must_use.stderr index b5bad22f3dc78..1bce8abbbf055 100644 --- a/src/test/ui/fn_must_use.stderr +++ b/src/test/ui/fn_must_use.stderr @@ -1,5 +1,5 @@ warning: unused return value of `need_to_use_this_value` which must be used - --> $DIR/fn_must_use.rs:60:5 + --> $DIR/fn_must_use.rs:65:5 | LL | need_to_use_this_value(); //~ WARN unused return value | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -12,39 +12,45 @@ LL | #![warn(unused_must_use)] = note: it's important warning: unused return value of `MyStruct::need_to_use_this_method_value` which must be used - --> $DIR/fn_must_use.rs:65:5 + --> $DIR/fn_must_use.rs:70:5 | LL | m.need_to_use_this_method_value(); //~ WARN unused return value | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: unused return value of `EvenNature::is_even` which must be used - --> $DIR/fn_must_use.rs:66:5 + --> $DIR/fn_must_use.rs:71:5 | LL | m.is_even(); // trait method! | ^^^^^^^^^^^^ | = note: no side effects +warning: unused return value of `MyStruct::need_to_use_this_associated_function_value` which must be used + --> $DIR/fn_must_use.rs:74:5 + | +LL | MyStruct::need_to_use_this_associated_function_value(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + warning: unused return value of `std::cmp::PartialEq::eq` which must be used - --> $DIR/fn_must_use.rs:72:5 + --> $DIR/fn_must_use.rs:80:5 | LL | 2.eq(&3); //~ WARN unused return value | ^^^^^^^^^ warning: unused return value of `std::cmp::PartialEq::eq` which must be used - --> $DIR/fn_must_use.rs:73:5 + --> $DIR/fn_must_use.rs:81:5 | LL | m.eq(&n); //~ WARN unused return value | ^^^^^^^^^ warning: unused comparison which must be used - --> $DIR/fn_must_use.rs:76:5 + --> $DIR/fn_must_use.rs:84:5 | LL | 2 == 3; //~ WARN unused comparison | ^^^^^^ warning: unused comparison which must be used - --> $DIR/fn_must_use.rs:77:5 + --> $DIR/fn_must_use.rs:85:5 | LL | m == n; //~ WARN unused comparison | ^^^^^^