Repository navigation
Conversation
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
S-waiting-on-review
|
r? @jieyouxu rustbot has assigned @jieyouxu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
cc @RalfJung |
This comment has been minimized.
This comment has been minimized.
ab84fb6 to
8714e50
Compare
| sess.time("MIR_borrow_checking", || { | ||
| tcx.par_hir_body_owners(|def_id| { | ||
| if tcx.is_intrinsic_that_must_be_overridden(def_id) { | ||
| return; |
There was a problem hiding this comment.
| return; | |
| return; // there is no MIR here to check |
| @@ -334,7 +334,10 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> { | |||
|
|
|||
| // Remove the fake bodies for `global_asm!`, since they're not useful | |||
There was a problem hiding this comment.
The comment needs to be updated
| let def_id = local_def_id.to_def_id(); | ||
| let kind = tcx.def_kind(def_id); | ||
| if !kind.is_fn_like() { | ||
| if !kind.is_fn_like() || tcx.is_intrinsic_that_must_be_overridden(def_id) { |
There was a problem hiding this comment.
Some intrinsics can unwind, so false does not seem like a conservative answer here?
There was a problem hiding this comment.
well, we previously also returned false, because there was nothing in the body (except for an unreachable terminator). So yea, we should def figure out what the right behaviour here is. I don't see anything marking intrinsics as possibly unwinding, so we'd need to start doing that.
also this is about ffi unwinds, so this would be limited to things like const_eval_select that invoke closures or callbacks?
There was a problem hiding this comment.
Where does this nice error come from? I expected this to ICE.
There was a problem hiding this comment.
Checking upstream crates' mir goes through is_mir_available as MIR isn't encoded unless the function is inline or generic
There was a problem hiding this comment.
Sure -- but why does that not ICE? Hitting this error is always a bug, isn't it?
There was a problem hiding this comment.
iirc there are some tests for weird combinations of cli flags that can cause this. But yea, ideally this would be an ICE
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
8714e50 to
1e52c52
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
1e52c52 to
b474d01
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
fixes #164984
In #122037 I created those fake bodies because we wanted to avoid all the checks I added here, and the intrinsics are always overridden anyway. Well, so we thought. If you're creative you can get at them, and it's possible there are cases that could be exposed on stable. Seems generally nicer to make it more robust