More powerful const panic

[nbdd0121]

Add a const_panic_extra feature gate that enables const panic to panic with any format arguments.

It should make the example given in the rust-lang/rfcs#2345 possible:

const fn parse_bool(s: &str) -> bool {
    match s {
        "true" => true,
        "false" => false,
        other => panic!("`{}` is not a valid bool", other),
    }
}
parse_bool("true");
parse_bool("false");
parse_bool("foo");

The mechanism implemented in this PR essentially extracts arguments from const-eval Arguments and delegate the formatting to formatting machinary that's compiled with rustc. Currently only these types are formatted:

• Non-reference primitives
• Reference to types under the blanket impl (e.g. impl<T: Display> for &Display)
• String (the only const String is empty string)
• Arguments
• dyn Trait if Trait is one of the formatting traits.

Other types will be formatted to <failed to format {ty}>.

I think it's possible to require that only types that can be correctly formatted are accepted by having an additional marker type, but that'll require const trait impl and specialization like this:

trait ConstDebug: Debug {}
impl<T: Debug> ConstDebug for T {}
impl const ConstDebug for &str {}

IMO it's okay to accept types that couldn't be formatted, given that const panic messages are considered diagnostics. It'll also make constification of assert_eq! easier (which uses &dyn Debug internally) since we current have no way to represent &dyn ~const ConstDebug.

I implement the feature in this way:

• This is reasonable simple;
• We won't see the whole formatting machinary be accepted by const-eval in near feature (previously suggested idea like using unleash miri doesn't work because the machinary's MIR isn't all available);
• People are removing info from panic messages to make it const, which is not desirable;
• There is a real need to make things like Result::unwrap const;

I plan to file/work on a few follow up PRs if this one is accepted:

• Constification of Result::unwrap
• Const panic fmt of additional types from std
• Const panic fmt of ADTs if the Debug implementation is derived

This is a bulky PR, but I've separated it to individually meaningful commits so I'd suggest to review by commits. Some changes are made to ArgumentV1::new to allow the const_panic_extra being used without const_fn_fn_ptr_basics and const_mut_refs. Given this is hot code I'd suggest a perf run.

r? @oli-obk
@rustbot label: T-compiler A-const-eval

[rust-highfive]

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

[camelid]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit c3ea64c with merge 513ad0280218aa8bfcb61c04b8d0384fceb042ea...

[camelid]

(FYI, you may want to wait to fix tests until after the bors try run finishes; IIRC, sometimes pushing will cancel a try.)

[bors]

☀️ Try build successful - checks-actions
Build commit: 513ad0280218aa8bfcb61c04b8d0384fceb042ea (513ad0280218aa8bfcb61c04b8d0384fceb042ea)

[rust-timer]

Queued 513ad0280218aa8bfcb61c04b8d0384fceb042ea with parent db062de, future comparison URL.

[rust-timer]

Finished benchmarking commit (513ad0280218aa8bfcb61c04b8d0384fceb042ea): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Large improvement in instruction counts (up to -1.9% on full builds of cranelift-codegen)
• Large regression in instruction counts (up to 3.3% on full builds of tokio-webpush-simple)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[bjorn3]

Can these be inlined into the format_args!() expansion?

[nbdd0121]

These are generic functions so I think yes.

[bjorn3]

I actually meant can we inline their definition into the format_args!() expansion and omit those function definitions themself.

[nbdd0121]

(Pushed to the wrong branch)

[Dylan-DPC]

@nbdd0121 this has conflicts to be resolved, once done we can get this reviewed quickly after that

[oli-obk]

Summary of the zulip thread:

There are three ways to continue:

• do nothing, keep status quo. Not bad, not great either
• do this PR, allowing all types that implement Display, but bail out by printing some form of <can't print>. This means we can just print all types conveniently, but it also means we need to keep this complex logic around forever and possibly extend it with more and more special handling. It may also be surprising, similar to how it would be suprising if {:?} would accept types that don't implement Debug, but just print some default message
• restrict this PR to types that we know can have some CTFE logic to print them in the future, so that we can revert the PR some day without a breaking change and instead.

Ralf and I consider option 2 to be problematic and would prefer to go with option 3. We can always go for option 2 later, but if we start with it we can't revert to option 3.

That said, we need a feature gate for this anyway, so we could just merge it modulo some additional tests making sure the feature gate works and split out option 3 under a separate gate later.

[c410-f3r]

In order to make #96496 work in constant environments (or even a future stabilization), this PR or functionality is a must have.

[nbdd0121]

This PR has been stagnated due to lack of strong motivating use case -- I am glad that it now has one!

[oli-obk]

Can you elaborate on what functionality of this PR is needed? Afaict option 3 is sufficient for #96496

[nbdd0121]

If we have a, b of type T: Debug but not a restricted const printable type, then assert!(a == b) currently works in const context, but not with #96496.

[RalfJung]

That sounds like a bug in the implementation of #96496.

Inside a const fn, it can only debug-print things if they are const Debug, of course.

[nbdd0121]

That sounds like a bug in the implementation of #96496.

Inside a const fn, it can only debug-print things if they are const Debug, of course.

That'll make a const fn and a non-const fn behave differently even if the const fn is not called in const context.

[oli-obk]

That'll make a const fn and a non-const fn behave differently even if the non-const fn is not called in const context.

you cannot call non-const fn from const fn at all. Did you flip non-const and const in the second part of your sentence?

[c410-f3r]

Can you elaborate on what functionality of this PR is needed? Afaict option 3 is sufficient for #96496

const _: () = {
    let a = 1;
    assert!(a != 1);
};

Currently works but such thing won't be possible in #96496 without constant panic! formatting. I am not familiar enough with the internals of CTFE to conclude which option could satisfy this use-case.

[oli-obk]

As long as #96496 only requires integers and booleans to be formattable, then it will work out of the box. If it requires arbitrary types to be formattable (e.g. Mutex or something else that does things that are impossible in const eval), then it will not be possible in const eval

[c410-f3r]

As long as #96496 only requires integers and booleans to be formattable, then it will work out of the box. If it requires arbitrary types to be formattable (e.g. Mutex or something else that does things that are impossible in const eval), then it will not be possible in const eval

Yes, types are arbitrary but all must implement Copy and Debug to be eligible for displaying. Will these bounds satisfy the required restrictions of const printing?

[oli-obk]

Yes, types are arbitrary but all must implement Copy and Debug to be eligible for displaying. Will these bounds satisfy the required restrictions of const printing?

ah perfect. If you are going via trait bounds instead of just syntax, then it will reject all types that do not implement const Debug, but only within const contexts. Even if your current impl does not manage this yet, we can always address that later. I feared it was a syntactical thing, where assert!(non_debug_type == foo) would fail to compile in the future.

[c410-f3r]

Yes, types are arbitrary but all must implement Copy and Debug to be eligible for displaying. Will these bounds satisfy the required restrictions of const printing?

ah perfect. If you are going via trait bounds instead of just syntax, then it will reject all types that do not implement const Debug, but only within const contexts. Even if your current impl does not manage this yet, we can always address that later. I feared it was a syntactical thing, where assert!(non_debug_type == foo) would fail to compile in the future.

Thanks! Looking forward to see

const _: () = {
    let a = 1i32;
    panic!("Something {}", a);
};

Work someday

[oli-obk]

That'll make a const fn and a non-const fn behave differently even if the const fn is not called in const context.

This is what will happen for all other traits, too, not just Debug/Display.

On the PR itself:

I would like to move this PR along. But neither Ralf nor I will approve the current version. I know you feel strongly for allowing arbitrary types to be printed in const eval, even if they are unprintable (e.g. pointers). While I personally don't ever want this, and think you should not propose it, I think the right way to get this approved in your version would be an RFC/lang-team-MCP. Your proposed scheme just diverges too far from what we have right now, it can't just be a simple feature and stabilization.

Please consider restricting your implementation to builtin types that are guaranteed to get const evaluable Debug and Display impls

[Dylan-DPC]

Closing this based on comment.