const_panic: ICE on non-&str panic payload

[RalfJung]

The panic payload of the std::panic! macro (as passed on to begin_panic) is generic, but the CTFE panic support assumes it will always be an &str. This leads to ICEs:

#![feature(const_panic)]

const C: () = {
    panic!(1234);
};

Cc @oli-obk

[oli-obk]

I guess we could support this, but we'd have to find some way to render the errors. Maybe we need to wait for const traits so we can require const Debug?

[RalfJung]

Alternatively constck could require the argument to have type &str.

[memoryruins]

core::panic! also requires a &str

#![feature(const_panic)]
const _: () = core::panic!(1234);

error[E0308]: mismatched types
 --> src/lib.rs:2:28
  |
2 | const _: () = core::panic!(1234);
  |                            ^^^^ expected `&str`, found integer

[RalfJung]

Yeah, the libcore panic machinery does not support non-format-string panics.

[memoryruins]

Aye, left out the latter of my message. &str is likely the most common argument to panic!, and it's not unusual with e.g. core::panic!; so I agree that waiting for const traits before supporting any other arguments is reasonable.

[DutchGhost]

Im hitting this ICE on stable without feature flag:

struct Bug([u8; panic!(1)]);

[RalfJung]

Hm indeed, that is interesting:

error[E0658]: panicking in constants is unstable
 --> src/lib.rs:1:17
  |
1 | struct Bug([u8; panic!(1)]);
  |                 ^^^^^^^^^
  |
  = note: see issue #51999 <https://github.com/rust-lang/rust/issues/51999> for more information
  = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

thread 'rustc' panicked at '`ref_to_mplace` called on non-ptr type', src/librustc_mir/interpret/place.rs:292:47
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

@rust-lang/wg-const-eval any idea why we go on here after seeing an error? Does this kind of behavior mean any part of CTFE is reachable on stable somehow?

[oli-obk]

Does this kind of behavior mean any part of CTFE is reachable on stable somehow?

yes. We're not gating CTFE on whether there were stability (or any other static checks) failing. I've been abusing this since 1.0 to demo things on the stable compiler and never got around to fixing it. I don't even know if we have a tracking issue for it.

[RalfJung]

On second thought, let's leave this issue for the ICE, and make a new one for "we can trigger nightly-only const code on stable because we don't abort compilation early enough": #76064.

[sffc]

Given that this ICE is pre-existing, as shown by #66693 (comment), can we un-block #51999? I'd really like to see const_panic stabilized, and this seems like a minor issue to be blocking on.

[RalfJung]

Given that this ICE is pre-existing, as shown by #66693 (comment), can we un-block #51999? I'd really like to see const_panic stabilized, and this seems like a minor issue to be blocking on.

That comment is an example of accidentally partially exposing an unstable feature on stable -- a separate bug tracked in #76064. I don't see how that's an argument for shipping that feature on stable when it is broken. In particular, #76064 applies to all unstable const features.

Fixing this bug here is not even that hard I think: somewhere around here

rust/compiler/rustc_mir/src/transform/check_consts/validation.rs

Line 771 in 4d63435

if is_lang_panic_fn(tcx, callee) {

there needs to be a check that the argument has type &str.

Re: #51999, AFAIK that one is also blocked on figuring out how the diagnostics should look like, and a further concern (though maybe not blocking) is this problem.

[davidhewitt]

I guess that #3007 might make this a non-issue, where it is proposed std::panic! also would require &str as first argument.

[CDirkx]

I guess that #3007 might make this a non-issue, where it is proposed std::panic! also would require &str as first argument.

For reference: rust-lang/rfcs#3007, which was recently merged. The plan is to fix the behaviour of panic in the Rust 2021 edition.

Given that this ICE is listed as a blocker for const_err, does this mean that stabilizing that will have to wait for the next edition?

[abonander]

No, because breaking changes are allowed to be made to unstable features without moving to a new edition; that's kinda the whole idea. The 2021 edition will effectively just extend the same restriction to all panic!() invocations, const context or not.

[abonander]

there needs to be a check that the argument has type &str.

@RalfJung I'd like to tackle this but I'm not very experienced with the CTFE code. I get that we need to check args[0] from the Terminator::Call that we've matched on in this branch and we need to match on the Operand enum, but:

match &args[0] {
    Operand::Copy(place) | Operand::Move(place) => // how to get the type of `Place` here?
    Operand::Constant(constant) => // compare constant.ty, but against what?
}

For the first question, I see Place::iter_projections() and that we probably want to check that the PlaceElem type of the tuple and that there's variants of ProjectionElem that are Ty, but do we stop the iteration when we get one that is a type? Is the projection list always guaranteed to terminate in a variant of ProjectionElem that has a Ty? And once we have an instance of Ty, do we just check that there's any projection in the list that has a Ty::kind() == TyKind::Str?

[RalfJung]

@abonander you can use Operand::ty to get the type of the Operand. Then you can test its kind() to be Str.