Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

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

Jump to bottom

Open

yotamofek wants to merge 1 commit into rust-lang:master

from yotamofek:pr/library/from-iter-char-string

Open

Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

yotamofek wants to merge 1 commit into rust-lang:master from yotamofek:pr/library/from-iter-char-string

+32 −0

Conversation

yotamofek

Copy link

Contributor

@yotamofek yotamofek commented May 23, 2025

Wanted to collect ascii chars into a String while working on #141369 , and was surprised these impls don't exist. Seems to me to be simply oversight.

BTW, I only added impl FromIterator<ascii::Char> for Cow<'_, str>, without a corresponding FromIterator<&Char> impl, because there's no existing impl for FromIterator<&char>, but that might be oversight too.

cc #110998

@rustbot rustbot assigned joboet

May 23, 2025

@rustbot

Copy link

Collaborator

rustbot commented May 23, 2025

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review T-libs labels

May 23, 2025

@rust-log-analyzer

This comment has been minimized.

@yotamofek yotamofek force-pushed the pr/library/from-iter-char-string branch from 4e9a021 to 8365332 Compare

May 23, 2025 13:51

joboet

joboet reviewed

May 23, 2025

View reviewed changes

Copy link

Member

@joboet joboet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes complete sense to me! This will need T-libs-api approval though...

Implementation-wise, I think it'd be good to go through Vec's FromIterator implementation. That way we get to take advantage of all of its nice specialisations...

r? libs-api

library/alloc/src/string.rs Outdated

Comment on lines 2380 to 2382

let mut buf = String::new();

buf.extend(iter);

buf

Copy link

Member

@joboet joboet May 23, 2025 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something along the lines of

Suggested change

let mut buf = String::new();

buf.extend(iter);

buf

let mut buf = iter.into_iter().map(Char::to_u8).collect();

unsafe { String::from_utf8_unchecked(buf) }

Copy link

Contributor Author

@yotamofek yotamofek May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that's clever, thanks!

library/alloc/src/string.rs Outdated

Comment on lines 2390 to 2392

let mut buf = String::new();

buf.extend(iter);

buf

Copy link

Member

@joboet joboet May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(likewise)

@rustbot rustbot added the T-libs-api label

May 23, 2025

@rustbot rustbot assigned joshtriplett and unassigned joboet

May 23, 2025

@yotamofek


 Add FromIterator impls for ascii::Chars to Strings

8569bb2

@yotamofek yotamofek force-pushed the pr/library/from-iter-char-string branch from 8365332 to 8569bb2 Compare

May 23, 2025 15:21

@the8472 the8472 mentioned this pull request

May 27, 2025

ACP: str::chunks with chunks being &str rust-lang/libs-team#592

Closed

@yotamofek

Copy link

Contributor Author

yotamofek commented Jun 18, 2025

Hey, @joshtriplett , friendly ping :) I'd love to get this looked at! Thank you 🙏

Labels

S-waiting-on-review T-libs T-libs-api

5 participants

@yotamofek @rustbot @rust-log-analyzer @joboet @joshtriplett

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

Are you sure you want to change the base?

Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

Conversation

@yotamofek yotamofek commented May 23, 2025

Uh oh!

rustbot commented May 23, 2025

Uh oh!

This comment has been minimized.

@joboet joboet left a comment

Choose a reason for hiding this comment

Uh oh!

@joboet joboet May 23, 2025 •

edited

Loading

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@yotamofek yotamofek May 23, 2025

Choose a reason for hiding this comment

Uh oh!

@joboet joboet May 23, 2025

Choose a reason for hiding this comment

Uh oh!

yotamofek commented Jun 18, 2025

Uh oh!

Uh oh!

Add FromIterator impls for ascii::Chars to Strings #141445

Are you sure you want to change the base?

Add FromIterator impls for ascii::Chars to Strings #141445

Conversation

@yotamofek yotamofek commented May 23, 2025

Uh oh!

rustbot commented May 23, 2025

Uh oh!

This comment has been minimized.

@joboet joboet left a comment

Choose a reason for hiding this comment

Uh oh!

@joboet joboet May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@yotamofek yotamofek May 23, 2025

Choose a reason for hiding this comment

Uh oh!

@joboet joboet May 23, 2025

Choose a reason for hiding this comment

Uh oh!

yotamofek commented Jun 18, 2025

Uh oh!

Uh oh!

Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

Add `FromIterator` impls for `ascii::Char`s to `String`s #141445

@joboet joboet May 23, 2025 •

edited

Loading