You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
E0593 regression when mismatched argument count leads to other errors in Beta/Nightly #47244
use std::collections::HashMap;fnmain(){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.
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 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.
$ 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" })
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.
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.
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.
@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.
Activity
nikomatsakis commentedon Feb 1, 2018
cc @estebank -- was this fixed by #47747 ? Any idea what the problem is here?
nikomatsakis commentedon Feb 1, 2018
triage: P-high
estebank commentedon Feb 1, 2018
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 commentedon Feb 1, 2018
@estebank great, if you don't have time, lemme know
estebank commentedon Feb 5, 2018
@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 commentedon Feb 5, 2018
cc @rust-lang/compiler -- can somebody pick this up to try and get to the bottom of it?
kennytm commentedon Feb 5, 2018
Bisection result is #45879.
Details
Test script:
1.rsis the source in the bug report.Output:
nikomatsakis commentedon Feb 6, 2018
D'oh, that's my pr :)
nikomatsakis commentedon Feb 8, 2018
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 commentedon Feb 8, 2018
Anyway I think the problem roughly is here:
rust/src/librustc_typeck/check/closure.rs
Lines 419 to 422 in 932c736
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 commentedon Feb 8, 2018
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 commentedon Feb 9, 2018
@nikomatsakis for what is worth, I don't think we're taking advantage of
TyErrnearly 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 commentedon Feb 9, 2018
I have a fix.
nikomatsakis commentedon Feb 9, 2018
Something weird happened with git but once I get that straightened out, will open a PR.
Rollup merge of rust-lang#48123 - nikomatsakis:issue-47244-expected-n…
Rollup merge of rust-lang#48123 - nikomatsakis:issue-47244-expected-n…
Rollup merge of rust-lang#48123 - nikomatsakis:issue-47244-expected-n…