Skip to content

Delete Rvalue::Len 🎉 #134330

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 22, 2024
Merged

Delete Rvalue::Len 🎉 #134330

merged 2 commits into from
Dec 22, 2024

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Dec 15, 2024 •
edited
Loading

Everything's moved to PtrMetadata, so we can get rid of the Len variant now.

Depends on #134326, so draft until that lands Ready!

r? mir

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 15, 2024
scottmcm
scottmcm commented Dec 15, 2024
View reviewed changes
compiler/rustc_mir_build/src/build/matches/test.rs Outdated
// actual = len(place)
self.cfg.push_assign(block, source_info, actual, Rvalue::Len(place));
let actual = self.len_of_slice_or_array(block, place, test.span, source_info);
Copy link
Member Author

Choose a reason for hiding this comment

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

One functional change here, reusing this helper from #133734

scottmcm
scottmcm commented Dec 15, 2024
View reviewed changes
tests/mir-opt/building/custom/arrays.rs
mir! {
{
let x = [5_i32; C];
let c = Len(x);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was here testing the custom-mir Len, which is removed here, so I deleted the test.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Dec 15, 2024

☔ The latest upstream changes (presumably #134332) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@scottmcm
Delete Rvalue::Len
5ba54c9 
Everything's moved to `PtrMetadata` instead.
@scottmcm
Update clippy
29889fd
@scottmcm scottmcm closed this Dec 22, 2024
@scottmcm scottmcm reopened this Dec 22, 2024
@scottmcm scottmcm marked this pull request as ready for review December 22, 2024 16:27
@rustbot
Copy link
Collaborator

rustbot commented Dec 22, 2024

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@matthewjasper
Copy link
Contributor

Discussion of this was here for anyone wondering: https://rust-lang.zulipchat.com/#narrow/channel/189540-t-compiler.2Fwg-mir-opt/topic/.60RValue.3A.3ALen.60.20vs.20.60UnOp.3A.3APtrMetadata.60

@bors r+ rollup=iffy
(iffy for perf)

@bors
Copy link
Collaborator

bors commented Dec 22, 2024

📌 Commit 29889fd has been approved by matthewjasper

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 Dec 22, 2024
@bors
Copy link
Collaborator

bors commented Dec 22, 2024

⌛ Testing commit 29889fd with merge e108481...

@bors
Copy link
Collaborator

bors commented Dec 22, 2024

☀️ Test successful - checks-actions
Approved by: matthewjasper
Pushing e108481 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 22, 2024
@bors bors merged commit e108481 into rust-lang:master Dec 22, 2024
13 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 22, 2024
@scottmcm scottmcm deleted the no-more-rvalue-len branch December 22, 2024 21:57
@bors bors mentioned this pull request Dec 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@matthewjasper matthewjasper

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
7 participants
@scottmcm @rust-log-analyzer @bors @rustbot @matthewjasper @rust-timer @Kobzol