-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Evaluating constants in MIR optimizations introduces const eval errors #122828
Comments
Ah, so there is a way to trigger this via inlining, dang. Cc @rust-lang/wg-const-eval @rust-lang/wg-mir-opt |
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. |
Also, note that even if we get such a "silent const-eval query", all the stuff in #122568 is still needed -- collection definitely wants to use the "loud" const-eval query. That's also what makes #122814 an independent issue from this one, even though both involve "inlining":
|
So how could such a silent query work? I think we need a query that returns roughly 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 |
Sounds like you want what #92674 tried to do. A That was nontrivial to actually do, but I don't remember the issues |
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. |
we'd still cache them. I mean, when you actually want to report an error, then you rerun with the regular |
Actually that example is more like #107503, not like this issue, since it fails i debug builds and compiles in release builds. |
That said, the required_consts stuff we do currently already doesn't cover type sizes. So it's likely #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. |
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. |
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()
} |
This compiles fine in debug builds, but fails in release builds:
Based on #107503 (comment) cc @RalfJung.
But also see this example based on type sizes.
The text was updated successfully, but these errors were encountered: