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

Return nonzero exit code if there are errors at a stop point #22117

Merged
merged 1 commit into from
Apr 20, 2015

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 9, 2015

At the moment, when compilation is stopped at a stop point (like -Z parse-only), rustc does not return an nonzero exit code even if there are errors (expect fatal ones, that cause it to panic immediately). As an example, compiling src/test/compile-fail/doc-before-semi.rs with -Z parse-only raises an error, but exists with 0.

Note that I could not use sess.abort_if_errors() in the macro, because sess is passed by value and move at some point.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@nikomatsakis
Copy link
Contributor

Can you add some kind of test for this?

@brson
Copy link
Contributor

brson commented Feb 13, 2015

@nikomatsakis #22118 contains a bunch of test cases for this. Since those are the intended use it may be hard to dig up other relevant test cases.

@fhahn
Copy link
Contributor Author

fhahn commented Feb 13, 2015

@nikomatsakis @brson yes, I would like to add the -Z parse-only flag to all parse-fail tests after #22118 lands to test this patch. Also I am going to check if there are test cases that fail at other stop points with this patch.

@fhahn
Copy link
Contributor Author

fhahn commented Feb 17, 2015

@brson I have updated the patch to use a straight forward solution to invoke the diagnostics handler. I added a session argument to the controller_entry_point macro to pass the current valid session. This way we can handle the case were session got moved.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 19, 2015
After making `rustc` fail on errors at a stop point, like `-Z parse-only`, in rust-lang#22117, the files in this PR also fail during the parse stage and should be moved as well. Sorry for spliting this move up in two PRs.
@fhahn
Copy link
Contributor Author

fhahn commented Feb 22, 2015

@brson what do you think about the new approach? Is it more reasonable?

@pczarn
Copy link
Contributor

pczarn commented Mar 7, 2015

Needs rebase

@fhahn fhahn force-pushed the fail-on-errors branch 2 times, most recently from be1ba0d to f29b4d7 Compare March 7, 2015 19:56
@fhahn
Copy link
Contributor Author

fhahn commented Mar 7, 2015

Thanks, I have rebased the PR.

@bors
Copy link
Contributor

bors commented Mar 17, 2015

☔ The latest upstream changes (presumably #23331) made this pull request unmergeable. Please resolve the merge conflicts.

@fhahn
Copy link
Contributor Author

fhahn commented Mar 20, 2015

I have resolved the merge conflicts.

@bors
Copy link
Contributor

bors commented Mar 24, 2015

☔ The latest upstream changes (presumably #23654) made this pull request unmergeable. Please resolve the merge conflicts.

@fhahn
Copy link
Contributor Author

fhahn commented Mar 26, 2015

rebased this PR again

@bors
Copy link
Contributor

bors commented Apr 3, 2015

☔ The latest upstream changes (presumably #23930) made this pull request unmergeable. Please resolve the merge conflicts.

@@ -8,4 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z parse-only
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be globally added to parse-fail rather than in each test file?

@nikomatsakis
Copy link
Contributor

r+

@fhahn I just realized you've been dutifully rebasing this PR and I didn't notice at all. I apologize for that. In any case, I think it seems clearly good!

@nikomatsakis
Copy link
Contributor

(Unfortunately, it does need to be rebased again...)

@fhahn
Copy link
Contributor Author

fhahn commented Apr 18, 2015

@nikomatsakis no worries, all rebases were very minimal & straightforward

@pnkfelix
Copy link
Member

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Apr 20, 2015

📌 Commit 2a24e97 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Apr 20, 2015

⌛ Testing commit 2a24e97 with merge be9a72b...

bors added a commit that referenced this pull request Apr 20, 2015
At the moment, when compilation is stopped at a stop point (like `-Z parse-only`), `rustc` does not return an nonzero exit code even if there are errors (expect fatal ones, that cause it to panic immediately). As an example, compiling `src/test/compile-fail/doc-before-semi.rs` with `-Z parse-only` raises an error, but exists with 0.

Note that I could not use `sess.abort_if_errors()` in the macro, because `sess` is passed by value and move at some point.
@bors bors merged commit 2a24e97 into rust-lang:master Apr 20, 2015
@fhahn fhahn deleted the fail-on-errors branch April 20, 2015 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants