Evaluating constants in MIR optimizations introduces const eval errors

[tmiasko]

#![feature(inline_const)]
#![crate_type = "lib"]
pub fn f<T>() -> usize {
    g::<()>()
}
pub fn g<T>() -> usize {
    const { 0 / std::mem::size_of::<T>() }
}

This compiles fine in debug builds, but fails in release builds:

$ rustc b.rs
$ rustc b.rs -O
error[E0080]: evaluation of `g::<()>::{constant#0}` failed
 --> b.rs:7:13
  |
7 |     const { 0 / std::mem::size_of::<T>() }
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to divide `0_usize` by zero

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0080`.

Based on https://github.com/rust-lang/rust/issues/107503#issuecomment-2011659833 cc @RalfJung.

But also see this example based on type sizes.

[RalfJung]

Ah, so there is a way to trigger this via inlining, dang.

Cc @rust-lang/wg-const-eval @rust-lang/wg-mir-opt

[RalfJung]

All functions in this example are generic, so no amount of collector tweaking can possibly help here.

I don't really see an alternative to developing some sort of const-eval query that suppresses the error.

[RalfJung]

Also, note that even if we get such a "silent const-eval query", all the stuff in https://github.com/rust-lang/rust/pull/122568 is still needed -- collection definitely wants to use the "loud" const-eval query. That's also what makes https://github.com/rust-lang/rust/issues/122814 an independent issue from this one, even though both involve "inlining":

• This one here is about the MIR inliner monomorphizing constants, which leads to new const-eval errors.
• The other one is about #[inline] functions being cross-crate-inlinable for LLVM which affects what happens during mono-item collection.

[RalfJung]

So how could such a silent query work? I think we need a query that returns roughly Result<RawConst, ErrorInfo> where ErrorInfo contains everything needed to render the error -- in particular, all the spans of the backtrace. The the query caller can decide whether they want to ignore or show errors. We probably want a 2nd query that shows errors so that we still get deduplication if the constant is eval'd multiple times.

That sounds very doable, though perf may become a problem. What's less clear to me is how to deal with nested queries: evaluating a const may require evaluating other consts; what if those fail? If the innermost const-eval query uses silent queries for nested eval, we'll have to propagate such error info up to the result of the outer query or else we'll never see the inner failures. So probably we want ErrorInfo to be interned such that one ErrorInfo can link to the ErrorInfo of a nested query that failed, or something like that?

[oli-obk]

Sounds like you want what https://github.com/rust-lang/rust/pull/92674 tried to do. A Reveal mode that silences const eval errors and reruns in an erroring mode if one wants to report an error. So you still get caching in the success case, but need to rerun in the error case.

That was nontrivial to actually do, but I don't remember the issues

[RalfJung]

With inlining we can be evaluating failing constants even in what should be successful compilation, like the example in the OP. So not caching failures doesn't seem great.

To be clear I think the intended behavior of the example in the OP is that compilation should succeed. The fact that with optimizations enabled, we get an error -- that's a bug.

[oli-obk]

So not caching failures doesn't seem great.

we'd still cache them. I mean, when you actually want to report an error, then you rerun with the regular Reveal::All and it'll report an error.

[RalfJung]

As @scottmcm points out, this doesn't just affect const evaluation, it also affects the "maximum size of a type" check. So we'd not just need to handle MIR with consts that can fail to evaluate, but also MIR with types that are too big to exist.

Actually that example is more like https://github.com/rust-lang/rust/issues/107503, not like this issue, since it fails i debug builds and compiles in release builds.

[RalfJung]

That said, the required_consts stuff we do currently already doesn't cover type sizes. So it's likely https://github.com/rust-lang/rust/issues/107503 still exists for type sizes, and it seems pretty tricky to fix. Given that such large types are very rare, it's also unclear whether it's really worth the compile time cost to ensure that type size errors are not optimization-dependent.

EDIT: Indeed, here is an example.

[RalfJung]

A version of this issue for type sizes would look something like this:

#![crate_type = "lib"]

pub fn f<T>() -> usize {
    g::<u8>()
}
pub fn g<T>() -> usize {
    let _val = std::mem::MaybeUninit::<[T; 1 << 47]>::uninit();
    42
}

But that does not error. It seems like MIR opts already do not care about type sizes, or we just haven't found the right example.

[tmiasko]

MIR optimizations do not report any layout errors by themselves, only code generation does. #122814 can be used to generate more code in an optimized build:

#![crate_type="lib"]
pub fn f() {
    loop {}
    g();
}
#[inline(never)]
fn g() -> std::mem::MaybeUninit::<[u8; 1 << 47]> {
    std::mem::MaybeUninit::<[u8; 1 << 47]>::uninit()
}