Skip to content

fix: Do not ICE on normalization failure of an extern static item #149210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 25, 2025
Merged

fix: Do not ICE on normalization failure of an extern static item #149210

merged 1 commit into from
Nov 25, 2025
+52 −4

Conversation

@ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Nov 22, 2025

Fixes #148161

@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 Nov 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2025

r? @jdonszelmann

rustbot has assigned @jdonszelmann.
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

jdonszelmann
jdonszelmann reviewed Nov 24, 2025
View reviewed changes
compiler/rustc_hir_analysis/src/check/wfcheck.rs Outdated

let is_foreign_item = tcx.is_foreign_item(item_id);

let forbid_unsized = !is_foreign_item || {
Copy link
Contributor

@jdonszelmann jdonszelmann Nov 24, 2025

Choose a reason for hiding this comment

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

Even though it was already done this way, I quite dislike the block on the right hand side of the or. Could you rewrite this condition to be a bit clearer?

jdonszelmann
jdonszelmann reviewed Nov 24, 2025
View reviewed changes
compiler/rustc_hir_analysis/src/check/wfcheck.rs Outdated
item_ty,
&ObligationCause::dummy(),
|ty| match tcx
.try_normalize_erasing_regions(wfcx.infcx.typing_env(wfcx.param_env), ty)
Copy link
Contributor

@jdonszelmann jdonszelmann Nov 24, 2025

Choose a reason for hiding this comment

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

This is an area of the compiler I'm still learning about, so I asked @BoxyUwU for her opinion to also learn from that (<3). To summarize our discussion:

Though this works the erasing_regions part is a bit awkward and probably unnecessary. It also skips some error reporting, hence we get a Result back. Though we think this probably almost never goes into the Err() case, it might be better to use wfcx.normalize instead. If there are diagnostics, it would then also emit those.

Having said that, maybe it leads to duplicate diagnostics so let me know if that works out or not.

jdonszelmann
jdonszelmann requested changes Nov 24, 2025
View reviewed changes
Copy link
Contributor

@jdonszelmann jdonszelmann left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

almost there I think! let me know how it goes and I'll give it another pass soon :)

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 24, 2025
ShoyuVanilla
ShoyuVanilla commented Nov 24, 2025
View reviewed changes
ShoyuVanilla
ShoyuVanilla commented Nov 24, 2025
View reviewed changes
ShoyuVanilla
ShoyuVanilla commented Nov 24, 2025
View reviewed changes
Copy link
Member Author

@ShoyuVanilla ShoyuVanilla left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

@rustbot ready

View changes since this review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 24, 2025
@jdonszelmann
Copy link
Contributor

jdonszelmann commented Nov 25, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 25, 2025

📌 Commit cf46682 has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors 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 Nov 25, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 25, 2025
Merged
bors added a commit that referenced this pull request Nov 25, 2025
@bors
Auto merge of #149322 - matthiaskrgr:rollup-uf0hmx6, r=matthiaskrgr
80d8f29 
Rollup of 8 pull requests

Successful merges:

 - #147736 (Stabilize `asm_cfg`)
 - #148652 (Cleanup and refactor FnCtxt::report_no_match_method_error)
 - #149167 (skip checking supertraits in object candidate for `NormalizesTo` goal)
 - #149210 (fix: Do not ICE on normalization failure of an extern static item)
 - #149268 (add implementation-internal namespace for globalasm)
 - #149274 (Fix invalid link generation for type alias methods)
 - #149302 (Fix comment wording in simplify_comparison_integral.rs)
 - #149305 (Simplify OnceCell Clone impl)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2f566a8 into rust-lang:main Nov 25, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 25, 2025
rust-timer added a commit that referenced this pull request Nov 25, 2025
@rust-timer
Unrolled build for #149210
0079b8c 
Rollup merge of #149210 - ShoyuVanilla:issue-148161, r=jdonszelmann

fix: Do not ICE on normalization failure of an extern static item

Fixes #148161
@ShoyuVanilla ShoyuVanilla deleted the issue-148161 branch November 26, 2025 00:11
github-actions bot pushed a commit to rust-lang/compiler-builtins that referenced this pull request Nov 27, 2025
@bors
Auto merge of #149322 - matthiaskrgr:rollup-uf0hmx6, r=matthiaskrgr
e3ff390 
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#147736 (Stabilize `asm_cfg`)
 - rust-lang/rust#148652 (Cleanup and refactor FnCtxt::report_no_match_method_error)
 - rust-lang/rust#149167 (skip checking supertraits in object candidate for `NormalizesTo` goal)
 - rust-lang/rust#149210 (fix: Do not ICE on normalization failure of an extern static item)
 - rust-lang/rust#149268 (add implementation-internal namespace for globalasm)
 - rust-lang/rust#149274 (Fix invalid link generation for type alias methods)
 - rust-lang/rust#149302 (Fix comment wording in simplify_comparison_integral.rs)
 - rust-lang/rust#149305 (Simplify OnceCell Clone impl)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdonszelmann jdonszelmann jdonszelmann requested changes

Assignees

@jdonszelmann jdonszelmann

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.93.0

Development

Successfully merging this pull request may close these issues.

ICE: Failed to normalize Alias(Projection, AliasTy...

4 participants

@ShoyuVanilla @rustbot @jdonszelmann @bors