New nightly shows warnings that will become hard errors in the future

[MajorBreakfast]

Nightly from 2018-07-29

warning: variable does not need to be mutable
   --> futures-io/src/lib.rs:365:21
    |
365 |                 let mut out = self.get_mut().as_mut();
    |                     ----^^^
    |                     |
    |                     help: remove this `mut`
    |
    = note: `-D unused-mut` implied by `-D warnings`
    = warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
            It represents potential unsoundness in your code.
            This warning will become a hard error in the future.

    Checking futures-util-preview v0.3.0-alpha.1 (file:///Users/josef/Documents/GitHub/futures-rs/futures-util)
warning: variable does not need to be mutable
   --> futures-util/src/sink/with.rs:124:28
    |
124 |             State::Process(mut fut) => Some(try_ready!(fut.poll(cx))),
    |                            ----^^^
    |                            |
    |                            help: remove this `mut`
    |
    = note: `-D unused-mut` implied by `-D warnings`
    = warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
            It represents potential unsoundness in your code.
            This warning will become a hard error in the future.

warning[E0499]: cannot borrow `promoted` as mutable more than once at a time
  --> futures-util/src/io/read_exact.rs:43:61
   |
43 |                 let (_, rest) = mem::replace(&mut this.buf, &mut []).split_at_mut(n);
   |                                                             ^^^^^^^ mutable borrow starts here in previous iteration of loop
   |
note: borrowed value must be valid for the lifetime 'a as defined on the impl at 35:6...
  --> futures-util/src/io/read_exact.rs:35:6
   |
35 | impl<'a, R: AsyncRead + ?Sized> Future for ReadExact<'a, R> {
   |      ^^
   = warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
           It represents potential unsoundness in your code.
           This warning will become a hard error in the future.

[MajorBreakfast]

Note: These warnings are special because they don't break the CI even though we deny warnings.

[Nemo157]

That read_exact one is concerning. I guess it's a bug that the new borrow checker is now noticing, specifically if the this.buf = rest; line was missing then that line would have left this.buf pointing at a stack allocated buffer.

Can hopefully steal the std::io::Read::read_exact implementation: https://github.com/rust-lang/rust/blob/f338dba29705e144bad8b2a675284538dd514896/src/libstd/io/mod.rs#L687

(I also really want to know how having an unused mut binding could lead to unsoundness, I'm probably gonna try and track down that issue at some point).

EDIT: Argh no, the std one only works because it's a local variable. I'm not sure how to resolve this now... maybe NLL will allow some new solution?

[Nemo157]

One potential fix that shouldn't impact the size/performance of ReadExact much, but I'm still hoping there's something nicer (could probably make this a little nicer with a helper function):

diff --git futures-util/src/io/read_exact.rs futures-util/src/io/read_exact.rs
index 420ea191..da99a97e 100644
--- futures-util/src/io/read_exact.rs
+++ futures-util/src/io/read_exact.rs
@@ -14,7 +14,7 @@ use std::mem::{self, PinMut};
 #[derive(Debug)]
 pub struct ReadExact<'a, R: ?Sized + 'a> {
     reader: &'a mut R,
-    buf: &'a mut [u8],
+    buf: Option<&'a mut [u8]>,
 }

 impl<'a, R: ?Sized> Unpin for ReadExact<'a, R> {}
@@ -24,7 +24,7 @@ impl<'a, R: AsyncRead + ?Sized> ReadExact<'a, R> {
         reader: &'a mut R,
         buf: &'a mut [u8]
     ) -> ReadExact<'a, R> {
-        ReadExact { reader, buf }
+        ReadExact { reader, buf: Some(buf) }
     }
 }

@@ -37,16 +37,27 @@ impl<'a, R: AsyncRead + ?Sized> Future for ReadExact<'a, R> {

     fn poll(mut self: PinMut<Self>, cx: &mut task::Context) -> Poll<Self::Output> {
         let this = &mut *self;
-        while !this.buf.is_empty() {
-            let n = try_ready!(this.reader.poll_read(cx, this.buf));
-            {
-                let (_, rest) = mem::replace(&mut this.buf, &mut []).split_at_mut(n);
-                this.buf = rest;
-            }
-            if n == 0 {
-                return Poll::Ready(Err(eof()))
+        let mut buf = this.buf.take().unwrap();
+        while !buf.is_empty() {
+            match this.reader.poll_read(cx, buf) {
+                Poll::Ready(Ok(0)) => {
+                    this.buf = Some(buf);
+                    return Poll::Ready(Err(eof()));
+                }
+                Poll::Ready(Ok(n)) => {
+                    buf = &mut buf[n..];
+                }
+                Poll::Ready(Err(e)) => {
+                    this.buf = Some(buf);
+                    return Poll::Ready(Err(e));
+                }
+                Poll::Pending => {
+                    this.buf = Some(buf);
+                    return Poll::Pending;
+                }
             }
         }
+        this.buf = Some(buf);
         Poll::Ready(Ok(()))
     }
 }

[Nemo157]

@MajorBreakfast did you and @cramertj discuss using async internally? read_exact is one combinator that is so much more readable when defined using an async block.

[cramertj]

@Nemo157 Yeah, there are lots of combinators that'd be nice to define that way, but I think we want to keep all our types nameable for the sake of downstream users, at least until we get some form of nameable existential types.

[Nemo157]

I was thinking of wrapping the async block inside a trivial ReadExact struct, but now I realise that relies on having nameable existential types itself anyway, and at that point you may as well make ReadExact the named existential type.

[Nemo157]

For reference rust-lang/rust#52671 provoked the change that caused the ReadExact issues. Whether this sort of code should be allowed is still under dispute, so that warning may just resolve itself.

[MajorBreakfast]

The warnings are gone now