reduce single_char_pattern to only lint on ascii chars

[llogiq]

This should mostly fix the single_char_pattern lint, because with a single byte, the optimizer will usually see through the char-to-string-expansion and single loop iteration. This fixes #11675 and #8111.

Update: As per the meeting on November 28th, 2023, we voted to also downgrade the lint to pedantic.

---

changelog: downgrade [single_char_pattern] to pedantic

[rustbot]

r? @Alexendoo

(rustbot has picked a reviewer for you, use r? to override)

[Alexendoo]

Maybe it's time to retire this lint? Which is faster seems unpredictable and subject to change across versions, it seems like a better avenue would be opening issues in rustc for any case where one is slower than the other

[david-monroe]

This is not true in the current implementation. If you want to be implementation agnostic, you might as well write the opposite:

Suggested change

	/// Performing these methods using a `char` can be faster than
	/// using a `str` because it needs one less indirection.
	/// Performing these methods using a `str` can be faster than
	/// using a `char` because it needs one less conversion.

[llogiq]

It apparently depends on a lot of factors. The char based version stores the UTF8 data inline, whereas the &str one stores a ref to the caller-supplied string slice instead, so if the Searcher instantiation is const, it gets by with one less pointer.

Of course e.g. the fastest starts_with for an ascii character is haystack.as_bytes().get(0) == Some(&(c as u8)), dropping below a nanosecond in my benchmarks.

[flip1995]

Maybe it's time to retire this lint?

At least I think we should move it to style, as it no longer affects performance IIUC.

[bors]

☔ The latest upstream changes (presumably #12030) made this pull request unmergeable. Please resolve the merge conflicts.

[llogiq]

@Alexendoo should I rebase or close?

[Alexendoo]

Sorry for the delay, given the decision to move it to pedantic do we still want to make the multibyte change? If we're dropping the perf aspect of the lint to be more a stylistic one I think it would make sense to lint multibyte characters still & modify the lint description to be less perf focused

[llogiq]

The problem I see with that is that for multibyte inputs using a char may actually hurt performance. So yes, I believe we should reduce the lint scope even as we move it to pedantic.

[xFrednet]

Hey @llogiq, this is a ping from triage, since there hasn't been any activity in some time. Are you still planning to continue this implementation?

If you have any questions, you're always welcome to ask them in this PR or on Zulip.

@rustbot author

[llogiq]

I'll take another look during the next week.

[Alexendoo]

Ignoring multibyte chars as a known issue sounds reasonable to avoid a perf regression if it's still there, ideally we'd have a rustc issue to link to

I still think the description should be changed though, the perf angle doesn't hold up to me

[llogiq]

@Alexendoo / @xFrednet I rebased the implementation and updated the docs. r?

[xFrednet]

Looks good to me.

Thank you for the update :)

@bors r+

[bors]

📌 Commit 54de78a has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit 54de78a with merge fc6dfeb...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing fc6dfeb to master...