Skip to content
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.

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

Also erase substs for new infcx in pin move error #107422

Merged
merged 1 commit into from
Jan 29, 2023

Conversation

Noratrieb
Copy link
Member

The code originally correctly erased the regions of the type it passed to the newly created infcx. But after the fn_sig query was made to return an EarlyBinder<T>, some substs that were around were substituted there without erasing their regions. They were then passed into the newly cerated infcx, which caused the ICE.

Fixes #107419

r? compiler-errors who reviewed the original PR adding this diagnostic

The code originally correctly erased the regions of the type it passed
to the newly created infcx. But after the `fn_sig` query was made to
return an `EarlyBinder<T>`, some substs that were around were
substituted there without erasing their regions. They were then passed
into the newly cerated infcx, which caused the ICE.
@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 Jan 28, 2023
@Noratrieb
Copy link
Member Author

cc @lcnr @kylematsuda apparently this is something that can happen in some places and needs some extra care when adding new subst calls :)

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me ci green

@Noratrieb
Copy link
Member Author

@bors r=compiler-errors rollup

@bors
Copy link
Contributor

bors commented Jan 28, 2023

📌 Commit 832751f has been approved by compiler-errors

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 Jan 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#106618 (Disable `linux_ext` in wasm32 and fortanix rustdoc builds.)
 - rust-lang#107097 (Fix def-use dominance check)
 - rust-lang#107154 (library/std/sys_common: Define MIN_ALIGN for m68k-unknown-linux-gnu)
 - rust-lang#107397 (Gracefully exit if --keep-stage flag is used on a clean source tree)
 - rust-lang#107401 (remove the usize field from CandidateSource::AliasBound)
 - rust-lang#107413 (make more pleasant to read)
 - rust-lang#107422 (Also erase substs for new infcx in pin move error)
 - rust-lang#107425 (Check for missing space between fat arrow and range pattern)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5ff6cdc into rust-lang:master Jan 29, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 29, 2023
@Noratrieb Noratrieb deleted the erase-the-ice branch January 29, 2023 10:24
@@ -1128,8 +1128,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"{place_name} {partially_str}moved due to this method call{loop_message}",
),
);

let infcx = tcx.infer_ctxt().build();
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we even create a new infcx here instead of using self.infcx?

Copy link
Member

Choose a reason for hiding this comment

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

Regions already solve so can_eq ices

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

remember that someone told me the reason behind this before and I forgot 😅

might be nice to add a comment about this here, but apart from that this does feel like a pretty good solution 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

ICE: Out of bounds when relating regions
5 participants