Skip to content

Remove unsafe_vector_initialization lint #3478

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

Merged
merged 3 commits into from
Dec 3, 2018
Merged

Remove unsafe_vector_initialization lint #3478

merged 3 commits into from
Dec 3, 2018

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Dec 2, 2018

This lint looks for:

let mut vec = Vec::with_capacity(len);
vec.set_len(len);

The suggested replacement is vec![0; len].

This is far too opinionated to be a deny-by-default lint because the performance characteristics of the suggested replacement are totally different.

I am not convinced that this lint has value beyond what deny(unsafe_code) gives you. Unsafe code is unsafe but we should not deny-by-default lint if that's the only reason.

r? collaborators involved in #3365: @flip1995 @oli-obk

This lint looks for:

    let mut vec = Vec::with_capacity(len);
    vec.set_len(len);

The suggested replacement is `vec![0; len]`.

This is far too opinionated to be a deny-by-default lint because the performance
characteristics of the suggested replacement are totally different.

I am not convinced that this lint has value beyond what deny(unsafe_code) gives
you. Unsafe code is unsafe but please don't deny-by-default lint it if that's
the only reason.
@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 2, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Dec 3, 2018

I think we should only take out the unsafe option, since that is indeed wrong.

Instead of moving the lint to restriction, we should fix the lint to not be emitted for set_len at all. The other cases are valid.

@dtolnay
Copy link
Member Author

dtolnay commented Dec 3, 2018

I believe unsafe_vector_initialization only looks for with_capacity followed by set_len with the same argument value between the two. The other vector initialization lints are under a different lint name.

Would you like me to remove unsafe_vector_initialization entirely?

@oli-obk
Copy link
Contributor

oli-obk commented Dec 3, 2018

@dtolnay dtolnay changed the title Downgrade unsafe_vector_initialization to restriction Remove unsafe_vector_initialization lint Dec 3, 2018
@dtolnay
Copy link
Member Author

dtolnay commented Dec 3, 2018

I added unsafe_vector_initialization to deprecated and removed the implementation.

@oli-obk oli-obk merged commit c00210d into rust-lang:master Dec 3, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Dec 3, 2018

Thanks!

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.

3 participants