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

Suggest to change std::iter::repeat(s).take(n).collect::<String>() to s.repeat(n) #7260

Closed
wooster0 opened this issue May 21, 2021 · 3 comments · Fixed by #7265
Closed

Suggest to change std::iter::repeat(s).take(n).collect::<String>() to s.repeat(n) #7260

wooster0 opened this issue May 21, 2021 · 3 comments · Fixed by #7265
Labels
A-lint Area: New lints

Comments

@wooster0
Copy link

wooster0 commented May 21, 2021

What it does

It suggests the user to change std::iter::repeat(string).take(n).collect::<String>() to string.repeat(n). It should work without the String type argument to collect too and clippy has to find out based on context whether it does collect to a String.
Additionally, when the argument to std::iter::repeat is a constant char like 'a' it should suggest to change that to "a".repeat(n). I said only constant chars because I'm not sure how well it will work if it does that for variable chars as well.

Categories (optional)

  • Kind: clippy::perf
use criterion::{criterion_group, criterion_main, Criterion};

fn repeat1(count: usize) -> String {
    std::iter::repeat("a").take(count).collect()
}

fn repeat2(count: usize) -> String {
    "a".repeat(count)
}


fn criterion_benchmark(c: &mut Criterion) {
    c.bench_function("std::iter::repeat", |b| b.iter(|| repeat1(100)));
    c.bench_function("str::repeat", |b| b.iter(|| repeat2(100)));
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
std::iter::repeat       time:   [433.62 ns 434.36 ns 435.09 ns]                              
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) low mild
  5 (5.00%) high mild
  1 (1.00%) high severe

str::repeat             time:   [52.540 ns 52.658 ns 52.857 ns]                        
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

It's faster and generates a lot less instructions, in the char case too.
It's also easier to read.

Drawbacks

None.

Example

std::iter::repeat("hello").take(10).collect::<String>();
std::iter::repeat('x').take(10).collect::<String>();

Could be written as:

"hello".repeat(10);
"x".repeat(10);

See also: rust-lang/rust#85538.

@ghost
Copy link

ghost commented May 22, 2021

https://doc.rust-lang.org/std/primitive.str.html#method.repeat
MSRV 1.16
There will be code that uses iter::repeat because it was written before str::repeat was added.

@wooster0
Copy link
Author

Then I suppose Clippy should check if the version is >=1.16.0 if that's a thing.

@ghost
Copy link

ghost commented May 22, 2021

Yes. My last comment was supposed to be a note for the implementer to add that config.

@bors bors closed this as completed in ca570f9 Jun 1, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 20, 2021
Replace some `std::iter::repeat` with `str::repeat`

I noticed that there were some instances where `std::iter::repeat` would be used to repeat a string or a char to take a specific count of it and then collect it into a `String` when `str::repeat` is actually much faster and better for that.

See also: rust-lang/rust-clippy#7260.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 1, 2021
Replace some `std::iter::repeat` with `str::repeat`

I noticed that there were some instances where `std::iter::repeat` would be used to repeat a string or a char to take a specific count of it and then collect it into a `String` when `str::repeat` is actually much faster and better for that.

See also: rust-lang#7260.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant