Skip to content
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

Disable vec_box when using different allocators #11780

Merged
merged 8 commits into from Nov 9, 2023

Conversation

Jacherr
Copy link
Contributor

@Jacherr Jacherr commented Nov 8, 2023

Fixes #7114

This PR disables the vec_box lint when the Box and Vec use different allocators (but not when they use the same - custom - allocator).

For example - Vec<Box<i32, DummyAllocator>> will disable the lint, and Vec<Box<i32, DummyAllocator>, DummyAllocator> will not disable the lint.

In addition, the applicability of this lint has been changed to Unspecified due to the automatic fixes potentially breaking code such as the following:

fn foo() -> Vec<Box<i32>> { // -> Vec<i32>
  vec![Box::new(1)]
}

It should be noted that the if_chain->let-chains fix has also been applied to this lint, so the diff does contain many changes.

changelog: disable vec_box lint when using nonstandard allocators

@rustbot
Copy link
Collaborator

rustbot commented Nov 8, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (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 Nov 8, 2023
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

Welcome to Clippy 👋. The changes look good to me, there is one small additional tests I would like to see and then we can merge this :D

tests/ui/vec_box_sized.rs Show resolved Hide resolved
@xFrednet
Copy link
Member

xFrednet commented Nov 9, 2023

Looks good to me, thank you for the swift update :D

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 9, 2023

📌 Commit eabc64f has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 9, 2023

⌛ Testing commit eabc64f with merge 6be0f74...

@bors
Copy link
Collaborator

bors commented Nov 9, 2023

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

@bors bors merged commit 6be0f74 into rust-lang:master Nov 9, 2023
5 checks passed
@Jacherr Jacherr deleted the vec-allocator-nolint branch November 9, 2023 23:58
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.

vec_box lint should not fire if the allocators between the Vec and Box are different
4 participants