Make const panic!("..") work in Rust 2021.

[m-ou-se]

During const eval, this replaces calls to core::panicking::panic_fmt and std::panicking::being_panic_fmt with a call to a new const fn: core::panicking::const_panic_fmt. That function uses fmt::Arguments::as_str() to get the str and calls panic_str with that instead.

panic!() invocations with formatting arguments are still not accepted, as the creation of such a fmt::Arguments cannot be done in constant functions right now.

r? @RalfJung

[m-ou-se]

A small problem with this is that the panic is now reported inside core instead of at the point where the panic macro is invoked:

error[E0080]: evaluation of constant value failed
   --> /home/mara/rust/library/core/src/panicking.rs:102:9
    |
102 |         panic_str(msg);
    |         ^^^^^^^^^^^^^^
    |         |
    |         the evaluated program panicked at 'hello', src/main.rs:4:5
    |         inside `core::panicking::panic_fmt` at /home/mara/rust/library/core/src/panicking.rs:102:9
    | 
   ::: src/main.rs:4:5
    |
4   |     panic!("hello")
    |     --------------- inside `hey` at /home/mara/rust/library/core/src/panic.rs:38:9
...
7   | const X: u32 = hey();
    |                ----- inside `X` at src/main.rs:7:16

Maybe const eval should use #[track_caller] too.

[m-ou-se]

Hm, looks like that should already work:

rust/compiler/rustc_mir/src/const_eval/machine.rs

Line 44 in ee86f96

let span = self.find_closest_untracked_caller_location();

Trying to figure out what goes wrong now.

[m-ou-se]

Ah, that's just used for the message, which is correct:

the evaluated program panicked at 'hello', src/main.rs:4:5

But maybe it should be used for the span to point the error at too.

[m-ou-se]

Fixed in #87000

Marking this as blocked on that.

[oli-obk]

do we have existing tests that ensure that format_args! cannot be called in const contexts? Just to make sure we don't accidentally allow things due to the various allow_internal_unstable on these macros.

[m-ou-se]

The test is failing now, but will pass once #87000 is merged.

[m-ou-se]

do we have existing tests that ensure that format_args! cannot be called in const contexts? Just to make sure we don't accidentally allow things due to the various allow_internal_unstable on these macros.

This change will indeed allow format_args!("something") in const functions. Not sure how to avoid that other than by adding a new format_args_const macro. I can add a rustc_const_unstable attribute with a new feature name to fmt::Arguments::new*, but I can't only activate that in format_args!() inside panic!() and not in just format_args!() by itself.

[RalfJung]

How does this MR relate to #86830? They seem to try to solve the same problem...?

[m-ou-se]

Oh, didn't see that one yet. Looking now.

[oli-obk]

How does this MR relate to #86830? They seem to try to solve the same problem...?

fixes the same problem, but the solution is more maintainable I believe.

This change will indeed allow format_args!("something") in const functions. Not sure how to avoid that other than by adding a new format_args_const macro. I can add a rustc_const_unstable attribute with a new feature name to fmt::Arguments::new*, but I can't only activate that in format_args!() inside panic!() and not in just format_args!() by itself.

I think it should work by adding rustc_const_unstable to the newly const functions and use a feature gate name other than fmt_internals. Or does allow_internal_unstable not permit this kind of scheme (adding it to panic! only, even if the usage is in format_args!?

[m-ou-se]

Or does allow_internal_unstable not permit this kind of scheme (adding it to panic! only, even if the usage is in format_args!?

Nope :(

[m-ou-se]

Looks like #86830 does the exact same, but in a different way. It also has the same problem of making format_args!("something") const.

[oli-obk]

Or does allow_internal_unstable not permit this kind of scheme (adding it to panic! only, even if the usage is in format_args!?

Nope :(

Oh... and we can't fix that, because any fix would then also allow user-expressions in the macro arguments to use the unstable things. So I guess we'd need @eddyb's solution and have a separate-but-equal forever-unstable const_format_args that panic uses internally.

[m-ou-se]

Oh... and we can't fix that, because any fix would then also allow user-expressions in the macro arguments to use the unstable things. So I guess we'd need @eddyb's solution and have a separate-but-equal forever-unstable const_format_args that panic uses internally.

Yeah. Did that now.

[oli-obk]

So... if my understanding is right, std::panic!(SOME_CONST) does not work anymore in 2021, does your change permit panic!("{}", some_str) to work? If so, that should have a test.

[oli-obk]

oof, our test suite is really subpar. We have zero tests for

const FOO: () = {
    let mut x = [4, 2];
    for i in 0..x.len() {
         x[i] = x[i] + b'0';
    }
    let z = unsafe { std::str::from_utf8_unchecked(&x) };
    panic!("{}", z);
};

[m-ou-se]

So... if my understanding is right, std::panic!(SOME_CONST) does not work anymore in 2021, does your change permit panic!("{}", some_str) to work?

Nope. I want to make a next PR that makes panic!("{}", "literal") work. panic!("{}", some_str_var) will be a step further into the future.

[RalfJung]

So I guess we'd need @eddyb's solution and have a separate-but-equal forever-unstable const_format_args that panic uses internally.

Why that? As I suggested in the other PR, the const checking code could accept format_args! *if it is the argument of a panic!, and reject otherwise.

That seems like a less ugly alternative than a 2nd macro.^^

[m-ou-se]

Or we could just do an FCP to start allowing format_args!("literal") in const context.

[bors]

☔ The latest upstream changes (presumably #86857) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

Just gonna keep repeating this everywhere, our const panic tests are really underwhelming, we should write more.

That does not block this PR tho, so

r? @oli-obk

@bors r+

[bors]

📌 Commit 312bf8e has been approved by oli-obk

[bors]

⌛ Testing commit 312bf8e with merge 6e0a8bf...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 6e0a8bf to master...