fix ice reporting in lintcheck #12439
Conversation
There was a problem hiding this comment.
Meow meow 😸
Hmmm... I think you're over-complicating things.
ClippyWarning::new has an argument of type Diagnostic, a field of this type describes if the diagnostic is produced by an ICE or just a normal warning (the field level).
You could maintain the is_ice field, and just fix the behaviour without manually parsing the stderr and implementing a new enum and struct :)
On line 668 of the original commit, it seems like some filtering is trying to be applied, look for what's wrong and work your way up from there.
|
The big issue is that there isn't any way to produce proper This is also compounded by the fact that For a bit more detailed insight as to why this PR does what it does, I put my findings into a comment on the original issue: #12185 (comment). However, if it would be best to take a new approach (such as making diagnostics no longer |
|
Okis, I'll just focus on logical errors and syntax then, let's hope that nobody has to look at this in the future ❤️ |
|
I agree the fix itself isn't the most elegant but sadly I couldn't really think of a better approach given the dilemma of a lack of proper JSON for ICEs 🙃 if such a feature ever comes to exist this can probably be revisited |
|
the missing json for ices is tracked here rust-lang/rust#114302 |
lintcheck/src/main.rs
Outdated
| for line in lines { | ||
| if line.contains("panicked at") { | ||
| reading_message = true; | ||
| message_lines.push(line.to_owned()); | ||
| } else if line.contains("stack backtrace:") { | ||
| reading_message = false; | ||
| reading_backtrace = true; | ||
| backtrace_lines.push(line.to_owned()); | ||
| } else if line.contains("the compiler unexpectedly panicked") { |
There was a problem hiding this comment.
you can probably steal some from here: :)
https://github.com/matthiaskrgr/icemaker/blob/master/src/main.rs#L1810
|
What you can also do is to check the exit code of clippy (should be |
lintcheck/src/main.rs
Outdated
|
|
||
| for line in lines { | ||
| if line.contains("panicked at") { | ||
| reading_message = true; | ||
| message_lines.push(line.to_owned()); | ||
| } else if line.contains("stack backtrace:") { | ||
| reading_message = false; | ||
| reading_backtrace = true; | ||
| backtrace_lines.push(line.to_owned()); | ||
| } else if line.contains("the compiler unexpectedly panicked") { | ||
| reading_backtrace = false; | ||
| } else if reading_message { | ||
| message_lines.push(line.to_owned()); | ||
| } else if reading_backtrace { | ||
| backtrace_lines.push(line.to_owned()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I remember that I had problems at some point where rustc would generate so much output that I would essentially run out of ram with stderr/stdout.contains() and had to switch over iterating over the output line by line.
there are also different kind of ices, like assertion failures or perhaps "panic!()" inside clippys and rustcs delayed bugs
#![allow(internal_features)]
#![feature(rustc_attrs)]
#[rustc_error(delayed_bug_from_inside_query)]
fn main() {}
I don't think this will be able to parse all of them very well
|
@Jacherr I assumed you were going to apply Matthias' suggestions, but it seems like this has been a little bit paused 😅. Could you please apply them ❤️ |
|
Hi, apologies for the delays, been fairly busy the last couple weeks. I'll go through and take a look now - @matthiaskrgr I suppose the easiest solution is to simply provide a raw |
|
@bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #12185
This PR fixes the lack of reported ICEs within lintcheck by modifying the way in which data is collected from each crate being linted.
Instead of lintcheck only reading
stdoutfor warnings, it now also readsstderrfor any potential ICE (although admittedly, it is not the cleanest method of doing so). If it is detected, it parses the ICE into its message and backtrace separately, and then adds them to the list of outputs via clippy.Once all outputs are collected, the formatter then proceeds to generate the file as normal.
Note that this PR also has a couple of side effects:
stderrwill be sent to the console;ClippyWarningbeing the primary struct for everything reported, there is nowClippyCheckOutput, an enum which splits the outputs into warnings and ICEs.changelog: none