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

Print out the output line that fails json parsing #33392

Closed
wants to merge 3 commits into from

Conversation

sophiajt
Copy link
Contributor

@sophiajt sophiajt commented May 4, 2016

To help with debugging issues with some of the unit tests on various platforms, go ahead and print out the output line that fails a json parse.

Should help with tracking down #33390.

cc @alexcrichton

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@alexcrichton
Copy link
Member

Perhaps we could just print out all of the output of the compiler when the compiler finishes? That way we can also see things like the exit code and the entirety of stderr (which may have information like about a panic)

@@ -72,7 +72,8 @@ fn parse_line(file_name: &str, line: &str) -> Vec<Error> {
expected_errors
}
Err(error) => {
panic!("failed to decode compiler output as json: `{}`", error);
panic!("failed to decode compiler output as json: `{}` when parsing: {}", error,
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I think I agree with @alexcrichton, seems like we should pass in output and -- in the event of error -- print both the entire output and the failed line. It might be better still to pass in more context (such as stderr etc) -- basically the ProcRes struct, I think it's called? Then we can just call fatal_proc_res which dumps a lot of context iirc.

Copy link
Contributor

Choose a reason for hiding this comment

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

(when these errors crop up on a bot, the more context the better)

@sophiajt
Copy link
Contributor Author

sophiajt commented May 4, 2016

Sure - sounds good. Something like that?

@alexcrichton
Copy link
Member

I think that this may either want to println! the proc_res after the compiler finishes or perhaps thread through the ProcRes returned to eventually call fatal_proc_rec to provide a nicer error message with contextual information. Specifically, this call may want to thread through the entire ProcRes?

@sophiajt
Copy link
Contributor Author

sophiajt commented May 4, 2016

@alexcrichton - updated PR to call fatal_proc_rec instead.

@alexcrichton
Copy link
Member

@bors: r+ 26aed43

@bors
Copy link
Contributor

bors commented May 7, 2016

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

@bors
Copy link
Contributor

bors commented May 17, 2016

🔒 Merge conflict

@sophiajt sophiajt closed this May 17, 2016
bors added a commit that referenced this pull request May 19, 2016
Fix for old school error issues, improvements to new school

This PR:
* Fixes some old school error issues, specifically #33559, #33543, #33366
* Improves wording borrowck errors with match patterns
* De-emphasize multi-line spans, so we don't color the single source character when we're trying to say "span starts here"
* Rollup of #33392 (which should help fix #33390)

r? @nikomatsakis
@sophiajt sophiajt deleted the output_bad_json branch May 19, 2016 12:18
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.

5 participants