Skip to content

Add clear_then_shrink lint#16766

Open
QuarticCat wants to merge 1 commit intorust-lang:masterfrom
QuarticCat:master
Open

Add clear_then_shrink lint#16766
QuarticCat wants to merge 1 commit intorust-lang:masterfrom
QuarticCat:master

Conversation

@QuarticCat
Copy link
Copy Markdown

Summary

Add clear_then_shrink, a new clippy::perf lint for consecutive clear(); shrink_to_fit(); calls on standard library containers.

The lint suggests replacing the pattern with std::mem::take(...) for Vec, VecDeque, String, PathBuf, OsString, HashMap, HashSet, and BinaryHeap.

Tests

Added UI tests for supported containers, &mut receivers, non-linted cases, and MSRV handling.

changelog: new lint: [clear_then_shrink]

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 26, 2026

r? @dswij

rustbot has assigned @dswij.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@QuarticCat
Copy link
Copy Markdown
Author

QuarticCat commented Mar 26, 2026

Quick benchmark:

use std::collections::HashSet;

use criterion::{BatchSize, Criterion, criterion_group, criterion_main};

#[inline(never)]
fn vec_clear(x: &mut Vec<u64>) {
    x.clear();
    x.shrink_to_fit();
}

#[inline(never)]
fn vec_take(x: &mut Vec<u64>) {
    std::mem::take(x);
}

#[inline(never)]
fn set_clear(x: &mut HashSet<u64>) {
    x.clear();
    x.shrink_to_fit();
}

#[inline(never)]
fn set_take(x: &mut HashSet<u64>) {
    std::mem::take(x);
}

fn clear_vs_take(c: &mut Criterion) {
    for size in [128, 1024, 8196] {
        let mut data = vec![0u64; size];
        rand::fill(&mut data[..]);

        c.bench_function(&format!("vec/clear+shrink_to_fit/{size}"), |b| {
            b.iter_batched(
                || data.to_vec(),
                |mut v| vec_clear(&mut v),
                BatchSize::SmallInput,
            );
        });

        c.bench_function(&format!("vec/std::mem::take/{size}"), |b| {
            b.iter_batched(
                || data.to_vec(),
                |mut v| vec_take(&mut v),
                BatchSize::SmallInput,
            );
        });

        c.bench_function(&format!("hashset/clear+shrink_to_fit/{size}"), |b| {
            b.iter_batched(
                || data.iter().copied().collect::<HashSet<_>>(),
                |mut set| set_clear(&mut set),
                BatchSize::SmallInput,
            );
        });

        c.bench_function(&format!("hashset/std::mem::take/{size}"), |b| {
            b.iter_batched(
                || data.iter().copied().collect::<HashSet<_>>(),
                |mut set| set_take(&mut set),
                BatchSize::SmallInput,
            );
        });
    }
}

criterion_group!(benches, clear_vs_take);
criterion_main!(benches);

For Vec, there's basically no difference. For HashSet, std::mem::take is faster.

From the assembly (https://godbolt.org/z/TdncMhzhj):

  • vec_take is tail call while vec_clear is not.
  • set_clear has an extra memset call while set_take does not.
  • set_take resets hash seed, this could be a concern.

In addition to potential performance benefits, std::mem::take is shorter and cleaner. Maybe it could be classified as clippy::style.

@QuarticCat
Copy link
Copy Markdown
Author

This lint only searches for consecutive clear + shrink_to_fit calls. Otherwise it's unclear to put std::mem::take in what place.

@xtqqczze
Copy link
Copy Markdown
Contributor

Would *x = Default::default() be more idiomatic than std::mem::take(x) when the result isn’t used?

@dswij
Copy link
Copy Markdown
Member

dswij commented Mar 29, 2026

r? clippy

@rustbot rustbot assigned ada4a and unassigned dswij Mar 29, 2026
@QuarticCat
Copy link
Copy Markdown
Author

Would *x = Default::default() be more idiomatic than std::mem::take(x) when the result isn’t used?

Yeah, good catch. I could change to that if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP 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