-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Conversation
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.
cc @lcnr @kylematsuda apparently this is something that can happen in some places and needs some extra care when adding new subst calls :) |
There was a problem hiding this 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
@bors r=compiler-errors rollup |
…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
@@ -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(); |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@compiler-errors suggested it in #106095 (comment)
There was a problem hiding this comment.
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 🤔
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 anEarlyBinder<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