rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.6k stars 12.74k forks source link

parse errors result in massive, useless spew #31994

Closed brson closed 8 years ago

brson commented 8 years ago

In a recent project I had a single typo - failing to close a paren. The error message is below. Remember, there is only one error here, a missing paren. Every single error here is useless. This happens a lot.

rust-install/src/mock/clitools.rs:336:17: 336:18 error: incorrect close delimiter: `]`
rust-install/src/mock/clitools.rs:336                 ]
                                                      ^
rust-install/src/mock/clitools.rs:333:13: 333:14 note: unclosed delimiter
rust-install/src/mock/clitools.rs:333             ("rustc".to_string(),
                                                  ^
rust-install/src/mock/clitools.rs:337:5: 337:6 error: incorrect close delimiter: `}`
rust-install/src/mock/clitools.rs:337     }
                                          ^
rust-install/src/mock/clitools.rs:332:25: 332:26 note: unclosed delimiter
rust-install/src/mock/clitools.rs:332         components: vec![
                                                              ^
rust-install/src/mock/clitools.rs:479:3: 479:3 error: this file contains an un-closed delimiter
rust-install/src/mock/clitools.rs:479 }
                                       ^
rust-install/src/mock/clitools.rs:329:90: 329:91 help: did you mean to close this delimiter?
rust-install/src/mock/clitools.rs:329 fn build_mock_rustc_installer(version: &str, version_hash: &str) -> MockInstallerBuilder {
                                                                                                                               ^
rust-install/src/mock/clitools.rs:340:1: 340:3 error: expected one of `.`, `;`, `}`, or an operator, found `fn`
rust-install/src/mock/clitools.rs:340 fn build_mock_cargo_installer(version: &str, version_hash: &str) -> MockInstallerBuilder {
                                      ^~
rust-install/src/mock/clitools.rs:73:5: 73:29 error: unresolved name `create_custom_toolchains` [E0425]
rust-install/src/mock/clitools.rs:73     create_custom_toolchains(config.customdir.path());
                                         ^~~~~~~~~~~~~~~~~~~~~~~~
rust-install/src/mock/clitools.rs:73:5: 73:29 help: run `rustc --explain E0425` to see a detailed explanation
rust-install/src/mock/clitools.rs:230:17: 230:43 error: unresolved name `build_mock_cargo_installer` [E0425]
rust-install/src/mock/clitools.rs:230     let cargo = build_mock_cargo_installer(version, version_hash);
                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~
rust-install/src/mock/clitools.rs:230:17: 230:43 help: run `rustc --explain E0425` to see a detailed explanation
rust-install/src/mock/clitools.rs:231:21: 231:50 error: unresolved name `build_mock_rust_doc_installer` [E0425]
rust-install/src/mock/clitools.rs:231     let rust_docs = build_mock_rust_doc_installer(channel);
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rust-install/src/mock/clitools.rs:231:21: 231:50 help: run `rustc --explain E0425` to see a detailed explanation
rust-install/src/mock/clitools.rs:232:16: 232:40 error: unresolved name `build_combined_installer` [E0425]
rust-install/src/mock/clitools.rs:232     let rust = build_combined_installer(&[&std, &rustc, &cargo, &rust_docs]);
                                                     ^~~~~~~~~~~~~~~~~~~~~~~~
rust-install/src/mock/clitools.rs:232:16: 232:40 help: run `rustc --explain E0425` to see a detailed explanation
rust-install/src/mock/clitools.rs:335:27: 335:35 error: unresolved name `mock_bin` [E0425]
rust-install/src/mock/clitools.rs:335              vec![(rustc, mock_bin("rustc", version, version_hash))]
                                                                ^~~~~~~~
rust-install/src/mock/clitools.rs:335:14: 335:69 note: in this expansion of vec! (defined in <std macros>)
rust-install/src/mock/clitools.rs:332:21: 337:6 note: in this expansion of vec! (defined in <std macros>)
rust-install/src/mock/clitools.rs:335:27: 335:35 help: run `rustc --explain E0425` to see a detailed explanation
error: aborting due to 9 previous errors
Could not compile `rust-install`.

To learn more, run the command again with --verbose.
nagisa commented 8 years ago

I blame the fact that we’re trying to recover from errors too hard. At the very least we should not continue into resolve after parse failures, should we? That will never work out.

petrochenkov commented 8 years ago

This is a result of https://github.com/rust-lang/rust/pull/31555 and/or https://github.com/rust-lang/rust/pull/31065, so the post-parsing error messages are kind of expected, but still mostly harmful.

nikomatsakis commented 8 years ago

I think continuing after parse errors can work well, but perhaps we should disable this sort of recovery on stable channels until we are better able to handle it. Another common failure I see is lifetime errors due to random type-checking failures -- I'm not exactly sure what causes those, but I'm sure you all have seen them.

nikomatsakis commented 8 years ago

We could recover "opt in" with -Z continue or something, but I'd like us to all experience the errors better so we can work better on heuristics and improvements...

alexcrichton commented 8 years ago

Although not related to parse errors, I've found a very similar bug related to worse error messages in the case of a resolve failure: https://github.com/rust-lang/rust/issues/31997

nikomatsakis commented 8 years ago

We probably want some kind of meta issue here, ultimately -- but my feeling is that we should continue to work towards better support for generating more errors, but we should not jump the gun and inflict these things on the "waiting public" until they are ready.

cc @rust-lang/compiler @rust-lang/tools

alexcrichton commented 8 years ago

I personally like the idea of perhaps a -Z flag to change the behavior here in terms of not letting this propagate to stable. I'm not a huge fan of using a flag to test our error messages as the likelihood of anyone using it seems very small, however.

nikomatsakis commented 8 years ago

I personally like the idea of perhaps a -Z flag to change the behavior here in terms of not letting this propagate to stable.

The main reason I proposed having the more advanced errors enabled by default (not behind a flag) on nightly was to ensure that we actually exercise the code, find ICEs, and so forth...

nikomatsakis commented 8 years ago

We discussed in the @rust-lang/core meeting today. General feeling was that we should make surgical changes to try and stop issuing so many follow-on messages, and then revisit later when we think we can do better about being more targeted in these error reports to things we think are likely real problems. We decided having nightly diverge from beta behavior was a bad idea.

nikomatsakis commented 8 years ago

Does anyone have a kind of standalone example of this?

brson commented 8 years ago

I spent a few minutes trying to make one and failed.

nikomatsakis commented 8 years ago

Nonetheless it seems clear that the recovery in https://github.com/rust-lang/rust/pull/31555 is at fault. So maybe we can modify the logic there -- but it'd be great to have some kind of test, so that we have some idea when we can turn that recovery back on. I'll try to experiment today. I plan to fix the problems in #31997 first, since I have a smaller testcase there.

brson commented 8 years ago

Here's a single-file test case that exhibits horrible spew.

nikomatsakis commented 8 years ago

For the record, this is the output I see from @brson's test case: https://gist.github.com/nikomatsakis/b5d93abd92fb30487753