Baffled By The Borrow Checker

help
1

Man, step away from Rust for six months and you feel like an idiot again.

Consider this sample:

    // Move client to an option so we can _move_ the inner value
    // on the first attempt to connect. All other attempts will fail.
    let mut client = Some(client);
    let channel = Endpoint::try_from("http://[::]:50051")?
        .connect_with_connector(service_fn(move |_: Uri| {
            let client = client.take();

            async move {
                if let Some(client) = client {
                    Ok(TokioIo::new(client))
                } else {
                    Err(std::io::Error::new(
                        std::io::ErrorKind::Other,
                        "Client already taken",
                    ))
                }
            }
        }))
        .await?;

I borrowed this code for my own in-memory gRPC server (for unit-testing purposes) and it works great. I wanted to simplify this block - I've no need to guard against multiple clients (which is apparently what the comment indicates this is guarding against).

The first step - getting rid of the Option let check - works (though the compiler then requires you to type the Result instance):

            let mut client = Some(client_duplex);
            Ok(Endpoint::try_from("http://[::]:50051")
                .map_err(|e| e.to_string())?
                .connect_with_connector(service_fn(move |_: Uri| {
                    let client: Option<tokio::io::DuplexStream> = client.take();

                    async move {
                        Ok::<TokioIo<DuplexStream>, std::io::Error>(TokioIo::new(client.unwrap()))
                    }
                }))
                .await

The next step would be to get rid of the Option. Why do we need an Option here? There would seem to be no reason: We create the wrapper then take the value out.

So I tried this:

            let mut _client = client_duplex;
            Ok(Endpoint::try_from("http://[::]:50051")
                .map_err(|e| e.to_string())?
                .connect_with_connector(service_fn(move |_: Uri| {
                    let client = _client;

                    async move {
                        Ok::<TokioIo<DuplexStream>, std::io::Error>(TokioIo::new(client))
                    }
                }))
                .await

Now the compiler will not have it:

error[E0525]: expected a closure that implements the `FnMut` trait, but this closure only implements `FnOnce`
   |
89 |                 .connect_with_connector(service_fn(move |_: Uri| {
   |                  ----------------------            ^^^^^^^^^^^^^ this closure implements `FnOnce`, not `FnMut`
   |                  |
   |                  the requirement to implement `FnMut` derives from here
90 |                     let client = _client;
   |                                  ------- closure is `FnOnce` because it moves the variable `_client` out of its environment
   |
   = note: required for `ServiceFn<{closure@services/src/./tests/eze_client_test.rs:89:52: 89:65}>` to implement `Service<Uri>`

Why does this resultant closure type only to FnOnce and not FnMut? The compiler tells us, helpfully, that "closure is FnOnce because it moves the variable _client out of its environment - but doesn't the previous version (that compiles) do the same thing?

(Secondarily, regarding "the requirement to implement FnMut derives from here," how is that evident from the function signature of connect_with_connector?)

2

The TLDR is a service_fn() can be called multiple times (for multiple http requests) so the compiler is yelling at you that you consume client on the first call and it doesn't know what to do on the second.

The example is explicitly returning a friendly error to the second connection, not actually supporting multiple clients (which would need to allocate new responses or share the existing one with Arc or the like)

2 Likes
3

I understand the original code doesn't support multiple clients - it guards against them, as I said.

Unfortunately, from your response I still don't see what the meaningful difference is between the version that compiles (which wraps the client in an Option and calls take() on it) and the one the borrow checker complains about.

4
  • Using Option and take() leaves behind a None for the second call of the function to find and handle with an error message.
  • Moving the client directly leaves behind an uninitialized variable _client, which means the function cannot be valid to call more than once, because reading an uninitialized variable must not happen.

Using Option and take() is essentially adding a marker for “this had been moved away and is no longer available”, which you don't have with a value moved directly out.

2 Likes
5

It’s a multi-step process.

pub async fn connect_with_connector<C>(
    &self,
    connector: C,
) -> Result<Channel, Error>
where
    C: Service<Uri> + Send + 'static,
    C::Response: Read + Write + Send + Unpin,
    C::Future: Send,
    Box<dyn Error + Send + Sync>: From<C::Error> + Send,

expects a C: Service<…>.

Now connecting the dots…

impl<T, F, Request, R, E> Service<Request> for ServiceFn<T>
where
    T: FnMut(Request) -> F,
    F: Future<Output = Result<R, E>>,

and there you have the FnMut :wink:

1 Like
6

The compiler determines which Fn* trait a closure implements by analyzing how the closure body uses the captures in its environment. If the closure moves and drops any of its captures, then it can only implement FnOnce. If it only uses its captures as &mut T, then it can implement FnMut. If it only uses its captures as &T, then it can implement Fn. [1]

There is some real subtlety, here! move closures do not control which trait is implemented. Only how the captures are used influences it. This point came up a short while ago in another thread:

In short, no! As explained by others, Option::take() only moves the inner T out of the Option. The Option itself doesn't get dropped. If it did, that would make the closure implement FnOnce instead of FnMut. Just like the demonstration closures in the playground.

  1. Note that the closure does not have to use all of its captures in these ways. The trait that gets implemented (by the compiler) is selected by the capture use that has the most restriction. Even if most captures are used by &T, just one use by &mut T of any capture will make the closure FnMut. If that makes sense... ↩︎

1 Like
7

I suspected something like this. In a sense, it's cool, but in another sense it's not because we need to basically trick the compiler into being happy.

(I'm in a position now of having to evangelize Rust to some skeptics and things like this make it a bit harder.)

8

Many other languages would probably leave a copy, a reference to the same object, or something like null behind. Rust doesn't do any of those automatically, but lets you pick which one fits your case. Including neither of them if that fits the semantics better. Here you essentially opted into the leave-null-behind case with Option. This could be seen as a selling point.

6 Likes
9

I wouldn’t call this “tricking the compiler” at all. Ownership simply means you can only consume the value once. The compiler will not allow code that consumes a value more than once, even potentially; an Option-wrapped value needs not be fully consumed because you can always put it into a None state instead, which is still valid.

But this isn’t something you can do in general for all types, because Rust avoids the “billion dollar mistake” (the implicit possibility for nulls, and thus the possibility for crashes absolutely everywhere throughout the whole program).

So normal moves in Rust are not a “replace with null” kind of operation (unlike the realization of “move” in C++[1]); and Option::take is the canonical way to do this kind of approach if you need it anyways.[2]

It then forces manual handling of a null-case. In the code example you started with, that’s a graceful error handling; in your “simplicification” it’s an explicit unwrap (with a "Option = explicitly nullable" kind of view, .unwrap() is but an explicit marker of all places where you’ve left potentially-unhandled cases of null pointer exceptions in your code).

  1. it’s also not exactly some uniform “null” there, but basically, you leave the moved-from value in a technically-still-“valid” state (and the destructor will later still run on this value); and for owning smart pointers that’s usually null ↩︎

  2. Some types already have possible null-like values themselves; the generalization of Option::take is std::mem::take (when the type’s Default value is a good thing to leave behind) or std::mem::replace (if you want to define yourself what arbitrary value to leave behind) ↩︎

5 Likes
10

Fair enough. You're right - it's the absence of null in the language that makes a context (wrapper/monad) necessary here.

1 Like
11

It's the compiler tricking the programmer into being happy by avoiding many sources of runtime UB or crashes or null pointer exceptions, by requiring them to be explicit about what to do when a value needs to be used after it has been moved.

2 Likes