Skip to content

new lint: unnecessary_reserve - #14114

Closed
wowinter13 wants to merge 7 commits into
rust-lang:masterfrom
wowinter13:new-lint-unnecessary-reserve
Closed

new lint: unnecessary_reserve#14114
wowinter13 wants to merge 7 commits into
rust-lang:masterfrom
wowinter13:new-lint-unnecessary-reserve

Conversation

@wowinter13

@wowinter13 wowinter13 commented Jan 30, 2025 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

resurrection of #10157

fixes #8982

changelog: [unnecessary_reserve]: add new lint unnecessary_reserve

@rustbot

rustbot commented Jan 30, 2025

Copy link
Copy Markdown
Collaborator

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 30, 2025
@wowinter13 wowinter13 changed the title new lint: unnecessary_reserve new lint: unnecessary_reserve Jan 30, 2025
samueltardieu
samueltardieu requested changes Jan 30, 2025
View reviewed changes
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
@wowinter13

wowinter13 commented Jan 30, 2025

Copy link
Copy Markdown
Contributor Author

@samueltardieu Samuel, really appreciate your review and points!
Will go through them tomorrow and make sure to apply the lessons

samueltardieu
samueltardieu reviewed Feb 4, 2025
View reviewed changes

@samueltardieu samueltardieu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you include a test (maybe I've missed it) where extend() happens before reserve(), to make sure the lint doesn't trigger?

Comment thread tests/ui/unnecessary_reserve.rs
Comment thread tests/ui/unnecessary_reserve.rs Outdated
Comment thread tests/ui/unnecessary_reserve.rs Outdated
Comment thread tests/ui/unnecessary_reserve.fixed Outdated
samueltardieu
samueltardieu reviewed Feb 15, 2025
View reviewed changes
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs Outdated
Comment thread clippy_lints/src/unnecessary_reserve.rs
samueltardieu
samueltardieu reviewed Feb 15, 2025
View reviewed changes
Comment thread tests/ui/unnecessary_reserve.rs

// do not lint
vec.reserve(1);
vec.extend([1]);

@samueltardieu samueltardieu Feb 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Btw, error markers are now required in order for tests to pass. If you rebase over master, you will see examples in all the UI test files.

@samueltardieu

samueltardieu commented Feb 15, 2025

Copy link
Copy Markdown
Member

Also, is there any reason why this lint is not under the methods module?

@wowinter13
wowinter13 force-pushed the new-lint-unnecessary-reserve branch from 0bf67e5 to 37dfd0a Compare March 1, 2025 15:50
@rustbot

rustbot commented Mar 31, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly d28d234) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 31, 2025
@Jarcho

Jarcho commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

r? samueltardieu

@wowinter13 do you plan to continue working on this?

@rustbot rustbot assigned samueltardieu and unassigned Jarcho Sep 17, 2025
@samueltardieu samueltardieu added the needs-fcp PRs that add, remove, or rename lints and need an FCP label Oct 17, 2025
@Jarcho

Jarcho commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closing since there was no response. @wowinter13 if you wish to return to working on this feel free to open a new PR.

@Jarcho Jarcho closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samueltardieu samueltardieu samueltardieu requested changes
@flip1995 flip1995 flip1995 left review comments
@Jarcho Jarcho Awaiting requested review from Jarcho

Assignees

@samueltardieu samueltardieu

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unnecessary Vec/VecDeque reserve lint

5 participants

@wowinter13 @rustbot @samueltardieu @Jarcho @flip1995