Add atomic module to alloc::sync #58175
Conversation
|
r? @aidanhs (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
r? @SimonSapin |
|
Good catch! RFC 2480 states a similar goal:
I agree that it should also be a superset of Regarding this PR’s implementation, I think it can be a single As to stability, I think it should be the same as |
Hmm on a closer look I’m not sure. There are many other modules that are present in libcore but not liballoc. |
| // Since the current `liballoc` is not a superset of `libcore`, | ||
| // using `pub use core::sync::atomic;` will fail with some link errors. | ||
| // If `liballoc` becomes a superset of `libcore`, replace this with | ||
| // `pub use core::sync::atomic;`. |
There was a problem hiding this comment.
Specifically, it is this error: #58175 (comment)
[01:36:43] alloc/sync/atomic/index.html:13: broken link - alloc/marker/trait.Sync.html
See #58175 (comment).
Thanks, I changed it. |
|
@rust-lang/libs Any thoughts on whether liballoc should be a superset of libcore? |
|
@SimonSapin I personally don't mind either way. |
|
Agreed this should be fine to land |
|
Oh I see, there’s a subtlety that I missed in the PR description: for modules that exist in both crates, this makes I think that conditional is not useful. If we care about having what’s in libcore, I’d rather we do that at the crate level and reexport everything. ( |
There is no sentence clearly specifying this in rust-lang/rfcs#2480, but the following sentence looks like to contain "
|
The current |
FYI: Comparison of imports
use std::borrow::{Borrow, Cow};
use std::iter;
use std::sync::{atomic, Arc};current use alloc::borrow::{Borrow, Cow};
use alloc::sync::Arc;
use core::iter;
use core::sync::atomic;after this PR: use alloc::borrow::{Borrow, Cow};
use alloc::sync::{atomic, Arc};
use core::iter;if use alloc::borrow::{Borrow, Cow};
use alloc::iter;
use alloc::sync::{atomic, Arc}; |
|
On second thought, I think that If we really want to keep pushing in that direction then we should consider renaming |
|
@taiki-e That RFC is not accepted yet and can still evolve, so don’t read too much is what exactly it says or not. In fact I think this very conversation will affect the RFC. As to the “intermediate” subset, it doesn’t have to be imported all from one crate. Whenever liballoc is available, libcore is too. @Amanieu If Whether to rename the alloc crate is an unresolved question of RFC 2480. |
|
@SimonSapin Yes, I would say that IMO the only 2 approaches that make sense are:
I am personally happy with either approach. |
|
ping from triage @taiki-e @SimonSapin what's the update on this? |
This was one of the unresolved questions of rust-lang/rfcs#2480. As the RFC says this is maybe not useful in the sense that we are unlikely to ever have a second version, but making the crate a true subset makes one less issue to think about if we stabilize it and later want to merge standard library crates and have Cargo feature flags to enable or disable parts of the `std` crate. See also discussion in rust-lang#58175
|
I agree with @Amanieu about the two approaches. I’ve added an unresolved question to rust-lang/rfcs#2480 about picking one. (I think I prefer the status quo.) I’ll close this PR since it does neither of those approaches, but thanks for the patch @taiki-e! |
Move alloc::prelude::* to alloc::prelude::v1, make alloc a subset of std This was one of the unresolved questions of rust-lang/rfcs#2480. As the RFC says this is maybe not useful in the sense that we are unlikely to ever have a second version, but making the crate a true subset makes one less issue to think about if we stabilize it and later want to merge standard library crates and have Cargo feature flags to enable or disable parts of the `std` crate. See also discussion in rust-lang#58175. Also rename the feature gate and point to a dedicated tracking issue: rust-lang#58935
Move alloc::prelude::* to alloc::prelude::v1, make alloc a subset of std This was one of the unresolved questions of rust-lang/rfcs#2480. As the RFC says this is maybe not useful in the sense that we are unlikely to ever have a second version, but making the crate a true subset makes one less issue to think about if we stabilize it and later want to merge standard library crates and have Cargo feature flags to enable or disable parts of the `std` crate. See also discussion in rust-lang#58175. Also rename the feature gate and point to a dedicated tracking issue: rust-lang#58935
Of the modules of the
alloccrate, modules with the same names with thecorecrate exceptpreludeandsyncare re-exporting items of the same name module of thecorecrate.I think should also be done for
alloc::sync.cc rust-lang/rfcs#2480