Fix ICE caused by invalid spans for shrink_file #148735
Conversation
|
cc @Muscraft |
rustbot
added
S-waiting-on-review
| WARN rustc_errors::emitter Invalid span $SRC_DIR/std/src/macros.rs:LL:COL (#4), error=SourceNotAvailable { filename: Real(Remapped { local_path: None, virtual_name: "$SRC_DIR/std/src/macros.rs" }) } | ||
| WARN rustc_errors::emitter Invalid span $SRC_DIR/std/src/macros.rs:LL:COL (#4), error=SourceNotAvailable { filename: Real(Remapped { local_path: None, virtual_name: "$SRC_DIR/std/src/macros.rs" }) } | ||
| WARN rustc_errors::emitter Invalid span $SRC_DIR/std/src/macros.rs:LL:COL (#4), error=SourceNotAvailable { filename: Real(Remapped { local_path: None, virtual_name: "$SRC_DIR/std/src/macros.rs" }) } |
There was a problem hiding this comment.
I have worries that this will not pass aarch64-msvc-1 the same way it was in my PR
There was a problem hiding this comment.
ah, it do have the same issue.
There was a problem hiding this comment.
so, I read the discussion on zulip, seems there is another standalone bug. so is it a temprary way we skip Windows for this testcase(and add it back after that bug is fixed)?
|
@bors try jobs=aarch64-msvc-1 |
This comment has been minimized.
This comment has been minimized.
Fix ICE caused by invalid spans for shrink_file try-job: aarch64-msvc-1
This comment has been minimized.
This comment has been minimized.
|
💔 Test for eca91bd failed: CI. Failed jobs:
|
| let hi_byte = spans.iter().map(|s| s.hi()).max()?; | ||
| let hi_loc = sm.lookup_char_pos(hi_byte); | ||
|
|
||
| if lo_loc.file.name != hi_loc.file.name { |
There was a problem hiding this comment.
| if lo_loc.file.name != hi_loc.file.name { | |
| if lo_loc.file.stable_id != hi_loc.file.stable_id { |
Using the stable_id avoids issues with file name collisions when multiple versions of the same crate are present in the dependency graph. In theory, collect_annotations should be ensuring that the stable_ids match but it would be better to have it checked in both places.
There was a problem hiding this comment.
I found a few other places to use file.name to cmp, maybe we can do a cleanup in another PR?
There was a problem hiding this comment.
Doing cleanup for the other places in another PR makes sense. I can open a PR for that in the next few days if you'd like.
There was a problem hiding this comment.
I just have a check by the way, and can fire a PR later.
|
r=me once the comments are addressed. @rustbot author |
rustbot
added
S-waiting-on-author
chenyukang
force-pushed
the
yukang-fix-ice-148732
branch
from
7262408 to
725b213
Compare
6 participants
Fixes #148732
There are two issues in this function:
7..7is beyond the end of buffer0) will be reported after fixing the first one, is caused by spans cross file boundaries due to macro expansion. It is fixed in the second commit.r? @nnethercote
edited: also fixes #148684, added a new testcase for it in the last commit.