Skip to content

Use eq_ignore_ascii_case to avoid heap alloc in detect_confuse_type #145152

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

Merged
merged 1 commit into from
Aug 10, 2025

Conversation

xizheyin
Copy link
Member

@xizheyin xizheyin commented Aug 9, 2025

A small optimization has been made, using to_ascii_lowercase() instead of to_lowercase().to_string().

r? compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 9, 2025

rustc_errors::emitter was changed

cc @Muscraft

@lqd
Copy link
Member

lqd commented Aug 9, 2025

Such error paths are cold, but sure, why not.

r? lqd I'll send it to bors when it's green.

@rustbot rustbot assigned lqd and unassigned petrochenkov Aug 9, 2025
@Kivooeo
Copy link
Member

Kivooeo commented Aug 9, 2025

It'll be green in 3... 2... 1...

Okay, it takes longer than I thought

@lqd
Copy link
Member

lqd commented Aug 9, 2025

only miri left, good enough for me

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 9, 2025

📌 Commit a8b0f75 has been approved by lqd

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 9, 2025
@marmeladema
Copy link
Contributor

Would https://doc.rust-lang.org/std/primitive.str.html#method.eq_ignore_ascii_case be even better in this case?

@lqd
Copy link
Member

lqd commented Aug 9, 2025

it's not a str it's a char, so https://doc.rust-lang.org/std/primitive.char.html#method.eq_ignore_ascii_case but why not. Even better is left in the eye of the beholder, because it's just a shorthand for what's in this PR.

but since it's not in a rollup yet, let's do that @xizheyin

@bors r-

Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 9, 2025
Use `to_ascii_lowercase` to avoid heap alloc in `detect_confuse_type`

A small optimization has been made, using `to_ascii_lowercase()` instead of `to_lowercase().to_string()`.

r? compiler
@xizheyin
Copy link
Member Author

xizheyin commented Aug 9, 2025

Yeah, no problem

@lqd
Copy link
Member

lqd commented Aug 9, 2025

It's in a rollup now...., race condition, or bors not seeing edits, no worries. Another PR will be fine, not in this one @xizheyin

@jieyouxu
Copy link
Member

jieyouxu commented Aug 9, 2025

Re-approving explicitly, bors state was weird.

@bors r=lqd

@bors
Copy link
Collaborator

bors commented Aug 9, 2025

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Aug 9, 2025

📌 Commit a8b0f75 has been approved by lqd

It is now in the queue for this repository.

bors added a commit that referenced this pull request Aug 9, 2025
Rollup of 8 pull requests

Successful merges:

 - #144739 (Use new public libtest `ERROR_EXIT_CODE` constant in rustdoc)
 - #145089 (Improve error output when a command fails in bootstrap)
 - #145112 ([win][arm64ec] Partial fix for raw-dylib-link-ordinal on Arm64EC)
 - #145135 (Stabilize `duration_constructors_lite` feature)
 - #145146 (remove `P`)
 - #145152 (Use `to_ascii_lowercase` to avoid heap alloc in `detect_confuse_type`)
 - #145156 (Override custom Cargo `build-dir` in bootstrap)
 - #145160 (Change days-threshold to 28 in [behind-upstream])

Failed merges:

 - #145145 (some `derive_more` refactors)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 10, 2025
Use `to_ascii_lowercase` to avoid heap alloc in `detect_confuse_type`

A small optimization has been made, using `to_ascii_lowercase()` instead of `to_lowercase().to_string()`.

r? compiler
@Zalathar
Copy link
Contributor

The rollup failed. Based on the conversation above, I'm assuming you would prefer to modify this PR instead of merging it as-is.

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 10, 2025
@xizheyin xizheyin force-pushed the detect-confusion-type branch from a8b0f75 to cf1a1b7 Compare August 10, 2025 04:36
@xizheyin xizheyin changed the title Use to_ascii_lowercase to avoid heap alloc in detect_confuse_type Use eq_ignore_ascii_case to avoid heap alloc in detect_confuse_type Aug 10, 2025
@xizheyin
Copy link
Member Author

@rustbot ready

CI is green :)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 10, 2025
@lqd
Copy link
Member

lqd commented Aug 10, 2025

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 10, 2025

📌 Commit cf1a1b7 has been approved by lqd

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2025
bors added a commit that referenced this pull request Aug 10, 2025
Rollup of 7 pull requests

Successful merges:

 - #144553 (Rehome 32 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`)
 - #145064 (Add regression test for `saturating_sub` bounds check issue)
 - #145121 (bootstrap: `x.py dist rustc-src` should keep LLVM's siphash)
 - #145150 (Replace unsafe `security_attributes` function with safe `inherit_handle` alternative)
 - #145152 (Use `eq_ignore_ascii_case` to avoid heap alloc in `detect_confuse_type`)
 - #145200 (mbe: Fix typo in attribute tracing)
 - #145222 (Fix typo with paren rustc_llvm/build.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 733568a into rust-lang:master Aug 10, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 10, 2025
rust-timer added a commit that referenced this pull request Aug 10, 2025
Rollup merge of #145152 - xizheyin:detect-confusion-type, r=lqd

Use `eq_ignore_ascii_case` to avoid heap alloc in `detect_confuse_type`

A small optimization has been made, using `to_ascii_lowercase()` instead of `to_lowercase().to_string()`.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants