-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Remove Rvalue::Len again. #146564
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
Remove Rvalue::Len again. #146564
Conversation
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in src/tools/clippy cc @rust-lang/clippy This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @vakaras Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to constck cc @fee1-dead Some changes occurred in match lowering cc @Nadrieril Some changes occurred to the CTFE machinery This PR changes rustc_public |
3114e1b
to
7613fdb
Compare
This comment has been minimized.
This comment has been minimized.
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.
Thanks for picking this up!
Put a couple of thoughts for you to think about, but r=me even without them if you think they're not needed.
// EMIT_MIR combine_array_len.norm2.InstSimplify-after-simplifycfg.diff | ||
fn norm2(x: [f32; 2]) -> f32 { | ||
// CHECK-LABEL: fn norm2( | ||
// CHECK-NOT: Len( |
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.
I think this also needs CHECK-NOT: PtrMetadata
or something? Since of course there's no Len
now.
// actual = len(place) | ||
self.cfg.push_assign(block, source_info, actual, Rvalue::Len(place)); | ||
let length_op = self.len_of_slice_or_array(block, place, test.span, source_info); | ||
self.cfg.push_assign(block, source_info, actual, Rvalue::Use(length_op)); |
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.
Now that this can be _3 = 4_usize;
instead of _3 = Len(_2);
, should we have a MIR building test that shows that?
This PR was rebased onto a different master 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. |
Thanks! Looks great. @bors r+ rollup=iffy |
Remove Rvalue::Len again. Now that we have `RawPtrKind::FakeForPtrMetadata`, we can reimplement `Rvalue::Len` using `PtrMetadata(&raw const (fake) place)`. r? `@scottmcm`
Rollup of 14 pull requests Successful merges: - #142807 (libtest: expose --fail-fast as an unstable command-line option) - #144871 (Stabilize `btree_entry_insert` feature) - #145071 (Update the minimum external LLVM to 20) - #145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - #145660 (initial implementation of the darwin_objc unstable feature) - #145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - #146259 (Suggest removing Box::new instead of unboxing it) - #146410 (Iterator repeat: no infinite loop for `last` and `count`) - #146460 (Add tidy readme) - #146552 (StateTransform: Do not renumber resume local.) - #146564 (Remove Rvalue::Len again.) - #146581 (Detect attempt to use var-args in closure) - #146588 (tests/run-make: Update list of statically linked musl targets) - #146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146564 - cjgillot:mir-nolen, r=scottmcm Remove Rvalue::Len again. Now that we have `RawPtrKind::FakeForPtrMetadata`, we can reimplement `Rvalue::Len` using `PtrMetadata(&raw const (fake) place)`. r? ``@scottmcm``
Relevant upstream PR: - rust-lang/rust#146564 (Remove Rvalue::Len again.) Resolves: model-checking#4365
Relevant upstream PR: - rust-lang/rust#146564 (Remove Rvalue::Len again.) Resolves: #4365 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
Rollup of 14 pull requests Successful merges: - rust-lang/rust#142807 (libtest: expose --fail-fast as an unstable command-line option) - rust-lang/rust#144871 (Stabilize `btree_entry_insert` feature) - rust-lang/rust#145071 (Update the minimum external LLVM to 20) - rust-lang/rust#145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - rust-lang/rust#145660 (initial implementation of the darwin_objc unstable feature) - rust-lang/rust#145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - rust-lang/rust#146259 (Suggest removing Box::new instead of unboxing it) - rust-lang/rust#146410 (Iterator repeat: no infinite loop for `last` and `count`) - rust-lang/rust#146460 (Add tidy readme) - rust-lang/rust#146552 (StateTransform: Do not renumber resume local.) - rust-lang/rust#146564 (Remove Rvalue::Len again.) - rust-lang/rust#146581 (Detect attempt to use var-args in closure) - rust-lang/rust#146588 (tests/run-make: Update list of statically linked musl targets) - rust-lang/rust#146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
Now that we have
RawPtrKind::FakeForPtrMetadata
, we can reimplementRvalue::Len
usingPtrMetadata(&raw const (fake) place)
.r? @scottmcm