Normalize capture place `ty`s to prevent ICE by JohnTitor · Pull Request #152165 · rust-lang/rust

JohnTitor · 2026-02-05T12:16:16Z

Fixes#151579
Fixes#120811
r? @lcnr

lcnr · 2026-02-08T20:05:09Z

+ HirPlaceBase::Upvar(upvar_id) => {
+ ty::TypingEnv::post_analysis(tcx, upvar_id.closure_expr_id)
+ }
+ _ => ty::TypingEnv::fully_monomorphized(),


This is hacky and subtly wrong in two ways.
even if the type is not from an upvar, we still need the right param_env to normalize
we use this during MIR build, so it should probably use TypingMode::Borrowck or whatever
see https://rustc-dev-guide.rust-lang.org/typing-parameter-envs.html

Ah I see, I've switched to use try_structurally_resolve_ty as mentioned in the issue: 176bf7d (this PR)
This way, we can avoid deep normalization here, right?

lcnr

ah, I didn't see we always get these types from the typeck_results. We should deeply normalize them in writeback and then assume that they are normalized afterwards

View changes since this review

lcnr · 2026-02-12T11:21:45Z

 }

+ fn try_structurally_resolve_place_ty(&self, span: Span, place: &mut Place<'tcx>) {
+ place.base_ty = self.try_structurally_resolve_type(span, place.base_ty);


use deeply_normalize here. We deeply normalize types in the writeback results, so I think we should do the same here

Just replacing didn't work so I had to do something like this instead: 750cd53
Is this what you meant for?

can you use at.deeply_normalize with both solvers?

I've tried but had to construct TraitEngine there which seems an unusual way (I haven't seen such a way in other places).
Instead, I guess using ObligationCtxt might be better here, other places do this way and it shoudl call at.deeply_normalize internally: a7ea420
How about this?

lcnr · 2026-02-13T13:59:05Z

 UpvarMigrationInfo::CapturingPrecise {
 source_expr: capture.info.path_expr_id,
- var_name: capture.to_string(self.tcx),
+ var_name: self.place_to_string_for_migration(capture),


can we move this normalization even further up so that catpure.place is already deeply normalized and we don't have to do so here?

Moved: c22ffdb
I wonder if we should reduce scope by if next_solver as we now normalize stuff more generally, i.e. other than emitting lint rust_2021_incompatible_closure_captures and it would bring unnecessary cost for most cases 🤔

lcnr · 2026-02-14T18:33:02Z

 }
 }

+ fn try_structurally_resolve_place_ty(&self, span: Span, place: &mut Place<'tcx>) {


deeply_normalize should normalize arbitrary TypeFoldable things. Is Place type foldable?
Also, change this function name :>

Ah, you're right, it's TypeFoldable. Addressed: 7def3bc
And I noticed we have another win, it would fix #120811 (N.B. crashes/120911 is a dup of 120811, so I removed it).

lcnr · 2026-02-15T12:30:46Z

I messed up by reviewing your commit directly yesterday, see c22ffdb#r177216090

JohnTitor · 2026-02-16T12:29:33Z

@lcnr Sorry, I missed it 🙏
I think I addressed your comments: ecdea2c
I need to call normalize_capture_place before capture_information.push to prevent ICEs.

rustbot · 2026-02-17T11:32:37Z

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.

JohnTitor · 2026-02-17T11:33:19Z

(rebase mistake, sorry for pinging)

lcnr · 2026-02-26T17:37:31Z

+ let at = self.at(&cause, self.param_env);
+ match solve::deeply_normalize_with_skipped_universes_and_ambiguous_coroutine_goals(
+ at,
+ place.clone(),


i guess to return some place in the error case?

lcnr · 2026-02-26T17:39:09Z

@bors try @rust-timer queue

Normalize capture place `ty`s to prevent ICE

rust-bors · 2026-02-26T19:48:43Z

☀️ Try build successful (CI)
Build commit: 00ebd52 (00ebd52fd0610fe0a77b0caf7ebfb506b3be5e2e, parent: bb779a91568ac1ee0b8a9dcb6b69219ef30b18a3)

rust-timer · 2026-02-26T20:44:08Z

Finished benchmarking commit (00ebd52): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[3.1%, 3.1%]	1

Cycles

Results (secondary -3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 480.119s -> 482.867s (0.57%)
Artifact size: 395.65 MiB -> 397.67 MiB (0.51%)

lcnr · 2026-02-27T15:31:29Z

@bors r+ rollup

rust-bors · 2026-02-27T15:31:33Z

📌 Commit dddbf96 has been approved by lcnr

It is now in the queue for this repository.

@lcnr

Normalize capture place `ty`s to prevent ICE Fixesrust-lang#151579Fixesrust-lang#120811 r? @lcnr

@lcnr

Normalize capture place `ty`s to prevent ICE Fixesrust-lang#151579Fixesrust-lang#120811 r? @lcnr

…uwer Rollup of 12 pull requests Successful merges: - #153211 (`rust-analyzer` subtree update) - #149027 (Improve cross-crate trait impl param mismatch suggestions ) - #152730 (add field representing types) - #153136 (Correctly handle `#[doc(alias = "...")]` attribute on inlined reexports) - #152165 (Normalize capture place `ty`s to prevent ICE) - #152615 (refactor 'valid for read/write' definition: exclude null) - #153109 (Fix LegacyKeyValueFormat report from docker build: aarch64-gnu-debug) - #153172 (Fix comment about placeholders) - #153187 (Fix ICE when macro-expanded extern crate shadows std) - #153190 (Don't allow subdiagnostic to use variables from their parent) - #153200 (Remove redundant clone) - #153216 (mark two polonius tests as known-bug)

@lcnr

Rollup merge of #152165 - JohnTitor:issue-151579, r=lcnr Normalize capture place `ty`s to prevent ICE Fixes#151579Fixes#120811 r? @lcnr

…uwer Rollup of 12 pull requests Successful merges: - rust-lang/rust#153211 (`rust-analyzer` subtree update) - rust-lang/rust#149027 (Improve cross-crate trait impl param mismatch suggestions ) - rust-lang/rust#152730 (add field representing types) - rust-lang/rust#153136 (Correctly handle `#[doc(alias = "...")]` attribute on inlined reexports) - rust-lang/rust#152165 (Normalize capture place `ty`s to prevent ICE) - rust-lang/rust#152615 (refactor 'valid for read/write' definition: exclude null) - rust-lang/rust#153109 (Fix LegacyKeyValueFormat report from docker build: aarch64-gnu-debug) - rust-lang/rust#153172 (Fix comment about placeholders) - rust-lang/rust#153187 (Fix ICE when macro-expanded extern crate shadows std) - rust-lang/rust#153190 (Don't allow subdiagnostic to use variables from their parent) - rust-lang/rust#153200 (Remove redundant clone) - rust-lang/rust#153216 (mark two polonius tests as known-bug)

rustbot assigned lcnr Feb 5, 2026

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 5, 2026

This comment has been minimized.
Sign in to view

theemathas mentioned this pull request Feb 6, 2026
ICE : internal error: entered unreachable code#152205
Open

lcnr closed this Feb 8, 2026

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2026

lcnr reopened this Feb 8, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2026

lcnr reviewed Feb 8, 2026
View reviewed changes

JohnTitor force-pushed the issue-151579 branch from a544343 to 176bf7dCompare February 11, 2026 10:53

This comment has been minimized.
Sign in to view

JohnTitor requested a review from lcnr February 11, 2026 11:04

lcnr reviewed Feb 12, 2026
View reviewed changes

JohnTitor requested a review from lcnr February 12, 2026 22:32

lcnr reviewed Feb 13, 2026
View reviewed changes

JohnTitor force-pushed the issue-151579 branch from a7ea420 to c22ffdbCompare February 14, 2026 11:44

This comment has been minimized.
Sign in to view

lcnr reviewed Feb 14, 2026
View reviewed changes

JohnTitor force-pushed the issue-151579 branch from c22ffdb to 88036b0Compare February 15, 2026 04:11

This comment has been minimized.
Sign in to view

JohnTitor force-pushed the issue-151579 branch from 88036b0 to 7def3bcCompare February 15, 2026 04:11

JohnTitor force-pushed the issue-151579 branch from 7def3bc to ecdea2cCompare February 16, 2026 12:22

This comment has been minimized.
Sign in to view

lcnr reviewed Feb 16, 2026
View reviewed changes

Comment threadcompiler/rustc_hir_typeck/src/upvar.rs Outdated

lcnr reviewed Feb 26, 2026
View reviewed changes

This comment has been minimized.
Sign in to view

rust-borsBot pushed a commit that referenced this pull request Feb 26, 2026
Auto merge of#152165- JohnTitor:issue-151579, r=<try>
00ebd52
Normalize capture place `ty`s to prevent ICE

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2026

This comment has been minimized.
Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2026

rust-borsBot 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 Feb 27, 2026

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 28, 2026
Rollup merge ofrust-lang#152165- JohnTitor:issue-151579, r=lcnr
a2f6273
Normalize capture place `ty`s to prevent ICE Fixesrust-lang#151579Fixesrust-lang#120811 r? @lcnr

JonathanBrouwer mentioned this pull request Feb 28, 2026
Rollup of 9 pull requests #153208
Closed

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 28, 2026
Rollup merge ofrust-lang#152165- JohnTitor:issue-151579, r=lcnr
42fd093
Normalize capture place `ty`s to prevent ICE Fixesrust-lang#151579Fixesrust-lang#120811 r? @lcnr

This was referenced Feb 28, 2026
Rollup of 11 pull requests #153212
Closed
Rollup of 12 pull requests #153217
Merged

rust-borsBot merged commit 0381e24 into rust-lang:mainFeb 28, 2026
12 checks passed

rustbot added this to the 1.96.0 milestone Feb 28, 2026

rust-timer added a commit that referenced this pull request Feb 28, 2026
Unrolled build for#152165
25d2366
Rollup merge of #152165 - JohnTitor:issue-151579, r=lcnr Normalize capture place `ty`s to prevent ICE Fixes#151579Fixes#120811 r? @lcnr

JohnTitor deleted the issue-151579 branch March 5, 2026 06:45

lqd mentioned this pull request Mar 12, 2026
[ICE]: broken MIR in DefId: NoSolution #153789
Closed

Uh oh!

Uh oh!

Conversation

JohnTitor commented Feb 5, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

JohnTitorFeb 11, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

lcnr commented Feb 15, 2026

Uh oh!

This comment has been minimized.

JohnTitor commented Feb 16, 2026

Uh oh!

Uh oh!

rustbot commented Feb 17, 2026

Uh oh!

JohnTitor commented Feb 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr commented Feb 26, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-borsBot commented Feb 26, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Feb 26, 2026

Overall result: no relevant changes - no action needed

Uh oh!

lcnr commented Feb 27, 2026

Uh oh!

rust-borsBot commented Feb 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JohnTitor commented Feb 5, 2026•
edited
Loading

JohnTitorFeb 11, 2026•
edited
Loading

lcnr left a comment •
edited by rustbot
Loading