single_char_pattern suggests using char instead of str for `starts_with`

[david-monroe]

Summary

warning: single-character string constant used as pattern
   --> src/core/data/mod.rs:314:31
    |
314 |         if string.starts_with("[") {
    |                               ^^^ help: try using a `char` instead: `'['`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
    = note: `#[warn(clippy::single_char_pattern)]` on by default

and then

Why is this bad?

Performing these methods using a char is faster than using a str.

But the opposite is true for starts_with.

Lint Name

single_char_pattern

Reproducer

No response

Version

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-unknown-linux-gnu
release: 1.73.0
LLVM version: 17.0.2

Additional Labels

No response

[llogiq]

But the opposite is true for starts_with.

Always being careful to accept or reject such positions without evidence, I wrote and ran a small benchmark testing with 'a', 'ö' (a 2-byte character) and a unicode smiley (which is 4 bytes IIRC). Just benchmarking &str::starts_with(self, char) vs. &str::starts_with(self, &str) of the same content. Without further ado, here are the results (I don't give error bounds, as the measurements were all within +/- 0.05 ns):

input	char	str
a	2.4052 ns	4.4219 ns
ö	2.4132 ns	4.3975 ns
😃	3.0733 ns	4.4460 ns

Based on that measurement, I cannot support your position. So I'll close this issue, feel free to reopen if you come up with a benchmark that supports your claims.

[david-monroe]

I ran your benchmark unchanged and got very different values:

char/0                  time:   [2.4864 ns 2.4870 ns 2.4877 ns]
char/1                  time:   [11.194 ns 11.195 ns 11.196 ns]
char/2                  time:   [13.212 ns 13.212 ns 13.213 ns]

&str/0                  time:   [2.4847 ns 2.4854 ns 2.4862 ns]
&str/1                  time:   [2.7149 ns 2.7205 ns 2.7289 ns]
&str/2                  time:   [2.7192 ns 2.7208 ns 2.7226 ns]

But there was no need to run such a benchmark in the first place. The starts_with impl for char just converts the char to a &str and then calls into the &str impl of starts_with. Therefore it can never be faster than the &str version.

[david-monroe]

I don't have permission to re-open so please go ahead and do it yourself.

[y21]

But the opposite is true for starts_with.

For ASCII search patterns, it compiles to identical machine code: https://godbolt.org/z/MqcaP69zh
I don't know how your example code in the original post could have been slower with a char.

For non-ASCII, it seems like char is currently slower, but that honestly sounds like a missed optimization opportunity in the standard library or in the compiler.
Keep in mind that this lint only ever triggers when the argument is a hardcoded string literal.
There's no reason to do any UTF-8 encoding at runtime when the char value is known.

The Pattern trait and its is_prefix_of function seems like it could totally be improved to nudge llvm in the direction to optimize it properly.

I don't see how either of them should be slower or faster than the other for a hardcoded searcher, so I do agree that we should just not lint at all

[david-monroe]

For ASCII search patterns, it compiles to identical machine code: https://godbolt.org/z/MqcaP69zh

Then, even in this case, the description of the clippy lint is false because it claims that the char version is faster.

However there are clearly cases where the &str version is faster and no cases where the char version is faster. So always using &str is the obvious choice.

For non-ASCII, it seems like char is currently slower, but that honestly sounds like a missed optimization opportunity in the standard library or in the compiler.

If clippy is supposed to be optimization agnostic, then this lint should be removed entirely. A sufficiently smart optimizer will convert the slower version to the faster one in any case.