delegation: fix ICE with double definition inside block in impl header - #159140
delegation: fix ICE with double definition inside block in impl header#159140mansiverma897993 wants to merge 1 commit into
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @jackh726 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
|
@workingjubilee Np apart from this I need to changes anything ? |
|
Could you also fix the description of the PR, as it contains some strange symbols like |
This comment has been minimized.
This comment has been minimized.
|
I've updated the implementation of |
4 similar comments
|
I've updated the implementation of |
|
I've updated the implementation of |
|
I've updated the implementation of |
|
I've updated the implementation of |
|
@aerooneqq Done! I implemented the suggested match expression, moved the test to a separate file, and cleaned up the PR description formatting. Please check it out. |
This comment has been minimized.
This comment has been minimized.
| @@ -68,5 +68,4 @@ mod ice_155202 { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Unnecessary change.
There was a problem hiding this comment.
Thanks for the review!
I have updated the PR with the following changes:
- Simplified the reproduction test case to avoid the off-topic duplicate definition errors.
- Reverted the unrelated whitespace change in
hir-crate-items-before-lowering-ices.rs. - Streamlined
fn_kindlogic to check forDefKind::AssocFnand default toFnKind::Freefor all other cases, which also prevents compiler panics/ICEs on foreign functions and unresolved items.
There was a problem hiding this comment.
Reverted the unrelated whitespace change in hir-crate-items-before-lowering-ices.rs.
This wasn't actually done.
Streamlined fn_kind logic to check for DefKind::AssocFn and default to FnKind::Free for all other cases
We still need to panic on unexpected variants there.
which also prevents compiler panics/ICEs on foreign functions and unresolved items.
If something previously produced an ICE, but is fixed now, then it needs a test case.
|
CI is also failing. |
|
Reminder, once the PR becomes ready for a review, use |
When a function is defined inside a block (such as an anonymous constant or a macro like `core::direct_const_arg!`) in an impl header, its parent is the enclosing `Impl` item. `fn_kind` determines if a function/delegation item is associated or free based on its parent's DefKind. When the parent is `DefKind::Impl`, `fn_kind` returned `FnKind::AssocInherentImpl` or `FnKind::AssocTraitImpl`, even though the function is actually block-local (free). This incorrect classification propagates, hitting `unreachable!` panics in delegation helper functions. Fix this by matching on `tcx.def_kind(def_id)` and returning `FnKind::Free` immediately if the function has `DefKind::Fn` (since associated functions always have `DefKind::AssocFn`). Fixes rust-lang#159127
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@mansiverma897993 |
And that's how you get no response at all 😉 |
View all comments
When a function is defined inside a block (such as an anonymous constant or a macro like
core::direct_const_arg!) in an impl header, its parent is the enclosingImplitem.fn_kinddetermines if a function/delegation item is associated or free based on its parent's DefKind. When the parent isDefKind::Impl,fn_kindreturnedFnKind::AssocInherentImplorFnKind::AssocTraitImpl, even though the function is actually block-local (free). This incorrect classification propagates, hittingunreachable!panics in delegation helper functions.Fix this by matching on
tcx.def_kind(def_id)and returningFnKind::Freeimmediately if the function hasDefKind::Fn(since associated functions always haveDefKind::AssocFn).Fixes #159127