ICE in const fn evaluation - Disagreement between legacy and dataflow-based const validators when a const fn takes &mut self

[fschutt]

Error

error[E0493]: destructors cannot be evaluated at compile-time
   --> cargo/azul-core/../../azul-core/dom.rs:895:69
    |
895 |     pub const fn set_node_type(&mut self, node_type: NodeType<T>) { self.node_type = node_type; }
    |                                                                     ^^^^^^^^^^^^^^ constant functions cannot evaluate destructors

error[E0019]: constant function contains unimplemented expression type
   --> cargo/azul-core/../../azul-core/dom.rs:895:69
    |
895 |     pub const fn set_node_type(&mut self, node_type: NodeType<T>) { self.node_type = node_type; }
    |                                                                     ^^^^^^^^^^^^^^

[ERROR rustc_mir::transform::qualify_consts] old validator: [(cargo/azul-core/../../azul-core/dom.rs:895:69: 895:83, "LiveDrop"), (cargo/azul-core/../../azul-core/dom.rs:895:69: 895:83, "MutDeref")]
[ERROR rustc_mir::transform::qualify_consts] new validator: [(cargo/azul-core/../../azul-core/dom.rs:895:69: 895:83, "MutDeref"), (cargo/azul-core/../../azul-core/dom.rs:895:69: 895:83, "LiveDrop")]
error: internal compiler error: src/librustc_mir/transform/qualify_consts.rs:1986: Disagreement between legacy and dataflow-based const validators.
    After filing an issue, use `-Zsuppress-const-validation-back-compat-ice` to compile your code.
   --> cargo/azul-core/../../azul-core/dom.rs:895:5
    |
895 |     pub const fn set_node_type(&mut self, node_type: NodeType<T>) { self.node_type = node_type; }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Backtrace

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:890:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1028
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1412
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:188
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:205
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:468
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::span_bug
  14: rustc_errors::Handler::span_bug
  15: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc::ty::context::tls::with_opt::{{closure}}
  17: rustc::ty::context::tls::with_context_opt
  18: rustc::ty::context::tls::with_opt
  19: rustc::util::bug::opt_span_bug_fmt
  20: rustc::util::bug::span_bug_fmt
  21: rustc_mir::transform::qualify_consts::Checker::check_const
  22: <rustc_mir::transform::qualify_consts::QualifyAndPromoteConstants as rustc_mir::transform::MirPass>::run_pass
  23: rustc_mir::transform::run_passes
  24: rustc_mir::transform::mir_validated
  25: rustc::ty::query::__query_compute::mir_validated
  26: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_validated>::compute
  27: rustc::dep_graph::graph::DepGraph::with_task_impl
  28: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  29: rustc_mir::borrow_check::mir_borrowck
  30: rustc::ty::query::__query_compute::mir_borrowck
  31: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_borrowck>::compute
  32: rustc::dep_graph::graph::DepGraph::with_task_impl
  33: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  34: rustc::ty::<impl rustc::ty::context::TyCtxt>::par_body_owners
  35: rustc_interface::passes::analysis
  36: rustc::ty::query::__query_compute::analysis
  37: rustc::dep_graph::graph::DepGraph::with_task_impl
  38: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  39: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
  40: rustc_interface::passes::create_global_ctxt::{{closure}}
  41: rustc_interface::passes::BoxedGlobalCtxt::enter
  42: rustc_interface::interface::run_compiler_in_existing_thread_pool
  43: std::thread::local::LocalKey<T>::with
  44: scoped_tls::ScopedKey<T>::set
  45: syntax::with_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.40.0-nightly (1423bec54 2019-11-05) running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type rlib --crate-type cdylib

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [mir_validated] processing `dom::NodeData::<T>::set_node_type`
#1 [mir_borrowck] processing `dom::NodeData::<T>::set_node_type`
#2 [analysis] running analysis passes on this crate
end of query stack
error: aborting due to 3 previous errors

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.40.0-nightly (1423bec54 2019-11-05) running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type rlib --crate-type cdylib

note: some of the compiler flags provided by cargo are hidden

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0019, E0493.
For more information about an error, try `rustc --explain E0019`.
error: could not compile `azul-core`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Code

https://github.com/maps4print/azul/tree/rust-const-ice

Commit fschutt/azul@b38fb1b

Changed here: fschutt/azul@b38fb1b#diff-ce9aead5dc9a44d99879f251fc571013R897

Code: https://github.com/maps4print/azul/blob/b38fb1bc3476ee47d313e891642f7673092e080b/azul-core/dom.rs#L895

Reproducing the bug

rustup update
git clone https://github.com/maps4print/azul
git checkout rust-const-ice
cargo check

Description

I was trying to turn a couple of functions that take a &mut self into const fns - which should technically work:

struct Dom { a: usize }

impl Dom {
    const fn do_thing(&mut self, a: usize) { self.a = a; }
}

As far as I can see, only the ordering of liveness-checking is wrong, the old liveness checker reports ["LiveDrop", "MutDeref"] while the new liveness checker reports ["MutDeref", "LiveDrop"]. Should be an easy (?) bug to fix (famous last words).

Rust version:

rustc 1.40.0-nightly (1423bec54 2019-11-05)
binary: rustc
commit-hash: 1423bec54cf2db283b614e527cfd602b481485d1
commit-date: 2019-11-05
host: x86_64-unknown-linux-gnu
release: 1.40.0-nightly
LLVM version: 9.0

[ecstatic-morse]

This occurs because the old checker calls super_terminator_kind after checking for NeedsDrop in a DropAndReplace terminator, while the new checker calls it before.

I'm inclined not to do a short-term patch for this. This code is correctly rejected by both const checkers, and this ICE is only ever shown to nightly users. This will ultimately be fixed when the old const-checker is removed.

[pnkfelix]

@ecstatic-morse is there a timeline for when the old const-checker will be removed?

We're going to keep getting bug reports like this until that happens, so it would be good to have an idea of how long its going to be in terms of deciding whether it is worth while to try to work-around this in some way.

(I don't know enough about how the two validators are comparing their results to know how hard such a task would be. It sounds like you are saying that both validators reject the input here. Does that mean you could use a coarser approach in the comparison, such as not emitting the ICE unless you hit a case where one validator accepts and the other rejects, rather than the fine-grained comparison it sounds like is happening here?)

[pnkfelix]

triage: leaving nominated for discussion at T-compiler meeting

[ecstatic-morse]

@pnkfelix As eddyb mentioned on Zulip, we should be able to move off the old checker in a week or two.

I do want to continue to be as fine-grained as possible here, since it uncovered an interesting case in #65485, and could still uncover more. However, looking at this again today, I see no reason not to sort the errors before comparing them, which would make this case go away. WDYT?

[pnkfelix]

@ecstatic-morse yes lets sort the errors, that would be great!

[pnkfelix]

discussed at T-compiler meeting. This seems well in hand; either we'll fix it by sorting the errors, as mentioned, or we'll just let it get fixed when we remove the old const-check, which is scheduled sooner than I expected.