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

Some fixes for error recovery in the compiler #32435

Merged
merged 3 commits into from Mar 26, 2016

Conversation

Projects
None yet
7 participants
@nrc
Copy link
Member

nrc commented Mar 22, 2016

No description provided.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Mar 22, 2016

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@sanxiyn

This comment has been minimized.

Copy link
Member

sanxiyn commented Mar 23, 2016

Travis failed.

failures:
    [parse-fail] parse-fail/issue-10636-2.rs
    [parse-fail] parse-fail/issue-2354-1.rs
    [parse-fail] parse-fail/macro-mismatched-delim-paren-brace.rs

@nrc nrc force-pushed the nrc:fix-err-recover branch from bf10f4e to 167ef75 Mar 23, 2016

@nrc nrc force-pushed the nrc:fix-err-recover branch from 167ef75 to 180d6b5 Mar 24, 2016

// Expand to cover the entire delimited token tree
let span = Span { hi: close_span.hi, ..pre_span };

match self.token {

This comment has been minimized.

@nikomatsakis

nikomatsakis Mar 24, 2016

Contributor

is this code specific to macros? if so, I don't see any tests below that would exercise it.

// Test that we do some basic error correcton in the tokeniser.

fn main() {
foo(bar(; //~ NOTE: unclosed delimiter

This comment has been minimized.

@nikomatsakis

nikomatsakis Mar 24, 2016

Contributor

I am a bit surprised to see so many errors here. I feel like after the ; we should "reset" and not consider the } to be an error.

@@ -14,4 +14,4 @@ fn main() {
foo! (
bar, "baz", 1, 2.0
} //~ ERROR incorrect close delimiter
}
} //~ ERROR unexpected close delimiter: `}`

This comment has been minimized.

@nikomatsakis

nikomatsakis Mar 24, 2016

Contributor

I guess this does exercise the macro logic, my mistake.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 24, 2016

I realize now this was targeting primarily #31804 -- does it also aim to address #31994 ?

if let Some(&(_, sp)) = self.open_braces.last() {
err.span_note(sp, "unclosed delimiter");
};
err.emit();

This comment has been minimized.

@nikomatsakis

nikomatsakis Mar 24, 2016

Contributor

I feel like once we emit one unclosed delimeter error, we should try to avoid reporting any further ones, since all further delimeters are pretty suspect.

// E.g., we try to recover from:
// fn foo() {
// bar(baz(
// } // Incorrect delimiter but matches the earlier `{`

This comment has been minimized.

@nikomatsakis

nikomatsakis Mar 24, 2016

Contributor

should this example be bar!(baz(? like, will parse_token_tree be invoked outside of a macro invocation?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 25, 2016

@eddyb tells me that we parse all things as token trees first -- I didn't realize that! Disregard my comments, will re-review.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 25, 2016

Just for reference, the output from @brson's test in #31994 is https://gist.github.com/nikomatsakis/e6058b1e264e28d4885c -- still quite a lot. WRONG!

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 25, 2016

Oops, totally ran that with the wrong branch. The output is:

/home/nmatsakis/tmp/error-spew.rs:843:9: 843:10 error: incorrect close delimiter: `}`
/home/nmatsakis/tmp/error-spew.rs:843         } else {
                                              ^
/home/nmatsakis/tmp/error-spew.rs:841:21: 841:22 note: unclosed delimiter
/home/nmatsakis/tmp/error-spew.rs:841             callback(path.as_ref(); // MISSING )
                                                          ^
/home/nmatsakis/tmp/error-spew.rs:841:35: 841:36 error: expected one of `,`, `.`, `?`, or an operator, found `;`
/home/nmatsakis/tmp/error-spew.rs:841             callback(path.as_ref(); // MISSING )
                                                                        ^
/home/nmatsakis/tmp/error-spew.rs:842:13: 842:15 error: expected one of `.`, `;`, `?`, `}`, or an operator, found `fs`
/home/nmatsakis/tmp/error-spew.rs:842             fs::create_dir_all(path.as_ref()).map(|()| true)
                                                  ^~
/home/nmatsakis/tmp/error-spew.rs:841:13: 841:36 error: mismatched types:
 expected `core::result::Result<bool, std::io::error::Error>`,
    found `()`
(expected enum `core::result::Result`,
    found ()) [E0308]
/home/nmatsakis/tmp/error-spew.rs:841             callback(path.as_ref(); // MISSING )
                                                  ^~~~~~~~~~~~~~~~~~~~~~~
/home/nmatsakis/tmp/error-spew.rs:841:13: 841:36 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 25, 2016

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 25, 2016

📌 Commit 180d6b5 has been approved by nikomatsakis

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 25, 2016

Seems good to me. :)

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 26, 2016

Rollup merge of rust-lang#32435 - nrc:fix-err-recover, r=nikomatsakis
Some fixes for error recovery in the compiler

bors added a commit that referenced this pull request Mar 26, 2016

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 26, 2016

Rollup merge of rust-lang#32435 - nrc:fix-err-recover, r=nikomatsakis
Some fixes for error recovery in the compiler

bors added a commit that referenced this pull request Mar 26, 2016

Auto merge of #32496 - Manishearth:rollup, r=Manishearth
Rollup of 11 pull requests

- Successful merges: #32131, #32199, #32257, #32325, #32435, #32447, #32448, #32456, #32469, #32476, #32482
- Failed merges: #32240

bors added a commit that referenced this pull request Mar 26, 2016

Auto merge of #32496 - Manishearth:rollup, r=Manishearth
Rollup of 11 pull requests

- Successful merges: #32131, #32199, #32257, #32325, #32435, #32447, #32448, #32456, #32469, #32476, #32482
- Failed merges: #32240

bors added a commit that referenced this pull request Mar 26, 2016

Auto merge of #32496 - Manishearth:rollup, r=Manishearth
Rollup of 11 pull requests

- Successful merges: #32131, #32199, #32257, #32325, #32435, #32447, #32448, #32456, #32469, #32476, #32482
- Failed merges: #32240

bors added a commit that referenced this pull request Mar 26, 2016

Auto merge of #32496 - Manishearth:rollup, r=Manishearth
Rollup of 11 pull requests

- Successful merges: #32131, #32199, #32257, #32325, #32435, #32447, #32448, #32456, #32469, #32476, #32482
- Failed merges: #32240

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 26, 2016

Rollup merge of rust-lang#32435 - nrc:fix-err-recover, r=nikomatsakis
Some fixes for error recovery in the compiler

bors added a commit that referenced this pull request Mar 26, 2016

Auto merge of #32496 - Manishearth:rollup, r=Manishearth
Rollup of 11 pull requests

- Successful merges: #32131, #32199, #32257, #32325, #32435, #32447, #32448, #32456, #32469, #32476, #32482
- Failed merges: #32240

@bors bors merged commit 180d6b5 into rust-lang:master Mar 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 31, 2016

discussion leads to conclusion that we will land
only first commit from #32435 (3ee841c)
since #32494 will cover the spew otherwise
for beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.