Skip to content

Suggest std::mem::size_of_val instead of std::mem::size_of_value #10659

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 1 commit into from
Apr 17, 2023
Merged

Suggest std::mem::size_of_val instead of std::mem::size_of_value #10659

merged 1 commit into from
Apr 17, 2023

Conversation

timvisee
Copy link
Contributor

This fixes the incorrect suggestion to use std::mem::size_of_value. It should be std::mem::size_of_val.

changelog: [manual_slice_size_calculation]: suggest std::mem::size_of_val rather than std::mem::size_of_value

@rustbot
Copy link
Collaborator

rustbot commented Apr 17, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 17, 2023
@Manishearth
Copy link
Member

@bors r+

nice catch, thanks!

We should probably also try to make this an applicable suggestion

@bors
Copy link
Contributor

bors commented Apr 17, 2023

📌 Commit 4fb38cf has been approved by Manishearth

It is now in the queue for this repository.

@timvisee timvisee changed the title Rename std::mem::size_of_value to std::mem::size_of_val Suggest std::mem::size_of_val instead of std::mem::size_of_value Apr 17, 2023
@bors
Copy link
Contributor

bors commented Apr 17, 2023

⌛ Testing commit 4fb38cf with merge e57deaa...

@timvisee
Copy link
Contributor Author

We should probably also try to make this an applicable suggestion

I'll look into that in a separate PR.

@bors
Copy link
Contributor

bors commented Apr 17, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing e57deaa to master...

@bors bors merged commit e57deaa into rust-lang:master Apr 17, 2023
@timvisee timvisee mentioned this pull request Apr 17, 2023
Merged
bors added a commit that referenced this pull request Apr 17, 2023
@bors
Auto merge of #10661 - timvisee:manual-slice-size-calc-suggestion, r=…
203c909 
…Manishearth

Add `manual_slice_size_calculation` applicable suggestion

Continuation of #10659 (comment).

This adds applicable suggestions to the `manual_slice_size_calculation` lint:

```
error: manual slice size calculation
  --> $DIR/manual_slice_size_calculation.rs:11:13
   |
LL |     let _ = s_i32.len() * size_of::<i32>(); // WARNING
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)`
   |
   = note: `-D clippy::manual-slice-size-calculation` implied by `-D warnings`
```

changelog: [`manual_slice_size_calculation`]: add machine applicable suggestion
@Alexendoo Alexendoo mentioned this pull request Apr 18, 2023
Merged
bors added a commit that referenced this pull request Apr 18, 2023
@bors
Auto merge of #10667 - Alexendoo:manual-slice-size-calculation-macros…
8099f19 
…, r=llogiq

Ignore `manual_slice_size_calculation` in code from macro expansions

changelog: none, assuming same release as #10659
@kpreid
Copy link
Contributor

kpreid commented May 9, 2023

manual_slice_size_calculation made it to beta without this fix, so this PR should be beta-nominated.

@flip1995 flip1995 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 17, 2023
@flip1995
Copy link
Member

This is the only PR that was nominated for beta backporting. Since this just fixes a help message and not a suggestion, it won't break anything for Clippy users. When updating the code by hand, their IDE should already tell them the correct function name. I don't think that we need a beta backport for just a help message typo fix.

Thanks for nominating this PR. But I don't think it warrants a backport on its own.

@flip1995 flip1995 removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 25, 2023
@kpreid
Copy link
Contributor

kpreid commented May 25, 2023

I agree that it is not very technically necessary; I only thought that the typo will create a poor impression of the Rust project and should be fixed on that basis.

@flip1995
Copy link
Member

I only thought that the typo will create a poor impression of the Rust project

Worse things got into stable Clippy 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth approved these changes

Assignees

@Manishearth Manishearth

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@timvisee @rustbot @Manishearth @bors @kpreid @flip1995