Fix cycle error only occurring with -Zdump-mir

[oli-obk]

fixes #134205

During mir dumping, we evaluate static items to render their allocations. If a static item refers to itself, its own MIR will have a reference to itself, so during mir dumping we end up evaluating the static again, causing us to try to build MIR again (mir dumping happens during MIR building).

Thus I disabled evaluation of statics during MIR dumps in case the MIR body isn't far enough along yet to be able to be guaranteed cycle free.

[rustbot]

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

Thanks, the impl changes look good to me, however the MIR test differences I don't know well -- it feels like some mir-opt tests actually do want the alloc contents to be in the dump (but that -Zdump-mir as-is might be insufficient?).

[jieyouxu]

Suggestion: could you slightly elaborate what this test intends to check (i.e. just the PR description)?

[oli-obk]

it feels like some mir-opt tests actually do want the alloc contents to be in the dump (but that -Zdump-mir as-is might be insufficient?).

yea. I think we can make some (most) of these work again by just avoiding the evaluation if we're in a const context, I'll give it a shot

[oli-obk]

@rustbot ready

r? compiler

Yea, the fix is now much more targeted and I couldn't come up with a situation where it could cause cycles through a non-const function

[compiler-errors]

Doesn't this still cycle when building the .after.mir after optimized mir is done building? Can you test this with -Zdump-mir locally?

[compiler-errors]

The comment that I left in the issue also noted that the last dump of MIR needs to happen outside of the query itself. As long as we are dumping MIR from within a query, this has the potential to be cyclical?

[oli-obk]

Hypothetically yes, but to get the optimized MIR from the const eval queries we'd need to have something that checks the optimized MIR for body-leaky information like "does this have panics after optimizations?". I see no other way we'd end up getting the optimized MIR from const eval without that being a bug, so invoking const eval is fine from optimized MIR (as it only uses mir_for_ctfe).

It would be more consistent to just never dump the statics as I had it previously, as then the MIR of a const fn and a normal fn would be the same if the constness was the only difference.

Doesn't this still cycle when building the .after.mir after optimized mir is done building? Can you test this with -Zdump-mir locally?

no, I get

// MIR for `thing` after runtime-optimized

fn thing() -> &Thing {
    let mut _0: &Thing;

    bb0: {
        _0 = const {alloc3: &Thing};
        return;
    }
}

alloc3 (static: MUTUALLY_RECURSIVE, size: 8, align: 8) {
    ╾─────alloc3<imm>─────╼                         │ ╾──────╼
}

in dump_mir_cycle.thing.006-000.runtime-optimized.after.mir

[compiler-errors]

@bors r+ rollup

[bors]

📌 Commit 15c01eb has been approved by compiler-errors

It is now in the queue for this repository.