Fetch the value of has_ffi_unwind_calls before stealing mir_built.#108777
Fetch the value of has_ffi_unwind_calls before stealing mir_built.#108777cjgillot wants to merge 1 commit intorust-lang:masterfrom
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
| // so make sure it is run. | ||
| if def.const_param_did.is_none() { | ||
| // Actually fetch the value, to avoid having to compute the query later. | ||
| let _ = tcx.has_ffi_unwind_calls(def.did); |
There was a problem hiding this comment.
There are ensure() calls few lines above this, are they unaffected by the same issue for some reason?
Also, what is the use-case for ensure, if it doesn't ensure that we have the query value?... I'm very confused by all of this...
There was a problem hiding this comment.
ensure creates a dependency edge. However, it does not compute the value. For instance, if has_ffi_unwind_calls were to emit diagnostics, ensure would check that the dependencies are unchanged, replay the diagnostics (loaded from disk), but wouldn't try to load the cached value.
And yes, all uses of ensure() before a steal() are probably wrong. However, I find unelegant to invoke a query just to ignore its result.
There are 2 ways to definitely fix this:
- change most of the
ensures; - check that we can decode the value from disk on
ensure.
I'm leaning on 2, but haven't finished implementing it.
|
Filed #108820 to implement the other solution. |
WaffleLapkin
removed
the
S-waiting-on-review
3 participants
Just calling
ensureis not enough, as it does not verify that the value can be loaded from the on-disk cache. Later code could then try to get the value, fail to find it in the cache, and call thehas_ffi_unwind_callsfunction, and boom.Fixes #104423
Fixes #105157
Fixes #107797
Fixes #108411
Fixes #108633