fix ICE in note_and_explain when handling cross crate DefIds#153382
fix ICE in note_and_explain when handling cross crate DefIds#153382Shrey-N wants to merge 12 commits intorust-lang:mainfrom
Conversation
Add a test for compiler ICE with cross crate traits
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
#153375 doesn't have any steps to reproduce. How did you determine that this test case reproduces the problem? |
|
The test doesn’t have error annotations either, does it actually ICE with the fix reverted? |
I tested this and no, it doesn't ICE. (It does fail with "error: ui test did not emit an error" both before and after the reversion.) |
|
Seeing the test the LLM produced having nothing to do with the code in the repository from that issue, I expected that as well. I wonder if the fix was tried on that crate. |
|
Hiya @nnethercote and @lqd My bad, I totally missed that. Because the test was using check pass, the compiler never actually entered the diagnostic code path. I am updating the test right now to include a lifetime mistake on purpose. That should force the compiler to look at the external trait and try to explain why the lifetimes don't match which is exactly where it hits that expect_local panic without the fix. Give me just a second to push it |
|
The issue mentions the ICE happening with cargo check, and the test wasn’t using check pass either. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
You are right @lqd my previous test used check pass, it was valid and never entered the diagnostic code path. I have pushed an update with a deliberate lifetime error to force the exact cross-crate region explanation that triggered the ICE. (And just fixed the tidy error which is now back again :( ). |
Did you try, and did it work? |
This comment has been minimized.
This comment has been minimized.
Fix lifetime issue in test function to ensure proper bounds.
|
@lqd I don't have my local environment built to test it properly but the CI just proved it works perfectly The recent CI run failed because the compiler successfully emitted the expected error: lifetime may not live long enough instead of crashing with the expect_local() ICE. I just pushed the missing ice-cross-crate-defid.stderr file, so the checks should pass completely now. Thanks for bearing with me :) |
Just running any rustc will work to do that. Let us know how that goes!
Did the LLM generate it, you need some kind of environment to bless it otherwise, and can't you check in that environment?
They will not, and our questions are to see how effective is the fix.
Not yet, because that depends on the question above.
(I had said quite the opposite.) |
This comment has been minimized.
This comment has been minimized.
|
@lqd I did not use an LLM, I actually just copied the ---stderr--- block out of the aarch64-gnu-llvm-20-1 CI failure log, that ran on the PR, I am having issues building x.py locally, I tried to take a wrong shortcut of using the CI logs... And yea i misread the earlier comment of yours, I apologise for the confusion from my side, I am trying to set up a proper linux env from source, for verification, I will message back once the tests run. :( |
|
Oh, it was quite unlucky that some characters had changed in the copy paste then. |
@lqd I actually made a manual edit before, I was dumb -_- messed it up thought I knew better than the compiler, now I ran it on my machine, this should work now (attached an ss), The updated .stderr should work properly |
|
Cool you have both a Windows and Linux environment now, did the ICE reproduce there? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@lqd I was able to reproduce the ICE locally :)
|
|
This is showing the test, not the ICE though. Try to rerun that test again, but with the first commit reverted. It needs to fail. Similarly, also try running the compiler you have now built in that screenshot (with the fix in the first commit still in) on the crate linked in the issue, following the steps to reproduce there. It needs to not ICE there. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |


View all comments
Fixes an Internal Compiler Error (ICE) in
note_and_explainwhere the compiler would panic withDefId::expect_local: DefId(...) isn't localwhen attempting to explain lifetime errors involving definitions from external crates.The crash occurred because the diagnostic logic assumed that items being explained were always local to the current crate. This replaces
.expect_local()with.as_local()to safely handle cross crateDefIds, falling back totcx.def_span()for external definitions.A regression test is included in
tests/ui/lifetimes/ice-cross-crate-defid.rsusing an auxiliary crate to reproduce the cross-crate boundary condition.Fixes
Fixes #153375