Skip to content

Commit

Permalink
Auto merge of #43728 - zackmdavis:fnused, r=eddyb
Browse files Browse the repository at this point in the history
#[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.
  • Loading branch information
bors committed Aug 9, 2017
2 parents 0f9317d + f5ac228 commit 3f977ba
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 9 deletions.
7 changes: 7 additions & 0 deletions src/libcore/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}
Expand Down Expand Up @@ -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>;

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
34 changes: 25 additions & 9 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(": ");
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/lint/fn_must_use.rs
Original file line number Diff line number Diff line change
@@ -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();
}
14 changes: 14 additions & 0 deletions src/test/ui/lint/fn_must_use.stderr
Original file line number Diff line number Diff line change
@@ -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();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0 comments on commit 3f977ba

Please sign in to comment.