Document `From::from` impls #137330

TimTheBig · 2025-02-20T18:09:28Z

This resolves #51430 by documenting all from impls in lib std and core.
I still need someone to look over a few of the comments to see if the style is correct.

rustbot · 2025-02-20T18:09:39Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2025-02-20T18:09:42Z

The Miri subtree was changed

cc @rust-lang/miri

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

TimTheBig · 2025-02-20T18:12:09Z

@rustbot label A-docs C-enhancement T-libs-api

TimTheBig · 2025-02-20T18:16:34Z

Should I move Portable SIMD and rust-analyzer changes even though they are small?

RalfJung · 2025-02-20T18:19:54Z

It is generally a good idea to split up large changes. :) You changed a whole bunch of files in rust-analyzer, so yeah that should be split. Miri and clippy have fewer changes, but the fact that there are any changes at all there still contradicts the PR description.

It's too late now, but the best strategy for PRs like this is to start with just a few files, e.g. just core, take the review feedback, and then apply that to the next PRs. That avoids having to re-do large amounts of work when there is overarching feedback.

rustbot · 2025-02-20T18:27:10Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

TimTheBig · 2025-02-20T18:29:05Z

My mistake, I will revert the rust-analyzer changes.

oli-obk

This PR requires a lot of changes, I did not get far with reviews, but a lot of the doc comments only explain the body in its rawest form. I feel like fully reviewing this just makes the reviewer do the work of documenting things, so it may be easier to just write PRs with documentation.

Also please disclose whether you generated this documentation, even just partially or as an initial thing that you edited.

library/alloc/src/bstr.rs

library/core/src/array/mod.rs

library/core/src/fmt/mod.rs

library/core/src/num/nonzero.rs

library/core/src/ptr/alignment.rs

Co-authored-by: Oli Scherer <github35764891676564198441@oli-obk.de>

Co-authored-by: Alejandra González <blyxyas@gmail.com>

Co-authored-by: Jubilee <workingjubilee@gmail.com>

library/alloc/src/bstr.rs

library/core/src/num/error.rs

library/std/src/os/unix/net/listener.rs

library/std/src/os/xous/ffi/definitions.rs

Sky9x

Remove comments on:

Obvious/self explanatory conversions. These comments are redundant with the signature of the method.
Internal/private types. These do not need rustdocs.
Platform specific code. These do not need rustdocs.

library/alloc/src/bstr.rs

Sky9x · 2025-04-15T00:46:44Z

library/alloc/src/bstr.rs

@@ -204,6 +207,7 @@ impl Default for ByteString {
 //
 // #[unstable(feature = "bstr", issue = "134915")]
 // impl From<Vec<u8>> for ByteString {
+//     /// Make a `ByteString` with `Vec<u8>` as inner


Suggested change

// /// Make a `ByteString` with `Vec<u8>` as inner

This clarifies that it is no copy

All From conversions between owned buffers like String, Vec, ByteString, OsString, etc are no copy

Sky9x · 2025-04-15T00:46:49Z

library/alloc/src/bstr.rs

@@ -212,6 +216,7 @@ impl Default for ByteString {

 #[unstable(feature = "bstr", issue = "134915")]
 impl From<ByteString> for Vec<u8> {
+    /// Return the inner `Vec` of the byte string.


Suggested change

/// Return the inner `Vec` of the byte string.

library/std/src/sys/process/windows.rs

Sky9x · 2025-04-15T01:04:16Z

library/std/src/sys/process/windows.rs

@@ -728,6 +742,7 @@ impl ExitStatus {

 /// Converts a raw `u32` to a type-safe `ExitStatus` by wrapping it without copying.
 impl From<u32> for ExitStatus {
+    /// Wrap `u32` in a type-safe `ExitStatus`


Suggested change

/// Wrap `u32` in a type-safe `ExitStatus`

Sky9x · 2025-04-15T01:05:26Z

library/std/src/sys/process/windows.rs

@@ -777,12 +792,14 @@ impl ExitCode {
 }

 impl From<u8> for ExitCode {
+    /// Convert the `u8` to a `u32` then wrap it in a `ExitCode`


Suggested change

/// Convert the `u8` to a `u32` then wrap it in a `ExitCode`

Sky9x · 2025-04-15T01:05:34Z

library/std/src/sys/process/windows.rs

    fn from(code: u8) -> Self {
        ExitCode(u32::from(code))
    }
 }

 impl From<u32> for ExitCode {
+    /// Wrap the `u32` in a `ExitCode`


Suggested change

/// Wrap the `u32` in a `ExitCode`

Sky9x · 2025-04-15T01:06:11Z

library/std/src/thread/mod.rs

@@ -1265,6 +1265,7 @@ mod thread_name_string {
    }

    impl From<String> for ThreadNameString {
+        /// Convert `String` to a `CString` then use that as the `ThreadNameString` inner


Suggested change

/// Convert `String` to a `CString` then use that as the `ThreadNameString` inner

Sky9x · 2025-04-15T16:28:45Z

library/alloc/src/bstr.rs

+    /// Try to allocate a `ByteStr`s bytes as a UTF-8 `String`.
+    ///
+    /// # Errors
+    /// If `ByteStr` is not valid UTF-8


Suggested change

/// Try to allocate a `ByteStr`s bytes as a UTF-8 `String`.

///

/// # Errors

/// If `ByteStr` is not valid UTF-8

library/alloc/src/task.rs

Sky9x · 2025-04-15T16:48:48Z

library/std/src/os/xous/ffi/definitions.rs

@@ -193,6 +195,7 @@ pub struct Connection(u32);

 #[stable(feature = "rust1", since = "1.0.0")]
 impl From<u32> for Connection {
+    /// Make a `Connection` with `u32` as inner


Suggested change

/// Make a `Connection` with `u32` as inner

Sky9x · 2025-04-15T16:50:11Z

library/std/src/sys/pal/windows/time.rs

+        /// Use `PerformanceCounterInstant` to get instant in nano sec, then make a `Duration` from nanos.
+        ///
+        /// ## Cost
+        /// This calls a function that makes an atomic oparation


Suggested change

/// Use `PerformanceCounterInstant` to get instant in nano sec, then make a `Duration` from nanos.

///

/// ## Cost

/// This calls a function that makes an atomic oparation

???

library/std/src/sync/mpmc/error.rs

library/std/src/sync/poison.rs

library/std/src/process.rs

library/core/src/ptr/non_null.rs

library/std/src/sys/process/windows.rs

Co-authored-by: Sky <sky@sky9.dev>

TimTheBig · 2025-04-16T00:24:24Z

@Sky9x Thank you for the suggestions.

rustbot assigned Amanieu Feb 20, 2025

fmease changed the title ~~Document from~~ Document From::from impls Feb 20, 2025

This comment has been minimized.

Sign in to view

RalfJung mentioned this pull request Feb 20, 2025

rustbot sends the same pings multiple times rust-lang/triagebot#1738

Open

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 20, 2025

This comment has been minimized.

Sign in to view

TimTheBig force-pushed the document-from branch from 68a5648 to d5de4c6 Compare February 20, 2025 19:47

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 20, 2025

This comment has been minimized.

Sign in to view

oli-obk requested changes Feb 21, 2025

View reviewed changes

TimTheBig and others added 10 commits March 24, 2025 20:42

Update exclusive.rs

44e4080

Apply suggestions from code review

42fd425

Update library/core/src/ptr/alignment.rs

5cddbcb

Apply suggestions from code review

b20bd28

Co-authored-by: Oli Scherer <github35764891676564198441@oli-obk.de>

Apply suggestions from code review

4cbfc84

Co-authored-by: Alejandra González <blyxyas@gmail.com>

Apply suggestions from code review

199448b

Co-authored-by: Jubilee <workingjubilee@gmail.com>

Update library/portable-simd/crates/core_simd/src/masks.rs

894f270

Apply suggestions from code review

124bcb2

Co-authored-by: Jubilee <workingjubilee@gmail.com>

Apply suggestions from code review

858b821

Remove changes from tools

59a2ea3

TimTheBig force-pushed the document-from branch from 544398c to 59a2ea3 Compare March 25, 2025 00:42

Update library/core/src/ptr/alignment.rs

6891a03

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 25, 2025

This comment has been minimized.

Sign in to view

TimTheBig commented Mar 26, 2025

View reviewed changes

library/alloc/src/bstr.rs Outdated Show resolved Hide resolved

Fix formating

84128a3

TimTheBig requested a review from workingjubilee March 31, 2025 00:34

TimTheBig commented Apr 10, 2025

View reviewed changes

library/alloc/src/bstr.rs Outdated Show resolved Hide resolved

TimTheBig commented Apr 10, 2025

View reviewed changes

library/core/src/num/error.rs Outdated Show resolved Hide resolved

TimTheBig commented Apr 10, 2025

View reviewed changes

library/std/src/os/unix/net/listener.rs Outdated Show resolved Hide resolved

TimTheBig commented Apr 10, 2025

View reviewed changes

library/std/src/os/xous/ffi/definitions.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

d03e4b8

Sky9x suggested changes Apr 15, 2025

View reviewed changes

TimTheBig commented Apr 16, 2025

View reviewed changes

library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved

TimTheBig commented Apr 16, 2025

View reviewed changes

library/std/src/sys/process/windows.rs Outdated Show resolved Hide resolved

TimTheBig commented Apr 16, 2025

View reviewed changes

library/std/src/sys/process/windows.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

750828d

Co-authored-by: Sky <sky@sky9.dev>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document `From::from` impls #137330

Document `From::from` impls #137330

TimTheBig commented Feb 20, 2025

rustbot commented Feb 20, 2025

rustbot commented Feb 20, 2025

TimTheBig commented Feb 20, 2025

TimTheBig commented Feb 20, 2025 •

edited

Loading

This comment has been minimized.

RalfJung commented Feb 20, 2025

rustbot commented Feb 20, 2025

TimTheBig commented Feb 20, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk left a comment •

edited

Loading

This comment has been minimized.

Sky9x left a comment

Sky9x Apr 15, 2025

TimTheBig Apr 16, 2025

Sky9x Apr 16, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

Sky9x Apr 15, 2025

TimTheBig commented Apr 16, 2025

Document From::from impls #137330

Are you sure you want to change the base?

Document From::from impls #137330

Conversation

TimTheBig commented Feb 20, 2025

rustbot commented Feb 20, 2025

rustbot commented Feb 20, 2025

TimTheBig commented Feb 20, 2025

TimTheBig commented Feb 20, 2025 • edited Loading

This comment has been minimized.

RalfJung commented Feb 20, 2025

rustbot commented Feb 20, 2025

TimTheBig commented Feb 20, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

Sky9x left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimTheBig commented Apr 16, 2025

Document `From::from` impls #137330

Document `From::from` impls #137330

TimTheBig commented Feb 20, 2025 •

edited

Loading

TimTheBig commented Feb 20, 2025 •

edited

Loading

oli-obk left a comment •

edited

Loading