Skip to content

Commit 8917616

Browse files
committed
add comparison operators to must-use lint (under fn_must_use feature)
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 #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 #43302.
1 parent 14039a4 commit 8917616

File tree

4 files changed

+28
-7
lines changed

4 files changed

+28
-7
lines changed

src/librustc_lint/unused.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
153153
};
154154

155155
let mut fn_warned = false;
156+
let mut op_warned = false;
156157
if cx.tcx.sess.features.borrow().fn_must_use {
157158
let maybe_def = match expr.node {
158159
hir::ExprCall(ref callee, _) => {
@@ -172,9 +173,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
172173
let def_id = def.def_id();
173174
fn_warned = check_must_use(cx, def_id, s.span, "return value of ");
174175
}
176+
177+
if let hir::ExprBinary(bin_op, ..) = expr.node {
178+
match bin_op.node {
179+
// Hardcoding the comparison operators here seemed more
180+
// expedient than the refactoring that would be needed to
181+
// look up the `#[must_use]` attribute which does exist on
182+
// the comparison trait methods
183+
hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
184+
let msg = "unused comparison which must be used";
185+
cx.span_lint(UNUSED_MUST_USE, expr.span, msg);
186+
op_warned = true;
187+
},
188+
_ => {},
189+
}
190+
}
175191
}
176192

177-
if !(ty_warned || fn_warned) {
193+
if !(ty_warned || fn_warned || op_warned) {
178194
cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
179195
}
180196

src/libsyntax/feature_gate.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ declare_features! (
380380
// #[doc(masked)]
381381
(active, doc_masked, "1.21.0", None),
382382

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

386386
// allow '|' at beginning of match arms (RFC 1925)

src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,9 @@ fn main() {
6161
m.need_to_use_this_method_value();
6262
m.is_even(); // trait method!
6363

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

66-
2.eq(&3);
66+
2.eq(&3); // comparison methods are `must_use`
6767

68-
// FIXME: operators should probably be `must_use` if underlying method is
69-
2 == 3;
68+
2 == 3; // lint includes comparison operators
7069
}

src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ warning: unused return value of `EvenNature::is_even` which must be used: no sid
2525
warning: unused return value of `std::cmp::PartialEq::eq` which must be used
2626
--> $DIR/fn_must_use.rs:66:5
2727
|
28-
66 | 2.eq(&3);
28+
66 | 2.eq(&3); // comparison methods are `must_use`
2929
| ^^^^^^^^^
3030

31+
warning: unused comparison which must be used
32+
--> $DIR/fn_must_use.rs:68:5
33+
|
34+
68 | 2 == 3; // lint includes comparison operators
35+
| ^^^^^^
36+

0 commit comments

Comments
 (0)