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

std: Derive `Default` for `io::Cursor` #60234

Merged
merged 1 commit into from May 10, 2019

Conversation

Projects
None yet
10 participants
@tesaguri
Copy link
Contributor

commented Apr 24, 2019

I think this change is quite obvious, so made it insta-stable, but I won't insist on that.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

r? @withoutboats

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

@jonas-schievink

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Unfortunately trait impls can't be made unstable, so this can only be landed as an insta-stable impl

@hellow554

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

So this would make Cursor::<Vec<u8>>::default() possible, but is that really something one would do? Don't you rather want to wrap an existing Vec, because you want that content?

@tesaguri

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

I'm dealing with a S: futures::Stream, so the content is not immediately available and I need a placeholder. I could just write Cursor::new(/* ... */) if S::Item were a concrete type (Cursor), but actually that is a type parameter with a trait bound S::Item: bytes::Buf + Default.

@erikdesjardins

This comment has been minimized.

Copy link

commented Apr 24, 2019

You should be able to use Cursor::new(Default::default()) anywhere Cursor::default() would work.
(not that I think this change shouldn't be made; I'm ambivalent)

@Centril Centril added the relnotes label Apr 25, 2019

@Centril Centril added this to the 1.36 milestone Apr 25, 2019

@Centril

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

@Amanieu

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

I think this is fine, but just in case anyone has objections...

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

commented Apr 29, 2019

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot

This comment has been minimized.

Copy link

commented Apr 29, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot

This comment has been minimized.

Copy link

commented May 9, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Centril

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@bors r=Amanieu rollup

@bors

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

📌 Commit 902904a has been approved by Amanieu

Centril added a commit to Centril/rust that referenced this pull request May 9, 2019

Rollup merge of rust-lang#60234 - tesaguri:cursor-default, r=Amanieu
std: Derive `Default` for `io::Cursor`

I think this change is quite obvious, so made it insta-stable, but I won't insist on that.

@Centril Centril referenced this pull request May 9, 2019

Merged

Rollup of 8 pull requests #60683

bors added a commit that referenced this pull request May 10, 2019

Auto merge of #60683 - Centril:rollup-p05qh5d, r=Centril
Rollup of 8 pull requests

Successful merges:

 - #59348 (Clean up and add tests for slice drop shims)
 - #60188 (Identify when a stmt could have been parsed as an expr)
 - #60234 (std: Derive `Default` for `io::Cursor`)
 - #60618 (Comment ext::tt::transcribe)
 - #60648 (Skip codegen for one UI test with long file path)
 - #60671 (remove unneeded `extern crate`s from build tools)
 - #60675 (Remove the old await! macro)
 - #60676 (Fix async desugaring providing wrong input to procedural macros.)

Failed merges:

r? @ghost

@bors bors merged commit 902904a into rust-lang:master May 10, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@tesaguri tesaguri deleted the tesaguri:cursor-default branch May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.