panic!() with formatting arguments sometimes accepted in const

[m-ou-se]

This issue is a combination of three things:

1. fn panic_fmt is marked with #[rustc_do_not_const_check] and fully relies on format_args!() with any arguments to not be accepted in const.

2. Inlining/flattening of format_args!() accidentally exposed as stable through const #139136 - I accidentally marked this function as const, meaning that flattened format_args are accepted in const. This went unnoticed until two weeks ago. Edit: This is actually caused by [generic_assert] Constify methods used by the formatting system #135139

3. We have a special case for panic!("{}", $expr) that we added at some point to make panic!("{}", my_string) work in const, as a workaround for panic!(my_string) which no longer works as of Rust 2021.

The result is pretty terrible:

const A1: () = if false { panic!("{}", "a") };
const A2: () = if false { panic!("{}", {"a"}) };
const A3: () = if false { panic!("{}", 1) }; // error!
const A4: () = if false { panic!("{}", {1}) }; // error!
const B1: () = if false { panic!(" {}", "a") };
const B2: () = if false { panic!(" {}", {"a"}) }; // error!
const B3: () = if false { panic!(" {}", 1) };
const B4: () = if false { panic!(" {}", {1}) }; // error!

Only the four marked lines error. The others compile fine. 🙃

(The A constants use the "{}" special case for panic!() and end up invoking panic_display for which we have a special const eval check that the argument is a &str. The B constants end up invoking panic_fmt (which does not have any const eval checks) and format_args!().)

[m-ou-se]

@rust-lang/wg-const-eval Continuing the discussion we started on #139135:

What do we do about this?

We could 'fix' the situation by not accepting any formatting with arguments in const panic, but that would mean a breaking change for B1 and B3 (and A1 and A2, if we don't we keep the special "{}" case). I'm not sure if that's acceptable.

If we move forward to make format_args!() always acceptable in const, then still trying to replicate the current situaiton (to not break anything but also not allow anything new) is going to be a mess.

But if we allow more, we end up with situations where we panic in const where we cannot format the message in const.

[RalfJung]

Ideally, panic! in const would only be accepted if the argument is literally a string literal and nothing else. I don't know the various layers of desugaring here well enough to translate that into a recipe of what needs to be checked where in the compiler.

that would mean a breaking change for B1 and B3 (and A1 and A2, if we don't we keep the special "{}" case). I'm not sure if that's acceptable.

I'd suggest cratering that.

[m-ou-se]

I'm no expert on const eval, but it seems the best way out might be to accept all panic!() with any formatting in const, and handle the situation where we cannot format the message in diagnostics. E.g. just print a generic "panic during evaluation" message without the formatted panic message in those cases.

Then if we can add support for more formatting in const in the future, we can always add more cases where we add the message, which is now just a diagnostics quality issue rather than a breaking/stable change.

[RalfJung]

I'm no expert on const eval, but it seems the best way out might be to accept all panic!() with any formatting in const, and handle the situation where we cannot format the message in diagnostics. E.g. just print a generic "panic during evaluation" message without the formatted panic message in those cases.

I guess that would be the other option -- though we should communicate somehow why we ignored the panic message that the user wrote. If t-libs-api can live with const panics having degraded panic messages in a way that is only apparent at "runtime" (i.e., const-eval time, not writing-the-code time), that's fine for me.

[m-ou-se]

Ideally, panic! in const would only be accepted if the argument is literally a string literal and nothing else. I don't know the various layers of desugaring here well enough to translate that into a recipe of what needs to be checked where in the compiler.

I think the main problem there is that panic!("hello") and panic!("hello {name}") look identical to the panic!() macro. It cannot distinguish those cases in the macro_rules. Both have to expand to the same panic_fmt(format_args!(..)) expression.

So if we make all format_args!(..) acceptable in const, then how is panic_fmt going to refuse some of them in const when it isn't evaluated?

We could add a special format_args_not_const_when_this_has_any_arguments!(), I suppose.

[RalfJung]

FWIW there have been requests to support formatting numbers in const panics, and the corresponding code would likely actually work in const-eval. So we may not want to tie us to "no arguments", and instead allow arguments with a given allowlist of types. That would involve some crude hacks though to get the heap allocations inside the formatting machinery to work, so the idea of making this best-effort rather than a stable guarantee is enticing.

[m-ou-se]

When const evaluating a panic, we know we're going to error out anyway, so can we then disable some const checks to allow 'const' evaluating more formatting code? I think @eddyb suggested that a few years ago (something with -Z unleash-the-miri-inside-of-you), but I don't know how feasible/reasonable that is with how things work today.

[RalfJung]

When const evaluating a panic, we know we're going to error out anyway, so can we then disable some const checks to allow 'const' evaluating more formatting code?

We raced saying the same thing. :)

One hard constraint I'd like to impose here though is that we do not do this for any user-defined code. Only code we fully control should be run in this "more permissive" const-eval panic formatting mode.

[m-ou-se]

One hard constraint I'd like to impose here though is that we do not do this for any user-defined code.

Just curious: why? It's a proper sandbox, right?

[oli-obk]

The previous attempt at this was #90488 (the PR's state was to allow all types to be formatted, and to do dummy formatting for the others, it was closed due to inactivity after being asked to limit it to types we know will be implementable as const Debug or const Display in the future)

[RalfJung]

Just curious: why? It's a proper sandbox, right?

I don't want to risk exposing the unstable parts of the interpreter to whatever creative things people come up with. That's just too many unknowns.