Stabilize box_into_pin
#97397
Merged
Stabilize box_into_pin
#97397
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
rustbot
added
the
T-libs
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
CI failure is unrelated: |
wwylele
reviewed
May 25, 2022
JohnTitor
force-pushed
the
stabilize-box-into-pin
branch
from
3e6fe5f
to
0a05584
Compare
library/alloc/src/boxed.rs
Outdated
| /// # Notes | ||
| /// | ||
| /// Because `Box` and `Pin` are `#[fundamental]`, implementing `From<Box>` | ||
| /// for `Pin<T>` may be ambiguous. |
There was a problem hiding this comment.
Fundamental is an unstable (and not very well known) feature, so this seems like an odd thing to document on such a potentially prominent impl/type without links or further elaboration.
I think the impl text is also wrong, it should be From<Box<T>> for Pin<T>, right?
Is this an impl that's possible to add outside std? Maybe if the T is concrete? The blanket impl definitely seems like it would be alloc-only.
There was a problem hiding this comment.
I think we should say something more concrete here, like "it's not recommended that crates add such an impl, as it'll introduce ambiguity, and give an example, rather than talking about fundamental -- that would make more sense to me.
There was a problem hiding this comment.
Updated the docs via 41beb30, how about?
library/alloc/src/boxed.rs
Outdated
| /// as it'll introduce ambiguity, for example: | ||
| /// ```compile_fail | ||
| /// # use std::pin::Pin; | ||
| /// # struct Foo; |
There was a problem hiding this comment.
Suggested change
| /// # struct Foo; | |
| /// struct Foo; // A type defined in this crate. |
library/alloc/src/boxed.rs
Outdated
| /// # Notes | ||
| /// | ||
| /// It's not recommended that crates add an impl like `From<Box<T>> for Pin<T>`, | ||
| /// as it'll introduce ambiguity, for example: |
There was a problem hiding this comment.
Suggested change
| /// as it'll introduce ambiguity, for example: | |
| /// as it'll introduce an ambiguity when calling `Pin::from`. A demonstration of such a poor impl is shown below. |
|
r=me with nits resolved; please squash commits |
Mark-Simulacrum
added
S-waiting-on-author
JohnTitor
force-pushed
the
stabilize-box-into-pin
branch
from
9b04af3
to
572c390
Compare
|
@bors r=Mark-Simulacrum |
|
📌 Commit 572c390 has been approved by |
bors
added
S-waiting-on-bors
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Jun 2, 2022
…=Mark-Simulacrum Stabilize `box_into_pin` FCP has been completed: rust-lang#62370 (comment) Also, adds notes as per rust-lang#62370 (comment) Closes rust-lang#62370
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 2, 2022
Rollup of 6 pull requests Successful merges: - rust-lang#96894 (Apply track_caller to closure on `expect_non_local()`) - rust-lang#97023 (Diagnose anonymous lifetimes errors more uniformly between async and regular fns) - rust-lang#97397 (Stabilize `box_into_pin`) - rust-lang#97587 (Migrate more diagnostics to use the `#[derive(SessionDiagnostic)]`) - rust-lang#97603 (Arc make_mut doc comment spelling correction.) - rust-lang#97635 (Fix file metadata documentation for Windows) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 2, 2022
…n-docs, r=thomcc Improve documentation for constructors of pinned `Box`es Adds a cross-references between `Box::pin` and `Box::into_pin` (and other related methods, i.e. the equivalent `From` implementation, and the unstable `pin_in` method), in particular now that `into_pin` [was stabilized](rust-lang#97397). The main goal is to further improve visibility of the fact that `Box<T> -> Pin<Box<T>>` conversion exits in the first place, and that `Box::pin(x)` is – essentially – just a convenience function for `Box::into_pin(Box::new(x))` The motivating context why I think this is important is even experienced Rust users overlooking the existence this kind of conversion, [e.g. in this thread on IRLO](https://internals.rust-lang.org/t/pre-rfc-function-variants/16732/7?u=steffahn); and also the fact that that discussion brought up that there would be a bunch of Box-construction methods "missing" such as e.g. methods with fallible allocation a la "`Box::try_pin`", and similar; while those are in fact *not* necessary, because you can use `Box::into_pin(Box::try_new(x)?)` instead. I have *not* included explicit mention of methods (e.g. `try_new`) in the docs of stable methods (e.g. `into_pin`). (Referring to unstable API in stable API docs would be bad style IMO.) Stable examples I have in mind with the statement "constructing a (pinned) Box in a different way than with `Box::new`" are things like cloning a `Box`, or `Box::from_raw`. If/when `try_new` would get stabilized, it would become a very good concrete example use-case of `Box::into_pin` IMO.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 2, 2022
…n-docs, r=thomcc Improve documentation for constructors of pinned `Box`es Adds a cross-references between `Box::pin` and `Box::into_pin` (and other related methods, i.e. the equivalent `From` implementation, and the unstable `pin_in` method), in particular now that `into_pin` [was stabilized](rust-lang#97397). The main goal is to further improve visibility of the fact that `Box<T> -> Pin<Box<T>>` conversion exits in the first place, and that `Box::pin(x)` is – essentially – just a convenience function for `Box::into_pin(Box::new(x))` The motivating context why I think this is important is even experienced Rust users overlooking the existence this kind of conversion, [e.g. in this thread on IRLO](https://internals.rust-lang.org/t/pre-rfc-function-variants/16732/7?u=steffahn); and also the fact that that discussion brought up that there would be a bunch of Box-construction methods "missing" such as e.g. methods with fallible allocation a la "`Box::try_pin`", and similar; while those are in fact *not* necessary, because you can use `Box::into_pin(Box::try_new(x)?)` instead. I have *not* included explicit mention of methods (e.g. `try_new`) in the docs of stable methods (e.g. `into_pin`). (Referring to unstable API in stable API docs would be bad style IMO.) Stable examples I have in mind with the statement "constructing a (pinned) Box in a different way than with `Box::new`" are things like cloning a `Box`, or `Box::from_raw`. If/when `try_new` would get stabilized, it would become a very good concrete example use-case of `Box::into_pin` IMO.
JohnTitor
added
the
relnotes
workingjubilee
pushed a commit
to tcdi/postgrestd
that referenced
this pull request
Sep 15, 2022
…=thomcc Improve documentation for constructors of pinned `Box`es Adds a cross-references between `Box::pin` and `Box::into_pin` (and other related methods, i.e. the equivalent `From` implementation, and the unstable `pin_in` method), in particular now that `into_pin` [was stabilized](rust-lang/rust#97397). The main goal is to further improve visibility of the fact that `Box<T> -> Pin<Box<T>>` conversion exits in the first place, and that `Box::pin(x)` is – essentially – just a convenience function for `Box::into_pin(Box::new(x))` The motivating context why I think this is important is even experienced Rust users overlooking the existence this kind of conversion, [e.g. in this thread on IRLO](https://internals.rust-lang.org/t/pre-rfc-function-variants/16732/7?u=steffahn); and also the fact that that discussion brought up that there would be a bunch of Box-construction methods "missing" such as e.g. methods with fallible allocation a la "`Box::try_pin`", and similar; while those are in fact *not* necessary, because you can use `Box::into_pin(Box::try_new(x)?)` instead. I have *not* included explicit mention of methods (e.g. `try_new`) in the docs of stable methods (e.g. `into_pin`). (Referring to unstable API in stable API docs would be bad style IMO.) Stable examples I have in mind with the statement "constructing a (pinned) Box in a different way than with `Box::new`" are things like cloning a `Box`, or `Box::from_raw`. If/when `try_new` would get stabilized, it would become a very good concrete example use-case of `Box::into_pin` IMO.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FCP has been completed: #62370 (comment)
Also, adds notes as per #62370 (comment)
Closes #62370