Skip to content

proc_macro: Generalize FromIterator impl - #49734

Merged
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:generalize-token-stream
Apr 12, 2018
Merged

proc_macro: Generalize FromIterator impl#49734
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:generalize-token-stream

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

While never intended to be stable we forgot that trait impls are insta-stable!
This construction of FromIterator wasn't our first choice of how to stabilize
the impl but our hands are tied at this point, so revert back to the original
definition of FromIterator before #49597

Closes#49725

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2018

@dtolnaydtolnay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we get away with

implFromIterator<TokenTree>forTokenStreamimplFromIterator<TokenStream>forTokenStream

for now instead of dealing with T: Into<TokenStream>?

@alexcrichton

Copy link
Copy Markdown
MemberAuthor

@dtolnay we could try yeah but it would still be a technical breaking change due to custom From impls.

@nikomatsakis

Copy link
Copy Markdown
Contributor

maybe worth a try though

@SimonSapin

SimonSapin commented Apr 6, 2018

Copy link
Copy Markdown
Contributor

To be clear, I filed #49725 to make sure we deal with it knowingly in case other projects are impacted, but I don’t mind if the resolution is "no change". It’s an easy fix for Servo, and that crate is not part of the subset used by Firefox that needs to build on the Stable channel.

While never intended to be stable we forgot that trait impls are insta-stable!
This construction of `FromIterator` wasn't our first choice of how to stabilize
the impl but our hands are tied at this point, so revert back to the original
definition of `FromIterator` before rust-lang#49597Closesrust-lang#49725
@alexcrichton
alexcrichtonforce-pushed the generalize-token-stream branch from 63dc0a2 to d985344CompareApril 6, 2018 20:01
@alexcrichton

Copy link
Copy Markdown
MemberAuthor

Ok, I've added just impl FromIterator<TokenStream> for TokenStream and we can see how far we get with that

@nikomatsakis

Copy link
Copy Markdown
Contributor

@alexcrichton did you want to do a crater run or something?

@nikomatsakis

Copy link
Copy Markdown
Contributor

Not sure what is expected path here

@alexcrichton

alexcrichton commented Apr 10, 2018

Copy link
Copy Markdown
MemberAuthor

I'm ok landing this and seeing if there's still any breakage. The damage is already done on nightly so it may be a bit late for a crater run :(

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 11, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit d985344 has been approved by nikomatsakis

@borsbors 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 Apr 11, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 11, 2018
…m, r=nikomatsakis
proc_macro: Generalize `FromIterator` impl
While never intended to be stable we forgot that trait impls are insta-stable!
This construction of `FromIterator` wasn't our first choice of how to stabilize
the impl but our hands are tied at this point, so revert back to the original
definition of `FromIterator` before rust-lang#49597Closesrust-lang#49725
@kennytmkennytm mentioned this pull request Apr 11, 2018
bors added a commit that referenced this pull request Apr 11, 2018
Rollup of 14 pull requests
Successful merges:
- #49525 (Use sort_by_cached_key where appropriate)
- #49575 (Stabilize `Option::filter`.)
- #49614 (in which the non-shorthand patterns lint keeps its own counsel in macros)
- #49665 (Small nits to make couple of tests pass on mips targets.)
- #49781 (add regression test for #16223 (NLL): use of collaterally moved value)
- #49795 (Properly look for uninhabitedness of variants in niche-filling check)
- #49809 (Stop emitting color codes on TERM=dumb)
- #49856 (Do not uppercase-lint #[no_mangle] statics)
- #49863 (fixed typo)
- #49857 (Fix "fp" target feature for AArch64)
- #49849 (Add --enable-debug flag to musl CI build script)
- #49734 (proc_macro: Generalize `FromIterator` impl)
- #49730 (Fix ICE with impl Trait)
- #48270 (Replace `structurally_resolved_type` in casts check.)
Failed merges:
@bors
bors merged commit d985344 into rust-lang:masterApr 12, 2018
@alexcrichton
alexcrichton deleted the generalize-token-stream branch April 20, 2018 06:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@alexcrichton@rust-highfive@nikomatsakis@SimonSapin@bors@dtolnay