From a6ef99e9f43e70c36dc9744bc87374033b2dedba Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 15 Sep 2017 22:57:12 -0400 Subject: [PATCH 1/9] Indicate how ChildStd{in,out,err} FDs are closed. Fixes https://github.com/rust-lang/rust/issues/41452. --- src/libstd/process.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index a3a7e91dd807d..4dba9a4cb1123 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -153,8 +153,12 @@ impl fmt::Debug for Child { /// /// This struct is used in the [`stdin`] field on [`Child`]. /// +/// When an instance of `ChildStdin` is [dropped], the `ChildStdin`'s underlying +/// file handle will be closed. +/// /// [`Child`]: struct.Child.html /// [`stdin`]: struct.Child.html#structfield.stdin +/// [dropped]: ../ops/trait.Drop.html #[stable(feature = "process", since = "1.0.0")] pub struct ChildStdin { inner: AnonPipe @@ -196,8 +200,12 @@ impl fmt::Debug for ChildStdin { /// /// This struct is used in the [`stdout`] field on [`Child`]. /// +/// When an instance of `ChildStdout` is [dropped], the `ChildStdout`'s +/// underlying file handle will be closed. +/// /// [`Child`]: struct.Child.html /// [`stdout`]: struct.Child.html#structfield.stdout +/// [dropped]: ../ops/trait.Drop.html #[stable(feature = "process", since = "1.0.0")] pub struct ChildStdout { inner: AnonPipe @@ -239,8 +247,12 @@ impl fmt::Debug for ChildStdout { /// /// This struct is used in the [`stderr`] field on [`Child`]. /// +/// When an instance of `ChildStderr` is [dropped], the `ChildStderr`'s +/// underlying file handle will be closed. +/// /// [`Child`]: struct.Child.html /// [`stderr`]: struct.Child.html#structfield.stderr +/// [dropped]: ../ops/trait.Drop.html #[stable(feature = "process", since = "1.0.0")] pub struct ChildStderr { inner: AnonPipe From a1f9052be76c7d28fe2ee1ff7dcb2464237c2bfc Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 15 Sep 2017 23:02:50 -0400 Subject: [PATCH 2/9] Expand some of the std{in,out,err} usages. --- src/libstd/process.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 4dba9a4cb1123..a70e632fe7cc4 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -106,15 +106,18 @@ use sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; pub struct Child { handle: imp::Process, - /// The handle for writing to the child's stdin, if it has been captured + /// The handle for writing to the child's standard input (stdin), if it has + /// been captured. #[stable(feature = "process", since = "1.0.0")] pub stdin: Option, - /// The handle for reading from the child's stdout, if it has been captured + /// The handle for reading from the child's standard output (stdout), if it + /// has been captured. #[stable(feature = "process", since = "1.0.0")] pub stdout: Option, - /// The handle for reading from the child's stderr, if it has been captured + /// The handle for reading from the child's standard error (stderr), if it + /// has been captured. #[stable(feature = "process", since = "1.0.0")] pub stderr: Option, } @@ -149,7 +152,7 @@ impl fmt::Debug for Child { } } -/// A handle to a child process's stdin. +/// A handle to a child process's standard input (stdin). /// /// This struct is used in the [`stdin`] field on [`Child`]. /// @@ -196,7 +199,7 @@ impl fmt::Debug for ChildStdin { } } -/// A handle to a child process's stdout. +/// A handle to a child process's standard output (stdout). /// /// This struct is used in the [`stdout`] field on [`Child`]. /// @@ -546,7 +549,8 @@ impl Command { self } - /// Configuration for the child process's stdin handle (file descriptor 0). + /// Configuration for the child process's standard input (stdin) handle + /// (file descriptor 0). /// /// # Examples /// @@ -566,7 +570,8 @@ impl Command { self } - /// Configuration for the child process's stdout handle (file descriptor 1). + /// Configuration for the child process's standard output (stdout) handle + /// (file descriptor 1). /// /// # Examples /// @@ -586,7 +591,8 @@ impl Command { self } - /// Configuration for the child process's stderr handle (file descriptor 2). + /// Configuration for the child process's standard error (stderr) handle + /// (file descriptor 2). /// /// # Examples /// From 5ee7db6a0ebe8803d420c591325605111a21a400 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Thu, 21 Sep 2017 21:01:51 -0400 Subject: [PATCH 3/9] Remove platform-specific terminology. --- src/libstd/process.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index a70e632fe7cc4..33451a470d013 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -549,8 +549,7 @@ impl Command { self } - /// Configuration for the child process's standard input (stdin) handle - /// (file descriptor 0). + /// Configuration for the child process's standard input (stdin) handle. /// /// # Examples /// @@ -570,8 +569,7 @@ impl Command { self } - /// Configuration for the child process's standard output (stdout) handle - /// (file descriptor 1). + /// Configuration for the child process's standard output (stdout) handle. /// /// # Examples /// @@ -591,8 +589,7 @@ impl Command { self } - /// Configuration for the child process's standard error (stderr) handle - /// (file descriptor 2). + /// Configuration for the child process's standard error (stderr) handle. /// /// # Examples /// From 859ebef62fa0e7fcfc2af200dac11d2f899ea37f Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Thu, 21 Sep 2017 21:11:11 -0400 Subject: [PATCH 4/9] Add note about being blocked on input. --- src/libstd/process.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 33451a470d013..1869ad3ed707a 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -157,7 +157,8 @@ impl fmt::Debug for Child { /// This struct is used in the [`stdin`] field on [`Child`]. /// /// When an instance of `ChildStdin` is [dropped], the `ChildStdin`'s underlying -/// file handle will be closed. +/// file handle will be closed. If the child process was blocked on input prior +/// to being dropped, it will become unblocked after dropping. /// /// [`Child`]: struct.Child.html /// [`stdin`]: struct.Child.html#structfield.stdin From 8917616e6a295fb4080c8e29362dc1e5a477c479 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 22 Sep 2017 15:45:47 -0700 Subject: [PATCH 5/9] add comparison operators to must-use lint (under `fn_must_use` feature) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (3645b062).) 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. --- src/librustc_lint/unused.rs | 18 +++++++++++++++++- src/libsyntax/feature_gate.rs | 2 +- .../fn_must_use.rs | 7 +++---- .../fn_must_use.stderr | 8 +++++++- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 439cc3a4b844e..b97920dd18b77 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -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, _) => { @@ -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"); } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 1fef382c83a34..5c730aaa8d0fa 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -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) diff --git a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs index 7eb4c32972a25..821cd30c8df41 100644 --- a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs +++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs @@ -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 } diff --git a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr index 69755c89b4849..7fc0a4ce1edc7 100644 --- a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr +++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr @@ -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 + | ^^^^^^ + From 1797a942b710f911cb5b0b0ae56b5dcf62a81d2d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sat, 23 Sep 2017 02:25:17 -0700 Subject: [PATCH 6/9] Add span label to E0381 for MIR borrowck --- src/librustc_mir/borrow_check.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 5133e528b099a..9b478fe230935 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -896,10 +896,13 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> fn report_use_of_moved(&mut self, _context: Context, (lvalue, span): (&Lvalue, Span)) { - let mut err = self.tcx.cannot_act_on_uninitialized_variable( - span, "use", &self.describe_lvalue(lvalue), Origin::Mir); - // FIXME: add span_label for use of uninitialized variable - err.emit(); + self.tcx.cannot_act_on_uninitialized_variable(span, + "use", + &self.describe_lvalue(lvalue), + Origin::Mir) + .span_label(span, format!("use of possibly uninitialized `{}`", + self.describe_lvalue(lvalue))) + .emit(); } fn report_move_out_while_borrowed(&mut self, From e30abfbfe78722bdd33cecca32caccf86365f9da Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 23 Sep 2017 14:06:35 +0200 Subject: [PATCH 7/9] Fix warning position in rustdoc code blocks --- src/librustdoc/html/static/rustdoc.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index c15051376bf27..926a76f828575 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -815,7 +815,7 @@ span.since { .information { position: absolute; - left: -1px; + left: -20px; margin-top: 7px; } From 6734d39b49dd00fe5ba1ea875a3e03f9eca23ac0 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 23 Sep 2017 10:11:39 -0700 Subject: [PATCH 8/9] update `fn_must_use` UI test to exercise nonprimitive comparisons --- .../fn_must_use.rs | 11 +++++-- .../fn_must_use.stderr | 32 +++++++++++++------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs index 821cd30c8df41..3741ba4f3ae7a 100644 --- a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs +++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.rs @@ -11,6 +11,7 @@ #![feature(fn_must_use)] #![warn(unused_must_use)] +#[derive(PartialEq, Eq)] struct MyStruct { n: usize, } @@ -58,12 +59,18 @@ fn main() { need_to_use_this_value(); let mut m = MyStruct { n: 2 }; + let n = MyStruct { n: 3 }; + m.need_to_use_this_method_value(); m.is_even(); // trait method! m.replace(3); // won't warn (annotation needs to be in trait definition) - 2.eq(&3); // comparison methods are `must_use` + // comparison methods are `must_use` + 2.eq(&3); + m.eq(&n); - 2 == 3; // lint includes comparison operators + // lint includes comparison operators + 2 == 3; + m == n; } diff --git a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr index 7fc0a4ce1edc7..fdd0a591bc78d 100644 --- a/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr +++ b/src/test/ui/rfc_1940-must_use_on_functions/fn_must_use.stderr @@ -1,7 +1,7 @@ warning: unused return value of `need_to_use_this_value` which must be used: it's important - --> $DIR/fn_must_use.rs:58:5 + --> $DIR/fn_must_use.rs:59:5 | -58 | need_to_use_this_value(); +59 | need_to_use_this_value(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: lint level defined here @@ -11,26 +11,38 @@ note: lint level defined here | ^^^^^^^^^^^^^^^ warning: unused return value of `MyStruct::need_to_use_this_method_value` which must be used - --> $DIR/fn_must_use.rs:61:5 + --> $DIR/fn_must_use.rs:64:5 | -61 | m.need_to_use_this_method_value(); +64 | m.need_to_use_this_method_value(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: unused return value of `EvenNature::is_even` which must be used: no side effects - --> $DIR/fn_must_use.rs:62:5 + --> $DIR/fn_must_use.rs:65:5 | -62 | m.is_even(); // trait method! +65 | m.is_even(); // trait method! | ^^^^^^^^^^^^ warning: unused return value of `std::cmp::PartialEq::eq` which must be used - --> $DIR/fn_must_use.rs:66:5 + --> $DIR/fn_must_use.rs:70:5 | -66 | 2.eq(&3); // comparison methods are `must_use` +70 | 2.eq(&3); | ^^^^^^^^^ +warning: unused return value of `std::cmp::PartialEq::eq` which must be used + --> $DIR/fn_must_use.rs:71:5 + | +71 | m.eq(&n); + | ^^^^^^^^^ + +warning: unused comparison which must be used + --> $DIR/fn_must_use.rs:74:5 + | +74 | 2 == 3; + | ^^^^^^ + warning: unused comparison which must be used - --> $DIR/fn_must_use.rs:68:5 + --> $DIR/fn_must_use.rs:75:5 | -68 | 2 == 3; // lint includes comparison operators +75 | m == n; | ^^^^^^ From ed59a868dddbd56749c13bc7ddc7ea849d83e107 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sat, 23 Sep 2017 11:52:53 -0700 Subject: [PATCH 9/9] Add span labels for E0505 for MIR borrowck --- src/librustc_mir/borrow_check.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 9b478fe230935..10825323e4129 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -408,7 +408,7 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> self.each_borrow_involving_path( context, lvalue_span.0, flow_state, |this, _idx, borrow| { if !borrow.compatible_with(BorrowKind::Mut) { - this.report_move_out_while_borrowed(context, lvalue_span); + this.report_move_out_while_borrowed(context, lvalue_span, borrow); Control::Break } else { Control::Continue @@ -907,12 +907,17 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> fn report_move_out_while_borrowed(&mut self, _context: Context, - (lvalue, span): (&Lvalue, Span)) { - let mut err = self.tcx.cannot_move_when_borrowed( - span, &self.describe_lvalue(lvalue), Origin::Mir); - // FIXME 1: add span_label for "borrow of `()` occurs here" - // FIXME 2: add span_label for "move out of `{}` occurs here" - err.emit(); + (lvalue, span): (&Lvalue, Span), + borrow: &BorrowData) { + self.tcx.cannot_move_when_borrowed(span, + &self.describe_lvalue(lvalue), + Origin::Mir) + .span_label(self.retrieve_borrow_span(borrow), + format!("borrow of `{}` occurs here", + self.describe_lvalue(&borrow.lvalue))) + .span_label(span, format!("move out of `{}` occurs here", + self.describe_lvalue(lvalue))) + .emit(); } fn report_use_while_mutably_borrowed(&mut self,