E0593 regression when mismatched argument count leads to other errors in Beta/Nightly

[Rantanen]

Playground

use std::collections::HashMap;
fn main() {
    
    let m = HashMap::new();
    m.insert( "foo", "bar" );
    
    m.iter().map( |_, b| { // Stable ONLY:
                           // > [E0593]: closure is expected to take a single tuple as argument

        b.to_string()      // Stable + Beta + Nightly:
                           // > [E0619]: the type of this value must be known in this context
    });
}

Commenting out the b.to_string() will yield the [E0593] error in all toolchains.

The [E0619] alone makes for a very confusing error message, as the user's assumption is that the type of b is correct.

[nikomatsakis]

cc @estebank -- was this fixed by #47747 ? Any idea what the problem is here?

[nikomatsakis]

triage: P-high

[estebank]

I don't think the presentation changes that that PR (and the one it's supplementing) introduced. It looks to me that it doesn't actually get to check E0593. I'll dig deeper.

[nikomatsakis]

@estebank great, if you don't have time, lemme know

[estebank]

@nikomatsakis I didn't have time to look into this and will unlikely have time to perform a bisect during the week. I can confirm that this was introduced earlier than #47747.

[nikomatsakis]

cc @rust-lang/compiler -- can somebody pick this up to try and get to the bottom of it?

[kennytm]

Bisection result is #45879.

Test script:

#!/bin/sh
$RUSTC_RELATIVE -V
! ( $RUSTC_RELATIVE 1.rs 2>&1 | grep E0593 )

1.rs is the source in the bug report.

Output:

$ RUST_LOG=rust_sysroot=info target/release/bisect --preserve --test test.sh --start 33374fa9d09e2a790979b31e61100dfed4b44139 --end b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d
INFO:rust_sysroot: Getting commits from the git checkout in 33374fa9d09e2a790979b31e61100dfed4b44139...b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d
INFO:rust_sysroot: Received 272 commits
Searching in 272 commits; about 9 steps
rustc 1.24.0-nightly (830599b19 2017-12-11)
rustc 1.24.0-nightly (315fbf751 2017-12-01)
rustc 1.23.0-nightly (2f84fb5cc 2017-11-26)
rustc 1.23.0-nightly (0916bbc00 2017-11-23)
rustc 1.23.0-nightly (96e9cee77 2017-11-22)
rustc 1.23.0-nightly (ebda7662d 2017-11-21)
error[E0593]: closure is expected to take a single tuple as argument, but it takes 2 distinct arguments
rustc 1.23.0-nightly (63739ab7b 2017-11-21)
error[E0593]: closure is expected to take a single tuple as argument, but it takes 2 distinct arguments
rustc 1.23.0-nightly (d6d09e0b4 2017-11-21)
searched commits 33374fa9d09e2a790979b31e61100dfed4b44139 through b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d
regression in 7; Some(Commit { sha: "d6d09e0b4dac93ae07dae6206bf95e7cea0124a2", date: 2017-11-21T22:52:19Z, summary: "Auto merge of #45879 - nikomatsakis:nll-kill-cyclic-closures, r=arielb1" })

[nikomatsakis]

D'oh, that's my pr :)

[nikomatsakis]

I wonder if this really broke in 053383d...? at least, when I trace what's happening, that seems to be where it goes wrong. But I guess the bisection doesn't lie.

[nikomatsakis]

Anyway I think the problem roughly is here:

rust/src/librustc_typeck/check/closure.rs

Lines 419 to 422 in 932c736

	} else if expected_sig.inputs_and_output.len() != decl.inputs.len() + 1 {
	// we could probably handle this case more gracefully
	return self.sig_of_closure_no_expectation(expr_def_id, decl, body);
	}

This corresponds to the case where (a) we figured out from context that the fn needs N parameters but (b) it is declared with M parameters. As you can see from the comment, "we could probably handle this case more gracefully". What we do now is to ignore the expectations and just give those parameters "unknown types".

I'm not sure yet the best way to fix this -- we could certainly report a useful error here, we have a lot of the information we want. But then I think we might get duplicate errors. We could perhaps report an error and then give the parameters the type of TyErr ("something went wrong!"), which would suppress duplicates and so forth.

Hmm, that's probably best.

[Rantanen]

We could perhaps report an error and then give the parameters the type of TyErr ("something went wrong!"), which would suppress duplicates and so forth.

As a user, the "Type must be known at compile time" error just ends up being confusing as that makes me search for ways to annotate the type, which is just wasted effort - so reporting the first error and skipping the second error might actually be better than reporting both issues.

[estebank]

@nikomatsakis for what is worth, I don't think we're taking advantage of TyErr nearly enough to silence errors down the line and I'm very much in favor of doing so in this case. As @Rantanen points out, the first error is much more useful than the later one, and this seems to be a common theme across different cases that result in multiple cascading errors.

[nikomatsakis]

I have a fix.

[nikomatsakis]

Something weird happened with git but once I get that straightened out, will open a PR.