Clarify examples related to Sync and SyncView - #162621
Conversation
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
| /// ``` | ||
| /// | ||
| /// `SyncView` ensures the struct is `Sync` without stripping the future of its | ||
| /// `SyncView` makes the struct `Sync` without stripping the struct of its |
There was a problem hiding this comment.
| /// `SyncView` makes the struct `Sync` without stripping the struct of its | |
| /// `SyncView` makes the value `Sync` without stripping the struct of its |
There was a problem hiding this comment.
Adopted. But I wonder if there is some references to teach more English or Rust's terminology to decide between words?
There was a problem hiding this comment.
tbh I do feel like struct or type is more accurate here. value refers to a specific instantiation of a struct/type whose Sync-ness is entirely dependent on the syncness of its type. And it applies here especially since the "struct" in this case is contrasting with the "struct" mentioned a few lines earlier
There was a problem hiding this comment.
Yeah, perhaps the best phrasing for both correctness and stylistic clarity would be “SyncView makes the type Sync without stripping it of its functionality…”.
|
@bors squash |
This comment has been minimized.
This comment has been minimized.
* Clarify examples related to `Sync` and `SyncView` Updated examples to clarify the role of non-`Sync` fields and the functionality of `SyncView`. * Apply suggestion from @tisonkun * Update library/core/src/sync/sync_view.rs Co-authored-by: Laine Taffin Altman <alexanderaltman@me.com>
|
🔨 3 commits were squashed into ea39610. |
3ab7a36 to
ea39610
Compare
Clarify examples related to `Sync` and `SyncView` Updated examples to clarify the role of non-`Sync` fields and the functionality of `SyncView`. This refers to rust-lang#98407
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #162636 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#162636), which was unapproved. |
|
Weird .. Let me see .. |
|
Add This should work now. cc @JohnTitor @JonathanBrouwer |
|
@bors try jobs=test-various |
Clarify examples related to `Sync` and `SyncView` try-job: test-various
This comment has been minimized.
This comment has been minimized.
|
@bors r=JohnTitor |
Clarify examples related to `Sync` and `SyncView` Updated examples to clarify the role of non-`Sync` fields and the functionality of `SyncView`. This refers to rust-lang#98407
…uwer Rollup of 7 pull requests Successful merges: - #161413 (mir-transform: Remove `is_optimization_stage`) - #161430 (Re-export `core::fmt::NumBuffer` in `alloc` (and `std`)) - #162446 (Prevent `should-fail` to be used in `rustdoc-ui` testsuite) - #162621 (Clarify examples related to `Sync` and `SyncView`) - #162650 (Initialize mingw for all mingw targets) - #162651 (Remove needless test running instructions) - #162658 (Remove noras target docs mention)
| /// ``` | ||
| /// ```ignore-wasm |
There was a problem hiding this comment.
This gives a "This example is not tested on wasm" warning in the docs https://doc.rust-lang.org/nightly/std/sync/struct.SyncView.html#examples which is a bit weird. Perhaps this and the original Atomic example should gate on target_has_threads instead?
This isn't super accurate anyway since we have wasm targets with threads and non-wasm targets without threads, it just happens that the wasm target in CI is the only thing we flag.
There was a problem hiding this comment.
Thanks for your suggestion.
My concern is that target_has_threads doesn't guarantee that std::thread::spawn is supported.
For example, on wasm32-unknown-unknown, enabling +atomics makes singlethread() return false, which enables target_has_threads. However, std still selects the unsupported thread backend, where Thread::new always returns UNSUPPORTED_PLATFORM.
View all comments
Updated examples to clarify the role of non-
Syncfields and the functionality ofSyncView.This refers to #98407