Skip to content

fixed generics of crate node ICE#153615

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
enthropy7:fix-ice-generics-of-crate-node
Apr 6, 2026
Merged

fixed generics of crate node ICE#153615
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
enthropy7:fix-ice-generics-of-crate-node

Conversation

@enthropy7
Copy link
Copy Markdown
Contributor

@enthropy7 enthropy7 commented Mar 9, 2026

when note_and_explain_type_err is called during dyn-compatibility checking, the body_owner_def_id can be CRATE_DEF_ID because ObligationCause::dummy() uses it as a placeholder when there is no real body owner. calling generics_of on CRATE_DEF_ID caused an ICE because the crate root is a module and modules don't have generics. the fix guards all generics_of call sites by checking whether body_owner_def_id is CRATE_DEF_ID before attempting to look up generics, and skips the type parameter span labeling in that case.

i also added regression test with original ice reproduce

fixes #152335

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 9, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 9, 2026

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 14 candidates

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch 2 times, most recently from 948df5b to 8aec809 Compare March 9, 2026 17:38
Copy link
Copy Markdown
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think overall this is not pretty fool-proof (what if someone else decides to call tcx.generics_of(body_owner_def_id) down the line, in the function or somewhere else?) and also requires quite some involved changes for an ICE that doesn't happen quite often.

I wonder whether we should simply return and not attempt to add additional information if the obligation cause is dummy.

I wonder whether we can try to make the original obligation cause not dummy.

View changes since this review

Comment on lines +104 to +106
if !tcx.is_impl_trait_in_trait(proj.def_id) && body_generics.is_some() =>
{
let param = tcx.generics_of(body_owner_def_id).type_param(p, tcx);
let param = body_generics.unwrap().type_param(p, tcx);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !tcx.is_impl_trait_in_trait(proj.def_id) && body_generics.is_some() =>
{
let param = tcx.generics_of(body_owner_def_id).type_param(p, tcx);
let param = body_generics.unwrap().type_param(p, tcx);
if !tcx.is_impl_trait_in_trait(proj.def_id) && let Some(body_generics) = body_generics =>
{
let param = body_generics.type_param(p, tcx);

};
if !sp.contains(p_span) {
diag.span_label(p_span, format!("{expected}this type parameter"));
if let Some(generics) = body_generics {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be (ty::Param(p), _) | (_, ty::Param(p)) if let Some(generics) = body_generics =>

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 29, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 29, 2026
@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 8aec809 to 7e8b18f Compare March 29, 2026 06:10
@enthropy7
Copy link
Copy Markdown
Contributor Author

enthropy7 commented Mar 29, 2026

ok so my first uproach won't work, i went with guarding generics_of call sites, as early return turned out too aggressive, it suppressed useful notes/helps for async blocks and closures that don't need generics at all.
guard in suggest_constraint for the same reason.

regarding making the obligation cause non-dummy: ObligationCause::dummy() comes from receiver_is_dispatchable in dyn_compatibility.rs, and there's no real body owner in that context, so I think guarding in error reporting is the right place for now.

@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 7e8b18f to 0d9419c Compare March 29, 2026 07:53
@rustbot

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 0d9419c to 3917a5e Compare April 3, 2026 13:28
@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 3917a5e to 4651565 Compare April 3, 2026 13:39
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

Some changes occurred in src/tools/compiletest

cc @jieyouxu

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

@rustbot rustbot added A-CI Area: Our Github Actions CI A-compiletest Area: The compiletest test runner A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Apr 3, 2026
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Apr 3, 2026
@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 4651565 to 4931206 Compare April 3, 2026 15:15
@rustbot

This comment has been minimized.

@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits PR has merge commits, merge with caution. labels Apr 3, 2026
@enthropy7 enthropy7 force-pushed the fix-ice-generics-of-crate-node branch from 4931206 to f1a4ae0 Compare April 3, 2026 15:18
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

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.

@enthropy7
Copy link
Copy Markdown
Contributor Author

ok so, i completely changed approach. body_owner_def_id is now Option<DefId> throughout note_and_explain_type_err and all its helpers (suggest_constraint, expected_projection, point_at_associated_type). CRATE_DEF_ID check happens once at the call site in mod.rs, and the Option propagates naturally. compiler won't let anyone call generics_of without handling None first. in fact, during rebase a new generics_of(body_owner_def_id) call site had been added upstream, and it was immediately caught as a compile error rather than a silent ICE.

@enthropy7
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 3, 2026
@fee1-dead
Copy link
Copy Markdown
Member

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 5, 2026

📌 Commit f1a4ae0 has been approved by fee1-dead

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 5, 2026
rust-bors bot pushed a commit that referenced this pull request Apr 6, 2026
Rollup of 4 pull requests

Successful merges:

 - #152853 (deny-by-default & report in deps `uninhabited_static`)
 - #153615 (fixed generics of crate node ICE)
 - #154744 (Remove `Clone` impl for `StableHashingContext`.)
 - #154841 (add regression test for Redundant memory strores with mut parameters …)
@rust-bors rust-bors bot merged commit 2aa5ef1 into rust-lang:main Apr 6, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Apr 6, 2026
rust-timer added a commit that referenced this pull request Apr 6, 2026
Rollup merge of #153615 - enthropy7:fix-ice-generics-of-crate-node, r=fee1-dead

fixed generics of crate node ICE

when `note_and_explain_type_err` is called during dyn-compatibility checking, the `body_owner_def_id` can be `CRATE_DEF_ID` because `ObligationCause::dummy()` uses it as a placeholder when there is no real body owner. calling `generics_of `on `CRATE_DEF_ID `caused an ICE because the crate root is a module and modules don't have generics. the fix guards all `generics_of` call sites by checking whether `body_owner_def_id` is `CRATE_DEF_ID` before attempting to look up generics, and skips the type parameter span labeling in that case.

i also added regression test with original ice reproduce

fixes #152335
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 7, 2026
Rollup of 4 pull requests

Successful merges:

 - rust-lang/rust#152853 (deny-by-default & report in deps `uninhabited_static`)
 - rust-lang/rust#153615 (fixed generics of crate node ICE)
 - rust-lang/rust#154744 (Remove `Clone` impl for `StableHashingContext`.)
 - rust-lang/rust#154841 (add regression test for Redundant memory strores with mut parameters …)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: Our Github Actions CI A-compiletest Area: The compiletest test runner A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ICE]: generics_of: unexpected node kind Crate(Mod..

4 participants