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.

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

Warn for Vec<Box<T>> where T: Sized #3530

Closed
Kampfkarren opened this issue Dec 11, 2018 · 4 comments · Fixed by #3545
Closed

Warn for Vec<Box<T>> where T: Sized #3530

Kampfkarren opened this issue Dec 11, 2018 · 4 comments · Fixed by #3545
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-complexity Lint: Belongs in the complexity lint group

Comments

@Kampfkarren
Copy link
Contributor

A fellow Rust programmer pointed me to this mistake in my code:

image

I'm quite new to Rust, but is there ever a point where Vec<Box<T: Sized>> is correct?
@sinkuu
Copy link
Contributor

sinkuu commented Dec 11, 2018

For example, it might make sense when T is a very large type. Calling .swap(0, 1) on Vec<T> will memcpy size_of::<T>() * 3, but it would be pointer_size * 3 for Vec<Box<T>>.

@flip1995 flip1995 added A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group good-first-issue These issues are a good way to get started with Clippy labels Dec 12, 2018
Kampfkarren added a commit to Kampfkarren/rust-clippy that referenced this issue Dec 13, 2018
@Kampfkarren
Lint for Vec<Box<T: Sized>> - Closes rust-lang#3530
ab07050
@Kampfkarren Kampfkarren mentioned this issue Dec 13, 2018
Merged
bors added a commit that referenced this issue Dec 14, 2018
@bors
Auto merge of #3545 - Kampfkarren:vec_boxed_sized, r=flip1995
cd74191 
Adds lint for Vec<Box<T: Sized>>

This adds, and subsequently closes #3530. This is the first time I've ever worked with anything remotely close to internal Rust code, so I'm very much unsure about the if_chain! to figure this out!

I can't get rustfmt working on WSL with nightly 2018-12-07:

`error: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is unavailable for download`
@flip1995 flip1995 mentioned this issue Dec 14, 2018
Closed
bors added a commit that referenced this issue Dec 14, 2018
@bors
Auto merge of #3545 - Kampfkarren:vec_boxed_sized, r=flip1995
a416c5e 
Adds lint for Vec<Box<T: Sized>>

This adds, and subsequently closes #3530. This is the first time I've ever worked with anything remotely close to internal Rust code, so I'm very much unsure about the if_chain! to figure this out!

I can't get rustfmt working on WSL with nightly 2018-12-07:

`error: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is unavailable for download`
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue May 5, 2020
@matthiaskrgr
submodules: update clippy from b7a431e to a416c5e
9d15aaf 
Changes:

````
rustup rust-lang/rust#52994
Fix test
Line length fix
Remove references to sized for end users
Remove DUMMY_SP
Add suggestion for replacement
Update lint definitions
Lint for Vec<Box<T: Sized>> - Closes rust-lang#3530
Fix doc_markdown mixed case false positive
question_mark: Suggest Some(opt?) for if-else
redundant_field_names: Do not trigger on path with type params
question_mark: Lint only early returns
question_mark: Fix applicability
Remove obsolete comment
new_without_default, partialeq_ne_impl: Use span_lint_node
Update .stderr after rebase
cargo fmt and remove stabilized feature
Make suggestion Applicability::MachineApplicable
Address review feedback
Extract method
Check array lengths to prevent OOB access
Add suggestion for explicit_write lint
Fix write_with_newline escaping false positive
````
@benma
Copy link

benma commented Dec 26, 2020

Another example: reallocation of the vector when growing it is slower and copies more bytes when the size of T is large.

@h-a-n-a h-a-n-a mentioned this issue Jul 23, 2024
Merged
2 tasks
@wiiznokes
Copy link

Could this warning be deactivate when the struct is large ?

@y21
Copy link
Member

y21 commented Dec 12, 2024

@wiiznokes The lint already checks that the type's size is lower than a threshold to avoid linting on large types. You can override vec-box-size-threshold to a lower value in config.toml (default is 4096 bytes)

@alex-semenyuk alex-semenyuk mentioned this issue Feb 19, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2025
@Alexendoo
Remove Known problems section for vec_box (#14252)
8108bd8 
Remove Known problems section for `vec_box` since
[issue](#3530 (comment))
> Vec<Box<T: Sized>> makes sense if T is a large type

can be handled

> The lint already checks that the type's size is lower than a threshold
to avoid linting on large types.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-complexity Lint: Belongs in the complexity lint group
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adds lint for Vec<Box<T: Sized>> Kampfkarren/rust-clippy
6 participants
@benma @Kampfkarren @sinkuu @flip1995 @y21 @wiiznokes