Make missing_const_for_fn operate on non-optimized MIR #14003
Conversation
This has two reasons: First of all we don't need the optimized MIR for evaluating the checks of this lint, as const-eval anyways operates on `mir_for_ctf` which is derived from `mir_drops_elaborated_and_const_checked`. Second of all we might transform MIR in the optimization passes in a way that doesn't work with const-eval, but it is irrelevant since const-eval uses another MIR. Specifically this came up when adding a new check in debug builds (rust-lang/rust#134424), which is added as part of an optimization pass.
rustbot
added
the
S-waiting-on-review
|
Huh, I'm surprised this works and doesn't ICE. I was under the impression that the only MIR we can use is optimized MIR, since every other intermediate form gets stolen/consumed by the next form. We use |
|
Ohh I was not aware of that, good point. It seems like it only works out of coincidence. Are you aware of any other way of obtaining a non-steal-able MIR before optimization passes? The problem is that the optimized MIR has checks inserted which are maybe not const (and that is fine if MIR for CTFE doesn't have these checks). A current example would be that the clippy lint struct Foo(*const u32);
impl Foo {
const fn deref_ptr_can_be_const(self) -> usize {
unsafe { *self.0 as usize }
}
}This is because the optimized MIR has a sequence of checks for alignment inserted: The |
|
Yeah there are also quite a few false positive reports for this lint for the same reason as you described, e.g. the I have also been thinking about the same problem but haven't been able to come up with a good solution. (I had some very hacky ideas like overriding some MIR query to temporarily have access to the unoptimized body, on which it would run the For your specific example, maybe we can just accept |
|
Oke I see, thanks a lot for the pointer to the already open bug, that's an interesting read. My mental model of how MIRs are created and thrown around isn't quite close to reality yet, but I still wonder why my code even works. I enabled all lints that create a MIR (all Regarding the alternative for |
|
Also #6080 talks about adding |
|
This only calls the query alone though and doesn't do anything with it, the part that can ICE is if you call I also feel like it's a coincidence and The warnings from
I'm not sure what the optimal way to resolve this is tbh (I'm only aware that this is a known issue), I feel like that could be useful to create a thread for in the t-compiler zulip to discuss
Do you mean that handling |
|
Thanks a lot for the discussion, this is all very insightful! I created a Zulip thread and CC'ed both of you.
While this is true (and I saw that
So as far as I understand, if clippy wants to properly model that case it would need to modify the existing check for transmute to only fire on pointers transmutes. For my case of debug checks this still keeps the problem, because we actually transmute a pointer (just not in the const-eval MIR). Does that sound correct to you? 😅 |
|
Looking at the whole situation, I think that I'll approve this PR. In theory, late linting happens in an isolated stage, one by one (which I'm working on to parallelize), so this should not mean anything bad. I'll try again to do a big lintcheck of the 1000 most popular crates and if everything's correct, I'll merge. Ideally, we would also check if other lints that use |
The main reason for this is that we might transform MIR in the optimization passes in a way that doesn't work with const-eval, but it is irrelevant since const-eval uses another MIR (
mir_for_ctfe).Specifically this came up when adding a new check in debug builds (rust-lang/rust#134424), which is added as part of an optimization pass.
changelog: none