Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling a function with wrong number of arguments and a while loop or its equivalence yields weird diagnostics #98894

Closed
WaffleLapkin opened this issue Jul 4, 2022 · 1 comment · Fixed by #98785
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information.

Comments

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Jul 4, 2022

Explanation / examples

This perfectly wrong statement causes compiler to mention ifs, ! and () even though nothing like this is written in the code:

(|_, ()| ())(while true {});
error[E0308]: `if` and `else` have incompatible types
 --> ./t.rs:2:18
  |
2 |     (|_, ()| ())(while true {});
  |                  ^^^^^^^^^^^--
  |                  |          |
  |                  |          expected because of this
  |                  expected `()`, found `!`
  |
  = note: expected unit type `()`
                  found type `!`

(here and after normal errors about argument number/type mismatch are not shown as they are not the problem)

Originally I've found this while editing some async code and passing String instead of &str. I was very confused and thought that it somehow relates to async/impl Trait/etc but after some minification I was left with the snippet above.

This error becomes less confusing after desugaring while loop by hand:

(|_, ()| ())(loop { if true {} else {break;} });
error[E0308]: `if` and `else` have incompatible types
 --> ./t.rs:2:42
  |
2 |     (|_, ()| ())(loop { if true {} else {break;} });
  |                                 --       ^^^^^^ expected `()`, found `!`
  |                                 |
  |                                 expected because of this
  |
  = note: expected unit type `()`
                  found type `!`

Note that using break; is required to trigger this bug, using break doesn't trigger it for some reason.

In fact, you don't even need a loop:

(|_, ()| ())(if true {} else {return;});
error[E0308]: `if` and `else` have incompatible types
 --> ./t.rs:2:35
  |
2 |     (|_, ()| ())(if true {} else {return;});
  |                          --       ^^^^^^^ expected `()`, found `!`
  |                          |
  |                          expected because of this
  |
  = note: expected unit type `()`
                  found type `!`

So in the end, this can be triggered with an if/else one of branches on which is diverging expression with ; that is being passed to a function with wrong number of arguments.

Note about confusion / applicability to real programs

As I previously said, I've found this while working on async code and I think this is the main way how this error can be seen (let's be serious, nobody calls functions with while loop arguments). The reason why it's easier to find this error with async is that async{} blocks are commonly passed to functions (like tokio::spawn) and can contain while loops inside them.

In my case the code looked something like this:

spawn(arg_with_wrong_type, async {
    while let Some(_) = rx.recv().await {
        // ..

        some_atomic.store(true, Ordering::Relaxed);
    }
})

Which caused an error like this:

2022-07-04_11-28

Note that "expected because of this" points to the last statement in the while{} block even though it has nothing to do, really, with the error.

Bisection

searched nightlies: from nightly-2022-04-10 to nightly-2022-05-09
regressed nightly: nightly-2022-04-17
searched commit range: 3f391b8...878c783
regressed commit: 07bb916

bisected with cargo-bisect-rustc v0.6.3

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --script=./test.sh --end=2022-05-09

src/main.rs:

fn main() {
    (|_, ()| ())(async { while true {} });
}

test.sh:

#!/bin/sh

# Fail if we no longer get a single error (and instead get at least two)
cargo check 2>&1 | grep "due to previous error;"

Cause

Bisection shows that this issue was introduced by #92364 which aligns with other observations. This is likely caused by the same underlying problem as #96335: diagnostic code calls check_expr a second time which confuses different parts of the compiler.

cc @compiler-errors as they've worked on fixing issues caused by #92364 before

Meta

This is reproducible with 1.63.0-beta.3 (2022-07-02 59f577d) and with most nightly-es after nightly-2022-04-17 (except for those that ICE).

@rustbot label +A-diagnostics +D-confusing +D-incorrect

@WaffleLapkin WaffleLapkin added the C-bug Category: This is a bug. label Jul 4, 2022
@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Jul 4, 2022
@compiler-errors
Copy link
Member

I wonder if this is also fixed by #98785 -- I will check and/or fix this too.

@rustbot claim

@WaffleLapkin WaffleLapkin changed the title Calling a function with wrong number of arguments and a while loop or its equivalence yileds weird diagnostics Calling a function with wrong number of arguments and a while loop or its equivalence yields weird diagnostics Jul 4, 2022
@bors bors closed this as completed in c6ff90b Jul 10, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 23, 2022
…rk-Simulacrum

Add some additional double-adjustment regression tests

I accidentally missed these when I rebased rust-lang#98785

cc rust-lang#98894 and rust-lang#98897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants