-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Refactor: Separate LocalRef
variant for not-evaluated-yet operands
#109582
Conversation
r? @oli-obk (rustbot has picked a reviewer for you, use r? to override) |
@@ -558,6 +558,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||
bug!("using operand local {:?} as place", place_ref); | |||
} | |||
} | |||
LocalRef::PendingOperand => { | |||
bug!("using still-pending operand local {:?} as place", place_ref); |
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.
This is the one place that wasn't already distinguishing between Some
/None
, and near as I can tell that's just because the None
case is impossible -- at least it seems plausible that we should never be using a not-evaluated-yet operand as a place, and I never hit an ICE from a stage2 build nor any codegen tests having added this check.
@bors r+ rollup nice! |
@bors r+ rollup nice! |
💡 This pull request was already approved, no need to approve it again.
|
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#108625 (More config.toml.example cleanups) - rust-lang#109418 (Rename 'src/bootstrap/native.rs' to llvm.rs) - rust-lang#109580 (Remove some stale FIXMEs in new solver) - rust-lang#109582 (Refactor: Separate `LocalRef` variant for not-evaluated-yet operands) - rust-lang#109650 (Remove Nilstrieb from review rotation) - rust-lang#109656 (Update cargo) - rust-lang#109658 (Backport 1.68.1 and 1.68.2 release notes to `master`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
As I was reading through this, I noticed that almost every place that was using this needed to distinguish between Some vs None in the match arm anyway, so thought that separating the cases at the variant level might be clearer instead.
I like how it ended up; let me know what you think!