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

Introduce into_raw_non_null on Rc and Arc #57934

Merged
merged 1 commit into from
Jan 31, 2019
Merged

Introduce into_raw_non_null on Rc and Arc #57934

merged 1 commit into from
Jan 31, 2019

Conversation

dwijnand
Copy link
Member

No description provided.

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2019
@dwijnand dwijnand mentioned this pull request Jan 27, 2019
Closed
@Centril
Copy link
Contributor

Centril commented Jan 27, 2019

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned bluss Jan 27, 2019
Centril
Centril previously requested changes Jan 27, 2019
View reviewed changes
src/liballoc/rc.rs Show resolved Hide resolved
src/liballoc/sync.rs Show resolved Hide resolved
@Centril Centril dismissed their stale review January 27, 2019 20:06

changes done

@alexcrichton
Copy link
Member

Thanks for this! On Box this is called into_raw_non_null, so should the name be updated to match that?

@dwijnand
Copy link
Member Author

On Box this is called into_raw_non_null, so should the name be updated to match that?

I assumed the "raw" in Box's version is because it already had an into_raw method, but I wasn't sure if it was appropriate for Arc/Rc. I also don't know if the rename is worth it, but I'm happy to make the old name conform with the new names or make the new names conform with the old name. Do you have a preference?

@alexcrichton
Copy link
Member

Nah yeah I think we'll want to prioritize consistency across pointer types, and I think that {Arc,Rc}::into_raw should exist as well!

@dwijnand
Copy link
Member Author

dwijnand commented Jan 28, 2019 •
edited
Loading

and I think that {Arc,Rc}::into_raw should exist as well!

I'm blind: I'm using {Arc,Rc}::into_raw in the implementation of into_raw_non_null 🙄🙈...

All done!

@alexcrichton
Copy link
Member

As one final nit, could the tracking issue be pointed to the same one as for Box?

@dwijnand
Copy link
Member Author

Sure!

@dwijnand dwijnand changed the title Introduce into_non_null on Rc and Arc Introduce into_raw_non_null on Rc and Arc Jan 28, 2019
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 28, 2019

📌 Commit 1e57726 has been approved by alexcrichton

@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 Jan 28, 2019
@Centril
Copy link
Contributor

Centril commented Jan 28, 2019

@bors rollup

Centril added a commit to Centril/rust that referenced this pull request Jan 30, 2019
@Centril
Rollup merge of rust-lang#57934 - dwijnand:from-Arc/Rc-to-NonNull, r=…
89f74ac 
…alexcrichton

Introduce into_raw_non_null on Rc and Arc

None
@Centril Centril mentioned this pull request Jan 30, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jan 30, 2019
@Centril
Rollup merge of rust-lang#57934 - dwijnand:from-Arc/Rc-to-NonNull, r=…
e9de86c 
…alexcrichton

Introduce into_raw_non_null on Rc and Arc

None
@Centril Centril mentioned this pull request Jan 30, 2019
Closed
@Centril Centril mentioned this pull request Jan 30, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jan 30, 2019
@Centril
Rollup merge of rust-lang#57934 - dwijnand:from-Arc/Rc-to-NonNull, r=…
f255361 
…alexcrichton

Introduce into_raw_non_null on Rc and Arc

None
Centril added a commit to Centril/rust that referenced this pull request Jan 30, 2019
@Centril
Rollup merge of rust-lang#57934 - dwijnand:from-Arc/Rc-to-NonNull, r=…
1c45d1e 
…alexcrichton

Introduce into_raw_non_null on Rc and Arc

None
@Centril Centril mentioned this pull request Jan 30, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jan 31, 2019
@Centril
Rollup merge of rust-lang#57934 - dwijnand:from-Arc/Rc-to-NonNull, r=…
e8173a6 
…alexcrichton

Introduce into_raw_non_null on Rc and Arc

None
@Centril Centril mentioned this pull request Jan 31, 2019
Merged
bors added a commit that referenced this pull request Jan 31, 2019
@bors
Auto merge of #58016 - Centril:rollup, r=Centril
f40aaa6 
Rollup of 12 pull requests

Successful merges:

 - #57008 (suggest `|` when `,` founds in invalid match value)
 - #57106 (Mark str::trim.* functions as #[must_use].)
 - #57920 (use `SOURCE_DATE_EPOCH` for man page time if set)
 - #57934 (Introduce into_raw_non_null on Rc and Arc)
 - #57971 (SGX target: improve panic & exit handling)
 - #57980 (Add the edition guide to the bookshelf)
 - #57984 (Improve bug message in check_ty)
 - #57999 (Add MOVBE x86 CPU feature)
 - #58000 (Fixes and cleanups)
 - #58005 (update docs for fix_start/end_matches)
 - #58007 (Don't panic when accessing enum variant ctor using `Self` in match)
 - #58008 (Pass correct arguments to places_conflict)

Failed merges:

r? @ghost
@bors bors merged commit 1e57726 into rust-lang:master Jan 31, 2019
@dwijnand dwijnand deleted the from-Arc/Rc-to-NonNull branch January 31, 2019 07:15
@SimonSapin SimonSapin mentioned this pull request Apr 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dwijnand @rust-highfive @Centril @alexcrichton @bors @bluss