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 Rc::into_inner, as a parallel to Arc::into_inner #109026

Merged
merged 2 commits into from
Mar 12, 2023
Merged

Introduce Rc::into_inner, as a parallel to Arc::into_inner #109026

merged 2 commits into from
Mar 12, 2023

Conversation

joshtriplett
Copy link
Member

Unlike Arc, Rc doesn't have the same race condition to avoid, but
maintaining an equivalent API still makes it easier to work with both
Rc and Arc.

@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2023

r? @cuviper

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

@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 Mar 11, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@joshtriplett joshtriplett mentioned this pull request Mar 11, 2023
Closed
6 tasks
dtolnay
dtolnay requested changes Mar 11, 2023
View reviewed changes
library/alloc/src/rc/tests.rs Outdated Show resolved Hide resolved
@joshtriplett
Introduce Rc::into_inner, as a parallel to Arc::into_inner
a2341fb 
Unlike `Arc`, `Rc` doesn't have the same race condition to avoid, but
maintaining an equivalent API still makes it easier to work with both
`Rc` and `Arc`.
@dtolnay
Copy link
Member

dtolnay commented Mar 11, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Mar 11, 2023

📌 Commit a2341fb has been approved by dtolnay

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 Mar 11, 2023
@dtolnay dtolnay assigned dtolnay and unassigned cuviper Mar 11, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 12, 2023
@matthiaskrgr
Rollup merge of rust-lang#109026 - joshtriplett:rc-into-inner, r=dtolnay
4065bb4 
Introduce `Rc::into_inner`, as a parallel to `Arc::into_inner`

Unlike `Arc`, `Rc` doesn't have the same race condition to avoid, but
maintaining an equivalent API still makes it easier to work with both
`Rc` and `Arc`.
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 12, 2023
Closed
@matthiaskrgr
Copy link
Member

@bors r- please r+ prs where CI is already failing... 🙃

@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 Mar 12, 2023
steffahn
steffahn reviewed Mar 12, 2023
View reviewed changes
library/alloc/src/rc/tests.rs Outdated
Comment on lines 168 to 169


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

One less newline will make the formatting tidy check happy :-)

@joshtriplett
Copy link
Member Author

@bors r=dtolnay

Thanks for the fix!

@bors
Copy link
Contributor

bors commented Mar 12, 2023

📌 Commit 992957e has been approved by dtolnay

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 12, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 12, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2023
@bors
Auto merge of rust-lang#109056 - matthiaskrgr:rollup-9trny1z, r=matth…
7b4f489 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#108651 (Forbid the use of `#[target_feature]` on `main`)
 - rust-lang#109009 (rustdoc: use restricted Damerau-Levenshtein distance for search)
 - rust-lang#109026 (Introduce `Rc::into_inner`, as a parallel to `Arc::into_inner`)
 - rust-lang#109029 (Gate usages of `dyn*` and const closures in macros)
 - rust-lang#109031 (Rename `config.toml.example` to `config.example.toml`)
 - rust-lang#109032 (Use `TyCtxt::trait_solver_next` in some places)
 - rust-lang#109047 (typo)
 - rust-lang#109052 (Add eslint check for rustdoc-gui tester)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b28775c into rust-lang:master Mar 12, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 12, 2023
@steffahn
Copy link
Member

I just noticed that the Rc::into_inner defined in this PR introduced its own feature gate “rc_into_inner” instead of using “arc_into_inner”, too. Was that intentional?

@joshtriplett
Copy link
Member Author

joshtriplett commented Mar 14, 2023 via email

@steffahn
Copy link
Member

steffahn commented Mar 14, 2023 •
edited
Loading

Okay. So does that mean the stabilization FCP in the tracking issue is only concerned with stabilizing Arc::into_inner or also Rc::into_inner at the same time? (Because that’s not completely clear to me, reading the statement “I think Arc::into_inner makes sense to stabilize at this point. And Rc::into_inner is trivial.”.)

Edit: Just noticed the sentence “shall we stabilize these” after that, which sounds like it fairly clearly refers to both, in which case, I can’t really say I’m fully understanding the procedure. Or actually… maybe the argument is simply that this makes it easier to have the option to stabilize just one of them without needing to re-label the function’s feature flag?

In any case, the tracking issue should perhaps be updated to mention #![feature(rc_into_inner)], too then? Edit: I’ve listed it in the tracking issues OP now, next to the other feature flag.

@joshtriplett joshtriplett deleted the rc-into-inner branch March 16, 2023 04:00
@joshtriplett
Copy link
Member Author

maybe the argument is simply that this makes it easier to have the option to stabilize just one of them without needing to re-label the function’s feature flag?

Exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steffahn steffahn steffahn left review comments

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
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.
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.
7 participants
@joshtriplett @rustbot @dtolnay @bors @matthiaskrgr @steffahn @cuviper