ICE: An unexpected panic occurs when const function containing panic!("") drops a value with const_precise_live_drops.

[lilasta]

This only occur in --edition=2021.

Code

playground

#![feature(const_precise_live_drops)]

const fn eh<T>(_val: T) {
    if false {
        // The same is true for `unimplemented`, `unreachable` and `todo`.
        // In the case of `panic!()`, an error doesn't occur.
        panic!("");
    }
}

fn main() {}

Meta

rustc --version --verbose:

rustc --version --verbose
rustc 1.57.0-nightly (c1026539b 2021-10-15)
binary: rustc
commit-hash: c1026539bd22e9d070988deaa47b1360cbc76436
commit-date: 2021-10-15
host: x86_64-unknown-linux-gnu
release: 1.57.0-nightly
LLVM version: 13.0.0

Error output

thread 'rustc' panicked at 'assertion failed: promoted.is_none()', compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs:292:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.57.0-nightly (c1026539b 2021-10-15) running on x86_64-unknown-linux-gnu

query stack during panic:
#0 [mir_drops_elaborated_and_const_checked] elaborating drops for `eh`
#1 [analysis] running analysis passes on this crate
end of query stack

Backtrace

stack backtrace:
   0: rust_begin_unwind
             at /rustc/c1026539bd22e9d070988deaa47b1360cbc76436/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/c1026539bd22e9d070988deaa47b1360cbc76436/library/core/src/panicking.rs:100:14
   2: core::panicking::panic
             at /rustc/c1026539bd22e9d070988deaa47b1360cbc76436/library/core/src/panicking.rs:50:5
   3: rustc_const_eval::transform::check_consts::qualifs::in_operand
   4: <rustc_const_eval::transform::check_consts::resolver::FlowSensitiveAnalysis<Q> as rustc_mir_dataflow::framework::Analysis>::apply_statement_effect
   5: rustc_mir_dataflow::framework::engine::Engine<A>::iterate_to_fixpoint
   6: rustc_const_eval::transform::check_consts::check::Qualifs::needs_drop
   7: <rustc_const_eval::transform::check_consts::post_drop_elaboration::CheckLiveDrops as rustc_middle::mir::visit::Visitor>::visit_terminator
   8: rustc_const_eval::transform::check_consts::post_drop_elaboration::check_live_drops
   9: rustc_mir_transform::mir_drops_elaborated_and_const_checked
  10: rustc_query_system::query::plumbing::try_execute_query
  11: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::mir_drops_elaborated_and_const_checked
  12: rustc_session::utils::<impl rustc_session::session::Session>::time
  13: rustc_interface::passes::analysis
  14: rustc_query_system::query::plumbing::try_execute_query
  15: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::analysis
  16: rustc_interface::passes::QueryContext::enter
  17: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  18: rustc_span::with_source_map
  19: scoped_tls::ScopedKey<T>::set
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[oli-obk]

🤦 ok yea, I can totally see how we got here. Const checking used to happen before promotion, but const_precise_live_drops is happening after, but re-uses const checking machinery.

I think we can safely change https://github.com/rust-lang/rust/blob/master/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs#L292-L294 to return false if promoted.is_some(), as the promoted can't possibly have any of the three qualifs that exist.

My reasoning for that is that in order for a promoted to have been created, it must already be

• free of things that drop, or
• things that can be mutated

and it can't be part of a pattern either, so we don't care about non-derived PartialEq impls.

cc @rust-lang/wg-const-eval

[RalfJung]

as the promoted can't possibly have any of the three qualifs that exist.

Is this also true for promoteds inside const/static bodies, which can call arbitrary const fn?

[oli-obk]

Is this also true for promoteds inside const/static bodies, which can call arbitrary const fn?

hmm good point, there may be a non-const drop value in a promoted there. But... using in_any_value_of_ty may be a breaking change. I guess we need to recurse into the MIR of the promoted.

[RalfJung]

there may be a non-const drop value in a promoted there.

The promoted is a const fn though... so I think there can be const drops (even const drops that panic), but no non-const drops?

[oli-obk]

Vec::new is const fn and not const drop

[RalfJung]

But AFAIK it is also not promoted even in const/static bodies?

[oli-obk]

Sorry, I misread your comment. You were saying this specific issue is for a promoted in a const fn.

We need to check whether, with precise drops, we can end up with promoteds that are problematic, as we delayed some of the drop checks. I don't think we delay the drop checks for promotion, so we should be good on that end. All other const checks are guaranteed to happen before promotion, so we can ignore them here.

If we want to make this all explicit, we could add an in_promoted function to Qualif and have all other qualifs keep panicking.

[RalfJung]

We need to check whether, with precise drops, we can end up with promoteds that are problematic, as we delayed some of the drop checks.

Wait, we are promoting things and then checking if that was a good idea? That doesn't work, as it could lead to compile errors in code that would have been fine had we not promoted.

[oli-obk]

We aren't. Or... we shouldn't. But we need to make sure before we can enable precise drops

[tmiasko]

If we want to make this all explicit, we could add an in_promoted function to Qualif and have all other qualifs keep panicking.

That sounds good to me. We can also fallback to type-based reasoning, after all promoted are references, so they don't need drop.

[oli-obk]

Ralf is right, we should never have to look into promoteds, they must be correctly constructed or not at all. So we don't need an in_promoted per se, we can just add an associated constant ALLOW_PROMOTED and panic if it is not set and return false otherwise

[tmiasko]

So we don't need an in_promoted per se, we can just add an associated constant ALLOW_PROMOTED and panic if it is not set and return false otherwise

FWIW, that's how I understood your earlier proposal, with NeedsNonConstDrop::in_promoted returning false and panicking everywhere else.

[oli-obk]

Yea, either const or fn is fine, it just seems like a function sends the wrong message, as it could mean you could actually add logic in it