Skip to content

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

Merged
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/library/from-iter-char-string
Oct 22, 2025
Merged

Add FromIterator impls for ascii::Chars to Strings#141445
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/library/from-iter-char-string

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
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 Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 23, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@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

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 23, 2025
@rustbot rustbot assigned joshtriplett and unassigned joboet May 23, 2025
@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
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 🙏

@Enselic
Copy link
Member

Enselic commented Sep 4, 2025

Triage: Re-rolling reviewer.

r? libs-api

@rustbot rustbot assigned the8472 and unassigned joshtriplett Sep 4, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Sep 25, 2025

Friendly ping to new reviewer @the8472 , thanks!

@yotamofek
Copy link
Contributor Author

yotamofek commented Oct 21, 2025

Re-shuffling reviewer again :)
r? libs-api

@rustbot rustbot assigned BurntSushi and unassigned the8472 Oct 21, 2025
@ChrisDenton
Copy link
Member

ChrisDenton commented Oct 21, 2025

For future reference, it's often a good idea to create an ACP if you want libs-api's attention. But I'll nominate this so someone sees it.

I would only note that adding traits can sometimes be more fraught than it may appear (e.g. it can unexpectedly break crates due to inference failures, etc) but I think this should be fine seeing as ascii::Char is still unstable.

@ChrisDenton ChrisDenton added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Oct 21, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Oct 21, 2025 •
edited
Loading

For future reference, it's often a good idea to create an ACP if you want libs-api's attention. But I'll nominate this so someone sees it.

I would only note that adding traits can sometimes be more fraught than it may appear (e.g. it can unexpectedly break crates due to inference failures, etc) but I think this should be fine seeing as ascii::Char is still unstable.

Thanks. I was under the misconceived notion that adding trait impls on top of an unstable API didn't warrant an ACP :)

@ChrisDenton
Copy link
Member

ChrisDenton commented Oct 21, 2025

For sure. String is a stable type though, which would be my only concern.

@the8472
Copy link
Member

the8472 commented Oct 21, 2025 •
edited
Loading

We discussed this during today's libs-api meeting. Since Char is still unstable we're ok with adding this for now and review it later during stabilization.

@bors r=the8472,joshtriplett

As followup it could also be added to Box<str>.

@bors
Copy link
Collaborator

bors commented Oct 21, 2025

📌 Commit 8569bb2 has been approved by the8472,joshtriplett

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 Oct 21, 2025
@ChrisDenton ChrisDenton mentioned this pull request Oct 21, 2025
Open
7 tasks
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 22, 2025
Merged
bors added a commit that referenced this pull request Oct 22, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
b2ee1b3 
Rollup of 7 pull requests

Successful merges:

 - #141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - #142339 (Add NonNull pattern types)
 - #147768 (Code refactoring on hir report_no_match_method_error)
 - #147788 (const Cell methods)
 - #147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - #147933 (os_str: Make platform docs more consistent)
 - #147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 664e3b0 into rust-lang:master Oct 22, 2025
6 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 22, 2025
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Oct 23, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
698b2f9 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Oct 27, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
51930ee 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
makai410 pushed a commit to makai410/rustc_public that referenced this pull request Nov 4, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
80008e8 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
makai410 pushed a commit to makai410/rust that referenced this pull request Nov 8, 2025
@bors
Auto merge of rust-lang#147957 - matthiaskrgr:rollup-aqt593u, r=matth…
1486d5e 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang#142339 (Add NonNull pattern types)
 - rust-lang#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang#147788 (const Cell methods)
 - rust-lang#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang#147933 (os_str: Make platform docs more consistent)
 - rust-lang#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
makai410 pushed a commit to makai410/rust that referenced this pull request Nov 10, 2025
@bors
Auto merge of rust-lang#147957 - matthiaskrgr:rollup-aqt593u, r=matth…
42b6509 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang#142339 (Add NonNull pattern types)
 - rust-lang#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang#147788 (const Cell methods)
 - rust-lang#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang#147933 (os_str: Make platform docs more consistent)
 - rust-lang#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
makai410 pushed a commit to makai410/rustc_public that referenced this pull request Nov 16, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
8cd7c95 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
@matthiaskrgr
Rollup merge of rust-lang#141445 - yotamofek:pr/library/from-iter-cha…
cafda41 
…r-string, r=the8472,joshtriplett

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

Wanted to `collect` ascii chars into a `String` while working on rust-lang#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 rust-lang#110998
Kobzol pushed a commit to Kobzol/rustc_codegen_cranelift that referenced this pull request Dec 29, 2025
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
700fbeb 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request Jan 27, 2026
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
16b51d1 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request Jan 27, 2026
@bors
Auto merge of #147957 - matthiaskrgr:rollup-aqt593u, r=matthiaskrgr
58cebee 
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#141445 (Add `FromIterator` impls for `ascii::Char`s to `String`s)
 - rust-lang/rust#142339 (Add NonNull pattern types)
 - rust-lang/rust#147768 (Code refactoring on hir report_no_match_method_error)
 - rust-lang/rust#147788 (const Cell methods)
 - rust-lang/rust#147932 (Create UTF-8 version of `OsStr`/`OsString`)
 - rust-lang/rust#147933 (os_str: Make platform docs more consistent)
 - rust-lang/rust#147948 (PassWrapper: Access GlobalValueSummaryInfo::SummaryList via getter for LLVM 22+)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joboet joboet joboet left review comments

Assignees

@BurntSushi BurntSushi

Labels

I-libs-api-nominated Nominated for discussion during a libs-api team meeting. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.92.0

Development

Successfully merging this pull request may close these issues.

10 participants

@yotamofek @rustbot @rust-log-analyzer @Enselic @ChrisDenton @the8472 @bors @joboet @joshtriplett @BurntSushi

Comments