ICEs should always print the top of the query stack.

[eddyb]

E.g. for #70942, instead of:

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

We could print something more like

error: internal compiler error: unexpected panic
    while const-evaluating `main::{{constant}}#0`
    ... (full query stack hidden, use `RUST_BACKTRACE=1` to enable)

note: the compiler unexpectedly panicked. this is a bug.

(not super sure about the RUST_BACKTRACE=1 suggestion message)

This (and the query stack more generally) should allow quicker diagnosis, and there should be no cost to just having this on by default for ICEs.

It could also be useful to users, as they could use it to figure out roughly where in their code they're triggering the ICE (similar to the Span from a span_bug!), to work around the ICE in the meanwhile or know which error to fix (in case it's a side-effect of an error) to get rid of the ICE.

cc @rust-lang/compiler @rust-lang/wg-diagnostics

[eddyb]

Nominating for a vibe check (this should be easy to implement if we agree we want it).

[Zoxc]

Getting / printing the query stack may panic, so I'd prefer it to be done after printing the backtrace, but I guess that would be hidden by default anyway.

[bjorn3]

I think always enabling backtraces for rustc would be useful, as it ensures that all bug reports contain a backtrace. Even when the issue is not reproducable.

[pnkfelix]

discussed in yesterday's T-compiler meeting. There was general support for adding information about the query context to the ICE output.

when we had a second ad hoc triage meeting today, @eddyb added that we had issues in the past with trying to always emit backtraces (but it is possible that they have been fixed).

• To be honest, I wasn't clear on whether @bjorn3 's suggest was to print out (unconditionally) the call stack backtrace, or the query stack.

In any case, it seems like we can immediately work on adding a printout for the top of the query stack, and leave the question of whether to add more info beyond that to a later discussion.

[eddyb]

we had issues in the past with trying to always emit backtrace

Not "always emit", but rather issues when enabling it via RUST_BACKTRACE=1, e.g. taking minute to compute the backtrace, or strange crashes. We may have updated/fixed libbacktrace since, though.

If we ever made the default to emit the backtrace, RUST_BACKTRACE=0 could be used as an opt-out.

[bjorn3]

To be honest, I wasn't clear on whether @bjorn3 's suggest was to print out (unconditionally) the call stack backtrace, or the query stack.

Both

[eddyb]

Something I completely forgot is that the query stack sometimes even has Spans (originally added for query cycle errors), so we could also try to show those, which could give even more precision to what part of the user code triggered the query that ICE'd.

[oli-obk]

For anyone looking to implement this. The relevant work needs to be done in

rust/src/librustc_driver/lib.rs

Line 1156 in 86c0b85

pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {

. The query stack is reported in

rust/src/librustc_driver/lib.rs

Line 1207 in 86c0b85

TyCtxt::try_print_query_stack(&handler);

, so I think we need another function for printing just the top of the query stack.

[georgio]

I'm trying to work on this. Is there some sort of template we are targeting?

My proposed solution is adding something like
pub fn try_print_query_stack_head(handler: &Handler) {...}
somewhere around here

rust/src/librustc_middle/ty/query/plumbing.rs

Line 127 in 86c0b85

pub fn try_print_query_stack(handler: &Handler) {

[oli-obk]

You could also add a num_frames: Option<usize> argument to try_print_query_stack that makes it stop after that number of elements (if any). This way you can just adjust a few invocation sites instead of having to build a new function that does almost the same.

[georgio]

Great idea, thanks. I assumed that we might break a lot of other functions by changing the signature of try_print_query_stack, turns out it's only being called twice. Will confirm after running tests.

[Dylan-DPC-zz]

@georgio any updates on this?

[hosseind75]

guys I am working on it and as @oli-obk recommendation, I want to add num_frames argument to try_print_query_stack function to break while loop when loop counter is equal to num_frames, but I am not sure that is it correct or not?
and also I am not sure that we should ask the user to set an environment variable that determines how much of the query stack should we print or just pass a strict value to this function?

[oli-obk]

We can just pass a strict value. This only happens when the compiler panics and is just a single line of information. I don't think this is something we really need to make configurable.

[hosseind75]

ok thanks, I will pass f.e 2 to just show two first queries

[GroteGnoom]

This should be closed because of
#77493