Auto merge of #50149 - aaronaaeng:master, r=estebank

Added warning for unused arithmetic expressions The compiler now displays a warning when a binary arithmetic operation is evaluated but not used. This resolves #50124 by following the instructions outlined in the issue. The changes are as follows: - Added new pattern matching for unused arithmetic expressions in `src/librustc_lint/unused.rs` - Added `#[must_use]` attributes to the binary operation methods in `src/libcore/internal_macros.rs` - Added `#[must_use]` attributes to the non-assigning binary operators in `src/libcore/ops/arith.rs`
rust-lang · Apr 28, 2018 · 1eb0cef · 1eb0cef
2 parents ede7f94 + 91aa267
commit 1eb0cef
Show file tree

Hide file tree

Showing 6 changed files with 224 additions and 15 deletions.
diff --git a/src/libcore/ops/arith.rs b/src/libcore/ops/arith.rs
@@ -94,6 +94,7 @@ pub trait Add<RHS=Self> {
     type Output;
 
     /// Performs the `+` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn add(self, rhs: RHS) -> Self::Output;
 }
@@ -191,6 +192,7 @@ pub trait Sub<RHS=Self> {
     type Output;
 
     /// Performs the `-` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn sub(self, rhs: RHS) -> Self::Output;
 }
@@ -310,6 +312,7 @@ pub trait Mul<RHS=Self> {
     type Output;
 
     /// Performs the `*` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn mul(self, rhs: RHS) -> Self::Output;
 }
@@ -433,6 +436,7 @@ pub trait Div<RHS=Self> {
     type Output;
 
     /// Performs the `/` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn div(self, rhs: RHS) -> Self::Output;
 }
@@ -517,6 +521,7 @@ pub trait Rem<RHS=Self> {
     type Output = Self;
 
     /// Performs the `%` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn rem(self, rhs: RHS) -> Self::Output;
 }
@@ -601,6 +606,7 @@ pub trait Neg {
     type Output;
 
     /// Performs the unary `-` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn neg(self) -> Self::Output;
 }

diff --git a/src/libcore/ops/bit.rs b/src/libcore/ops/bit.rs
@@ -46,6 +46,7 @@ pub trait Not {
     type Output;
 
     /// Performs the unary `!` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn not(self) -> Self::Output;
 }
@@ -129,6 +130,7 @@ pub trait BitAnd<RHS=Self> {
     type Output;
 
     /// Performs the `&` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn bitand(self, rhs: RHS) -> Self::Output;
 }
@@ -212,6 +214,7 @@ pub trait BitOr<RHS=Self> {
     type Output;
 
     /// Performs the `|` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn bitor(self, rhs: RHS) -> Self::Output;
 }
@@ -298,6 +301,7 @@ pub trait BitXor<RHS=Self> {
     type Output;
 
     /// Performs the `^` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn bitxor(self, rhs: RHS) -> Self::Output;
 }
@@ -385,6 +389,7 @@ pub trait Shl<RHS=Self> {
     type Output;
 
     /// Performs the `<<` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn shl(self, rhs: RHS) -> Self::Output;
 }
@@ -493,6 +498,7 @@ pub trait Shr<RHS=Self> {
     type Output;
 
     /// Performs the `>>` operation.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn shr(self, rhs: RHS) -> Self::Output;
 }

diff --git a/src/libcore/ops/deref.rs b/src/libcore/ops/deref.rs
@@ -77,6 +77,7 @@ pub trait Deref {
     type Target: ?Sized;
 
     /// Dereferences the value.
+    #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn deref(&self) -> &Self::Target;
 }

diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs
@@ -91,23 +91,35 @@ 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;
-                    },
-                    _ => {},
-                }
+            let must_use_op = match expr.node {
+                // Hardcoding 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::ExprBinary(bin_op, ..)  => {
+                    match bin_op.node {
+                        hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
+                            Some("comparison")
+                        },
+                        hir::BiAdd | hir::BiSub | hir::BiDiv | hir::BiMul | hir::BiRem => {
+                            Some("arithmetic operation")
+                        },
+                        hir::BiAnd | hir::BiOr => {
+                            Some("logical operation")
+                        },
+                        hir::BiBitXor | hir::BiBitAnd | hir::BiBitOr | hir::BiShl | hir::BiShr => {
+                            Some("bitwise operation")
+                        },
+                    }
+                },
+                hir::ExprUnary(..) => Some("unary operation"),
+                _ => None
+            };
+            if let Some(must_use_op) = must_use_op {
+                cx.span_lint(UNUSED_MUST_USE, expr.span,
+                    &format!("unused {} which must be used", must_use_op));
+                op_warned = true;
             }
         }
-
         if !(ty_warned || fn_warned || op_warned) {
             cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
         }

diff --git a/src/test/ui/lint/must-use-ops.rs b/src/test/ui/lint/must-use-ops.rs
@@ -0,0 +1,52 @@
+// Copyright 2018 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.
+
+// Issue #50124 - Test warning for unused operator expressions
+
+// compile-pass
+
+#![feature(fn_must_use)]
+#![warn(unused_must_use)]
+
+fn main() {
+    let val = 1;
+    let val_pointer = &val;
+
+// Comparison Operators
+    val == 1;
+    val < 1;
+    val <= 1;
+    val != 1;
+    val >= 1;
+    val > 1;
+
+// Arithmetic Operators
+    val + 2;
+    val - 2;
+    val / 2;
+    val * 2;
+    val % 2;
+
+// Logical Operators
+    true && true;
+    false || true;
+
+// Bitwise Operators
+    5 ^ val;
+    5 & val;
+    5 | val;
+    5 << val;
+    5 >> val;
+
+// Unary Operators
+    !val;
+    -val;
+    *val_pointer;
+}
diff --git a/src/test/ui/lint/must-use-ops.stderr b/src/test/ui/lint/must-use-ops.stderr
@@ -0,0 +1,132 @@
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:23:5
+   |
+LL |     val == 1;
+   |     ^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/must-use-ops.rs:16:9
+   |
+LL | #![warn(unused_must_use)]
+   |         ^^^^^^^^^^^^^^^
+
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:24:5
+   |
+LL |     val < 1;
+   |     ^^^^^^^
+
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:25:5
+   |
+LL |     val <= 1;
+   |     ^^^^^^^^
+
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:26:5
+   |
+LL |     val != 1;
+   |     ^^^^^^^^
+
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:27:5
+   |
+LL |     val >= 1;
+   |     ^^^^^^^^
+
+warning: unused comparison which must be used
+  --> $DIR/must-use-ops.rs:28:5
+   |
+LL |     val > 1;
+   |     ^^^^^^^
+
+warning: unused arithmetic operation which must be used
+  --> $DIR/must-use-ops.rs:31:5
+   |
+LL |     val + 2;
+   |     ^^^^^^^
+
+warning: unused arithmetic operation which must be used
+  --> $DIR/must-use-ops.rs:32:5
+   |
+LL |     val - 2;
+   |     ^^^^^^^
+
+warning: unused arithmetic operation which must be used
+  --> $DIR/must-use-ops.rs:33:5
+   |
+LL |     val / 2;
+   |     ^^^^^^^
+
+warning: unused arithmetic operation which must be used
+  --> $DIR/must-use-ops.rs:34:5
+   |
+LL |     val * 2;
+   |     ^^^^^^^
+
+warning: unused arithmetic operation which must be used
+  --> $DIR/must-use-ops.rs:35:5
+   |
+LL |     val % 2;
+   |     ^^^^^^^
+
+warning: unused logical operation which must be used
+  --> $DIR/must-use-ops.rs:38:5
+   |
+LL |     true && true;
+   |     ^^^^^^^^^^^^
+
+warning: unused logical operation which must be used
+  --> $DIR/must-use-ops.rs:39:5
+   |
+LL |     false || true;
+   |     ^^^^^^^^^^^^^
+
+warning: unused bitwise operation which must be used
+  --> $DIR/must-use-ops.rs:42:5
+   |
+LL |     5 ^ val;
+   |     ^^^^^^^
+
+warning: unused bitwise operation which must be used
+  --> $DIR/must-use-ops.rs:43:5
+   |
+LL |     5 & val;
+   |     ^^^^^^^
+
+warning: unused bitwise operation which must be used
+  --> $DIR/must-use-ops.rs:44:5
+   |
+LL |     5 | val;
+   |     ^^^^^^^
+
+warning: unused bitwise operation which must be used
+  --> $DIR/must-use-ops.rs:45:5
+   |
+LL |     5 << val;
+   |     ^^^^^^^^
+
+warning: unused bitwise operation which must be used
+  --> $DIR/must-use-ops.rs:46:5
+   |
+LL |     5 >> val;
+   |     ^^^^^^^^
+
+warning: unused unary operation which must be used
+  --> $DIR/must-use-ops.rs:49:5
+   |
+LL |     !val;
+   |     ^^^^
+
+warning: unused unary operation which must be used
+  --> $DIR/must-use-ops.rs:50:5
+   |
+LL |     -val;
+   |     ^^^^
+
+warning: unused unary operation which must be used
+  --> $DIR/must-use-ops.rs:51:5
+   |
+LL |     *val_pointer;
+   |     ^^^^^^^^^^^^
+