Constant operands are dropped in an unusual order

[tmiasko]

For example:

struct Print(&'static str);

impl Drop for Print {
    fn drop(&mut self) {
        println!("{}", self.0);
    }
}

const A: Print = Print("a");
const B: Print = Print("b");

fn main() {
    loop {
        std::mem::forget(({A}, B, Print("c"), break));
    }
}

$ rustc -Aunreachable_code x.rs && ./x
c
a

[RalfJung]

To be clear, the 'evaluation' of the const items is not observable, but the drop order in the MIR of main is (which doesn't have anything to do with the order in which the operands are compile-time evaluated). I will rename the issue to mention dropping.

[RalfJung]

FWIW I would have expected the order to be consistent with calling a function that returns the Print, but it is not:

struct Print(&'static str);

impl Drop for Print {
    fn drop(&mut self) {
        println!("{}", self.0);
    }
}

fn a() -> Print { Print("a") }
fn b() -> Print { Print("b") }

fn main() {
    loop {
        std::mem::forget(({a()}, b(), Print("c"), break));
    }
}

this prints

c
b
a

Which is right-to-left, so "last variable dropped first", which I guess makes sense.

Cc @rust-lang/wg-const-eval, though this is more about MIR generation that const evaluation (but not sure whom to ping for that).

[oli-obk]

hmm... this is a weird situation, but I agree that the behaviour is wrong and should be like with the function calls.

The main problem is that the tuple's MIR would be (Operand::Copy, Operand::Const(B), Operand::Copy, Operand::Copy), but then that tuple construction gets optimized out, along with the Operand::Const(B). I guess the only way to fix this is to copy all operand into locals before doing the tuple construction (or a function call or.... basically we can stop using operands and just use locals :( ).

[RalfJung]

that tuple construction gets optimized out, along with the Operand::Const(B)

So there is some pre-drop-elaboration pass which assumes that Operand::Const has no side-effect? That sounds like a wrong assumption.

[oli-obk]

The general assumption is that Operand has no side effects. I don't see how we can uphold this as long as constants are directly usable as operands.

On the other hand, a lot of MIR and codegen would get more complex if we didn't have constants in operands as we'd end up with lots and lots of short lived locals

[RalfJung]

The general assumption is that Operand has no side effects.

And this is true once we are done generating the MIR.

But before drop elaboration, MIR is not done building yet. Things can have 'side-effects' in the sense of affecting how the final (post-drop-elab) MIR will look. So many of the assumptions one might make about MIR simply do not hold.

[oli-obk]

Yea that makes sense.

So there are four ways forward:

• do nothing, keep inconsistent behaviour
• make mir building more complex by having it understand that statements and terminators with multiple operands may not reach all operands if one of the operands is diverging. Horrible plan...
• make mir building never emit const operands except for some blessed rvalues like Use. This creates a temporary for every constant used. OK, need to optimize via some const/copy prop, but probably just for optimizations, not for analysis. May cause a perf regression, probably just in stress tests tho
• add a new statement that is inserted where the temp assignment would be inserted in the previous pass. This statement solely serves to let the constant's drop get generated. Throw away all such statements after analysis. Probably OK, but seems fragile

[tmiasko]

There might be a room for some middle ground solution, where temporaries are introduced only if a type actually needs to be dropped. Constants that need to be dropped are rather uncommon.

[RalfJung]

make mir building more complex by having it understand that statements and terminators with multiple operands may not reach all operands if one of the operands is diverging. Horrible plan...

That sounds like the proper solution to me? In fact this is required anyway in case any of these operands is a function call that might not return. So I am probably misunderstanding what you mean by "operand" here -- are you using this as a surface Rust term or as a MIR term? In MIR there are no diverging operands though... confused

[oli-obk]

That sounds like the proper solution to me?

it's the same solution as the locals solution, just requiring us to have imaginary locals and figuring it all out in separate logic.

MIR can have diverging operands by having an uninhabited local and using that as in an Operand::Copy. This is what happens when you create an aggregate or do a function call where one of the arguments is uninhabited. That aggregate or call may be unreachable, but unfortunately in the present MIR builder we'll still put the Operand::Const directly into the unreachable aggregate/call so it is never built, even if the surface syntax suggests it should be.

[RalfJung]

Okay so it's not the operand that is diverging, but the code that would initialize the local diverges.