Skip to content

Conversation

@Coekjan
Copy link
Contributor

@Coekjan Coekjan commented Oct 2, 2024

When a slice is ref more than once, current suggestion given by manual_slice_size_calculation is wrong. For example:

let s: &[i32] = &[1, 2][..];
let ss: &&[i32] = &s;  // <-----

let _ = size_of::<i32>() * ss.len();

clippy now suggests:

- let _ = size_of::<i32>() * ss.len();
+ let _ = size_of_val(ss);

However, this can result in calculating the size of &[i32], instead of [i32] (this wrong suggestion also leads to size_of_ref warning: https://rust-lang.github.io/rust-clippy/master/index.html#/size_of_ref )

Now I am sending this PR to fix this bug, so that clippy will suggest (some deref added):

- let _ = size_of::<i32>() * ss.len();
+ let _ = size_of_val(*ss);

As I am not familiar with current clippy code-base, please correct me if I am not doing well or I can do it better :)

changelog: [manual_slice_size_calculation]: fix a bug when a slice is ref more than once.

@rustbot
Copy link
Collaborator

rustbot commented Oct 2, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) some time within the next two weeks.

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 Oct 2, 2024
@Coekjan Coekjan force-pushed the fix-slice-size-calc-on-ref-ref branch from f8ba12e to 2080634 Compare October 3, 2024 04:47
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@dswij
Copy link
Member

dswij commented Oct 13, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2024

📌 Commit 2080634 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 13, 2024

⌛ Testing commit 2080634 with merge 6a281e9...

@bors
Copy link
Contributor

bors commented Oct 13, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 6a281e9 to master...

@bors bors merged commit 6a281e9 into rust-lang:master Oct 13, 2024
8 checks passed
@Coekjan Coekjan deleted the fix-slice-size-calc-on-ref-ref branch October 13, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants