Skip to content

Commit

Permalink
#[must_use] for associated functions is supposed to actually work
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
zackmdavis committed Oct 13, 2018
1 parent c47785f commit ab91a6b
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ impl<T: ?Sized> Clone for Rc<T> {
///
/// let five = Rc::new(5);
///
/// Rc::clone(&five);
/// let _ = Rc::clone(&five);
/// ```
#[inline]
fn clone(&self) -> Rc<T> {
Expand Down Expand Up @@ -1304,7 +1304,7 @@ impl<T: ?Sized> Clone for Weak<T> {
///
/// let weak_five = Rc::downgrade(&Rc::new(5));
///
/// Weak::clone(&weak_five);
/// let _ = Weak::clone(&weak_five);
/// ```
#[inline]
fn clone(&self) -> Weak<T> {
Expand Down
4 changes: 2 additions & 2 deletions src/liballoc/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ impl<T: ?Sized> Clone for Arc<T> {
///
/// let five = Arc::new(5);
///
/// Arc::clone(&five);
/// let _ = Arc::clone(&five);
/// ```
#[inline]
fn clone(&self) -> Arc<T> {
Expand Down Expand Up @@ -1135,7 +1135,7 @@ impl<T: ?Sized> Clone for Weak<T> {
///
/// let weak_five = Arc::downgrade(&Arc::new(5));
///
/// Weak::clone(&weak_five);
/// let _ = Weak::clone(&weak_five);
/// ```
#[inline]
fn clone(&self) -> Weak<T> {
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/resolve-pseudo-shadowing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

fn check<Clone>(_c: Clone) {
fn check2() {
<() as std::clone::Clone>::clone(&());
let _ = <() as std::clone::Clone>::clone(&());
}
check2();
}
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/fn_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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`
Expand Down
20 changes: 13 additions & 7 deletions src/test/ui/fn_must_use.stderr
Original file line number Diff line number Diff line change
@@ -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
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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
| ^^^^^^
Expand Down

0 comments on commit ab91a6b

Please sign in to comment.