Skip to content

Commit

Permalink
add comparison operators to must-use lint (under fn_must_use feature)
Browse files Browse the repository at this point in the history
Although RFC 1940 is about annotating functions with `#[must_use]`, a
key part of the motivation was linting unused equality operators.

(See
https://github.com/rust-lang/rfcs/pull/1812#issuecomment-265695898—it
seems to have not been clear to discussants at the time that marking the
comparison methods as `must_use` would not give us the lints on
comparison operators, at least in (what the present author understood
as) the most straightforward implementation, as landed in rust-lang#43728
(3645b06).)

To rectify the situation, we here lint unused comparison operators as
part of the unused-must-use lint (feature gated by the `fn_must_use`
feature flag, which now arguably becomes a slight (tolerable in the
opinion of the present author) misnomer).

This is in the matter of rust-lang#43302.
  • Loading branch information
zackmdavis committed Sep 22, 2017
1 parent 14039a4 commit 8917616
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 7 deletions.
18 changes: 17 additions & 1 deletion src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
};

let mut fn_warned = false;
let mut op_warned = false;
if cx.tcx.sess.features.borrow().fn_must_use {
let maybe_def = match expr.node {
hir::ExprCall(ref callee, _) => {
Expand All @@ -172,9 +173,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
let def_id = def.def_id();
fn_warned = check_must_use(cx, def_id, s.span, "return value of ");
}

if let hir::ExprBinary(bin_op, ..) = expr.node {
match bin_op.node {
// Hardcoding the comparison 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::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
let msg = "unused comparison which must be used";
cx.span_lint(UNUSED_MUST_USE, expr.span, msg);
op_warned = true;
},
_ => {},
}
}
}

if !(ty_warned || fn_warned) {
if !(ty_warned || fn_warned || op_warned) {
cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ declare_features! (
// #[doc(masked)]
(active, doc_masked, "1.21.0", None),

// allow `#[must_use]` on functions (RFC 1940)
// allow `#[must_use]` on functions and comparison operators (RFC 1940)
(active, fn_must_use, "1.21.0", Some(43302)),

// allow '|' at beginning of match arms (RFC 1925)
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ fn main() {
m.need_to_use_this_method_value();
m.is_even(); // trait method!

m.replace(3);
m.replace(3); // won't warn (annotation needs to be in trait definition)

2.eq(&3);
2.eq(&3); // comparison methods are `must_use`

// FIXME: operators should probably be `must_use` if underlying method is
2 == 3;
2 == 3; // lint includes comparison operators
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ warning: unused return value of `EvenNature::is_even` which must be used: no sid
warning: unused return value of `std::cmp::PartialEq::eq` which must be used
--> $DIR/fn_must_use.rs:66:5
|
66 | 2.eq(&3);
66 | 2.eq(&3); // comparison methods are `must_use`
| ^^^^^^^^^

warning: unused comparison which must be used
--> $DIR/fn_must_use.rs:68:5
|
68 | 2 == 3; // lint includes comparison operators
| ^^^^^^

0 comments on commit 8917616

Please sign in to comment.