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

Push a char instead of a str with len one into a String #82022

Merged
merged 1 commit into from
Feb 13, 2021

Conversation

LingMan
Copy link
Contributor

@LingMan LingMan commented Feb 12, 2021

@rustbot modify labels +C-cleanup +T-compiler

@rustbot rustbot added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 12, 2021
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 12, 2021
@jonas-schievink
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 12, 2021

📌 Commit fda71d6 has been approved by jonas-schievink

@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 Feb 12, 2021
@bluss
Copy link
Member

bluss commented Feb 12, 2021

Just curious - the push_str handles string to string data so no encoding needed, in most cases using push_str should be simpler and more minimal; maybe there are other upsides to pushing a char?

@ldm0
Copy link
Contributor

ldm0 commented Feb 12, 2021

Just curious - the push_str handles string to string data so no encoding needed, in most cases using push_str should be simpler and more minimal; maybe there are other upsides to pushing a char?

Check: rust-lang/rust-clippy#5875

@LingMan
Copy link
Contributor Author

LingMan commented Feb 12, 2021

push has a fast-path for ASCII chars that directly pushes the byte to the String's underlying Vec. The encoding as well as the extend_from_slice machinery are skipped entirely. While I didn't run any benchmark, I'd assume that makes push faster than push_str for ASCII-heavy data. Though, you are probably right for chars with 2+ bytes in their UTF-8 representation.

Since we're working on a literal here, it get's optimized down to basically the same assembly anyway.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 12, 2021
Push a `char` instead of a `str` with len one into a String

`@rustbot` modify labels +C-cleanup +T-compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword)
 - rust-lang#81012 (Stabilize the partition_point feature)
 - rust-lang#81479 (Allow casting mut array ref to mut ptr)
 - rust-lang#81506 (HWAddressSanitizer support)
 - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…)
 - rust-lang#81850 (use RWlock when accessing os::env)
 - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id)
 - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String)
 - rust-lang#82023 (Remove unnecessary lint allow attrs on example)
 - rust-lang#82030 (Use `Iterator::all` instead of open-coding it)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ef7c45a into rust-lang:master Feb 13, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 13, 2021
@LingMan LingMan deleted the single_char branch February 13, 2021 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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.

8 participants