chore: to_lowercase -> to_lowercase_cow #4030
Conversation
|
@dyc3 There are still places which are cannot easily migrated to Cow version. For now I just want to see if there is any perf improvement. |
crates/biome_string_case/src/lib.rs
Outdated
| fn to_lowercase_cow(&self) -> Cow<Self> { | ||
| panic!("OsStr is not supported") | ||
| } |
There was a problem hiding this comment.
I am not sure what todo here. OsStr does not support to_lowercase. Implementing it manually is hard and we don't really need it.
Is leaving as is fine? Do we need separate trait?
There was a problem hiding this comment.
I suppose a separate trait would be safest.
There was a problem hiding this comment.
I split it into two separate traits, shame trait-alias are not stable so only one trait could be imported for both. We could use trait-set macro but I imagine introducing new crate for this would be overkill.
CodSpeed Performance ReportMerging #4030 will degrade performances by 7.41%Comparing Summary
Benchmarks breakdown
|
7695433 to
df730d4
Compare
Summary
Follow up to #4014
Implement to_lowercase_cow and use it instead to_lowercase. Enforce future usage with clippy rule.
There are still some to_lowercase left which are trickier to migrate, those and the clippy enforcement will be done with separate PR.Test Plan
All tests should pass.