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.

Sign up for GitHub

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

Add RefCell::take #71398

Merged
merged 3 commits into from
May 3, 2020
Merged

Add RefCell::take #71398

merged 3 commits into from
May 3, 2020

Conversation

ThinkChaos
Copy link
Contributor

Add RefCell::take to match Cell and Option.
I also changed a couple of calls to .replace to .take.

Tracking issue is #71395.

This is my first contribution, please tell me if there's anything I could improve, thanks!

@ThinkChaos ThinkChaos force-pushed the feat_refcell_take branch 2 times, most recently from 85ad89d to 0fb7907 Compare April 21, 2020 20:03
@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 21, 2020
@therealprof
Copy link
Contributor

I don't think we should have the #[inline] attributes added here (unless there's a good reason to do it?) but the rest looks very good and useful to me.

@ThinkChaos
Copy link
Contributor Author

I removed the inline attributes :)

@bors
Copy link
Contributor

bors commented Apr 25, 2020

☔ The latest upstream changes (presumably #71439) made this pull request unmergeable. Please resolve the merge conflicts.

@ThinkChaos
Add RefCell::take
f121f09 
In the same vein as `Cell::take` and `Option::take`.
@ThinkChaos
Use Cell::take in a couple places
4ea83bf
LukasKalbertodt
LukasKalbertodt suggested changes May 2, 2020
View reviewed changes
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Thanks for your PR and please excuse my late response. Looks like a useful addition! Just one inline comment.

As this is a tiny feature, I think the doc example is sufficient as test. The PR adding Cell::take (#39287) also didn't add any tests.

src/libcore/cell.rs Show resolved Hide resolved
@ThinkChaos
Copy link
Contributor Author

No worries, thanks for the review!

I copied the docs from RefCell::replace.

@LukasKalbertodt
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 3, 2020

📌 Commit 4a79424 has been approved by LukasKalbertodt

@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 May 3, 2020
@ThinkChaos ThinkChaos mentioned this pull request May 3, 2020
Closed
2 tasks
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 3, 2020
@Dylan-DPC
Rollup merge of rust-lang#71398 - ThinkChaos:feat_refcell_take, r=Luk…
d0bbd25 
…asKalbertodt

Add `RefCell::take`

Add `RefCell::take` to match `Cell` and `Option`.
I also changed a couple of calls to `.replace` to `.take`.

Tracking issue is rust-lang#71395.

This is my first contribution, please tell me if there's anything I could improve, thanks!
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request May 3, 2020
Closed
@bors
Copy link
Contributor

bors commented May 3, 2020

⌛ Testing commit 4a79424 with merge bba798dd3eaf4c68b9f1ef3cb223dbacf9f764c1...

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 3, 2020
@Dylan-DPC
Rollup merge of rust-lang#71398 - ThinkChaos:feat_refcell_take, r=Luk…
18e8b9d 
…asKalbertodt

Add `RefCell::take`

Add `RefCell::take` to match `Cell` and `Option`.
I also changed a couple of calls to `.replace` to `.take`.

Tracking issue is rust-lang#71395.

This is my first contribution, please tell me if there's anything I could improve, thanks!
This was referenced May 3, 2020
Closed
Merged
@Dylan-DPC-zz
Copy link

included in rollup, yielding

@bors retry

bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2020
@bors
Auto merge of rust-lang#71853 - Dylan-DPC:rollup-4qi6ry9, r=Dylan-DPC
ea733c3 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#71398 (Add `RefCell::take`)
 - rust-lang#71663 (Fix exceeding bitshifts not emitting for assoc. consts (properly this time, I swear!))
 - rust-lang#71726 (Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability)
 - rust-lang#71808 (Add long error explanation for E0539)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented May 3, 2020

☔ The latest upstream changes (presumably #71853) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 3, 2020
@bors bors merged commit 6f5de87 into rust-lang:master May 3, 2020
@ThinkChaos ThinkChaos deleted the feat_refcell_take branch November 22, 2020 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukasKalbertodt LukasKalbertodt LukasKalbertodt approved these changes

Assignees

@LukasKalbertodt LukasKalbertodt

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ThinkChaos @therealprof @bors @LukasKalbertodt @Dylan-DPC-zz @JohnTitor