Logically identical code. One compiles, one does not

[schneems]

I tried to compile this code playground link:

use std::time::{Duration, Instant};
use std::thread;

fn time_bounded_retry<T, E>(
    max_time: Duration,
    sleep_for: Duration,
    f: impl Fn() -> Result<T, E>,
) -> Result<T, E> {
    let start = Instant::now();
    while let result = f() {
        match result {
            Ok(_) => return result,
            Err(_) => {
                if start.elapsed() > max_time {
                    return result;
                }

                thread::sleep(sleep_for);
            }
        }
    }
    // unreachable!()
}

However, it produces an error. If you uncomment the unreachable!(), it will work. Here's the error message:

    |
191 |     while let result = f() {
    |     ^^^^^^^^^^^^^^^^^^^^^^ this might have zero elements to iterate on
192 |         match result {
193 |             Ok(_) => return result,
    |                      ------------- if the loop doesn't execute, this value would never get returned
...
196 |                     return result;
    |                     ------------- if the loop doesn't execute, this value would never get returned
    = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility
help: try adding an expression at the end of the block

This code is functionally identical to the while let code and can compile without an unreachable!():

fn time_bounded_retry<T, E>(
    max_time: Duration,
    sleep_for: Duration,
    f: impl Fn() -> Result<T, E>,
) -> Result<T, E> {
    let start = Instant::now();
    loop {
        let result = f();
        match result {
            Ok(_) => return result,
            Err(_) => {
                if start.elapsed() > max_time {
                    return result;
                }

                thread::sleep(sleep_for);
            }
        }
    }
}

The code in the while loop is irrefutable, so it is functionally the same as the loop case. Even if it's not a good practice to use an irrefutable statement in a while loop (and clippy will catch this) I think it should still compile.

My main reason for wanting it to compile is that while iterating, I wrote the while let version first, and because I could not get it to compile without the unreachable!() statement mistakenly believed that the compiler would not allow something like the second result. As a result of that mistaken impression ended up with this code:

fn time_bounded_retry<T, E>(
    max_time: Duration,
    sleep_for: Duration,
    f: impl Fn() -> Result<T, E>,
) -> Result<T, E> {
    let start = Instant::now();
    let mut result;
    loop {
        result = f();
        if result.is_ok() || start.elapsed() > max_time {
            break;
        }
        thread::sleep(sleep_for);
    }
    result
}

Which is not ideal (needs a temp mutable variable). A co-worker without the above learned helplessness unreachable!() experience wrote this code below, which I believe is a much better way to implement the logic as it does not require a temp mutable variable:

fn time_bounded_retry<T, E>(
    max_time: Duration,
    sleep_for: Duration,
    f: impl Fn() -> Result<T, E>,
) -> Result<T, E> {
    let start = Instant::now();
    loop {
        result = f();
        if result.is_ok() || start.elapsed() > max_time {
            return result;
        }
        thread::sleep(sleep_for);
    }
}

So effectively, I'm advocating for while let var = //... to be treated the same as loop { let var = //..., not because I think it's a good idea, but because having one error and not the other gives false feedback to the coder.

Meta

rustc --version --verbose:

$ rustc --version --verbose
rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-apple-darwin
release: 1.73.0
LLVM version: 17.0.2

[Nadrieril]

If we do this, I'd expect while let (x, y) = foo() {} to also typecheck like a loop {}. This would require typechecking to depend on the code that checks refutability of patterns, which would be pretty hard to do today.

A workable solution would be instead to change the error message: the code that emits "this might have zero elements to iterate on" could check if the pattern is a single binding and emit a better error message if that's the case. I expect this is rather easy to do, so marking this as an easy issue.

[yossydev]

@rustbot claim

[Nadrieril]

You figured it out 👍 Open a topic on #t-compiler/help on Zulip when you have questions!

[yossydev]

@Nadrieril

You figured it out 👍 Open a topic on #t-compiler/help on Zulip when you have questions!

Thank you for your kind attention!

[maxprogrammer007]

In the while let version, the compiler sees the while let result = f() statement and tries to match the result of f() to the pattern result. However, since f() returns a Result<T, E> which can be either Ok(T) or Err(E), the compiler is unsure how to match this result to the pattern result. This ambiguity leads to the error you encountered.

On the other hand, in the loop version, the compiler doesn't have to match the result of f() to any pattern. It simply calls f() in each iteration and checks the result directly. This approach avoids the ambiguity and compiles successfully.

[Kohei316]

@Nadrieril
Is this still open? Can I work on it?

[Nadrieril]

Yes, go ahead!

[Kohei316]

@rustbot claim

[traviscross]

Minimization:

fn foo() -> bool {
    while let x = false {
        if x { return true }
    }
    //~^ ERROR mismatched types
}

Perhaps the compiler treats it similarly to this, which is what I would expect it might do:

fn foo() -> bool {
    loop {
        let x = false else { break };
        //                   ^^^^^
        //~^ ERROR mismatched types
        if x { return true }
    }
}

@rustbot labels +A-diagnostics

@Kohei316: To confirm, you're planning to improve the diagnostics as described above, not try to change the behavior, yes?

[Kohei316]

@traviscross

@Kohei316: To confirm, you're planning to improve the diagnostics as described above, not try to change the behavior, yes?

Yes.And thank you for your minimum code.It became clearer what to do.