Fix #124275: Implemented Default for Arc<str> #124367
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jhpratt (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 (
|
rustbot
added
S-waiting-on-review
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| @@ -1284,6 +1287,15 @@ impl Default for Box<str> { | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(not(no_global_oom_handling))] | |||
There was a problem hiding this comment.
| #[cfg(not(no_global_oom_handling))] | |
| #[cfg(all(not(no_global_oom_handling), not(no_rc)))] |
|
Is there a particular reason this is done in It might also make sense to implement |
|
jhpratt
added
the
I-libs-api-nominated
m-ou-se
added
T-libs-api
|
We discussed this in today's libs-api meeting and it seems reasonable. We would like to see the additional impls for:
Starting a libs-api FCP to confirm that we want to approve these six impls: @rfcbot merge |
|
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
Also |
I've implemented these on my branch here https://github.com/Billy-Sheppard/rust/tree/81b0331cc7a298f79ced7aec395510379df8fa1b I've had a guess at the best locations/feature attrs for them but they might not be correct. However I'm unclear how to get the error[E0117]: only traits defined in the current crate can be implemented for types defined outside of the crate
--> library/std/src/ffi/os_str.rs:1350:1
|
1350 | impl Default for Arc<OsStr> {
| ^^^^^^^^^^^^^^^^^----------
| | |
| | `Arc` is not defined in the current crate
| impl doesn't use only types from inside the current crate
|
= note: define and implement a trait or new type instead
error[E0117]: only traits defined in the current crate can be implemented for types defined outside of the crate
--> library/std/src/ffi/os_str.rs:1360:1
|
1360 | impl Default for Rc<OsStr> {
| ^^^^^^^^^^^^^^^^^---------
| | |
| | `Rc` is not defined in the current crate
| impl doesn't use only types from inside the current crate
|
= note: define and implement a trait or new type instead
For more information about this error, try `rustc --explain E0117`. |
|
Import the struct, or implement where it's defined? |
|
DefaultInArc// alloc::sync
pub trait DefaultInArc {
fn default_arc() -> Arc<Self>;
}
impl<T: Default> DefaultInArc for T { /* call T::default() */ }
impl<T> DefaultInArc for [T] { /* empty slice */
impl DefaultInArc for str { /* empty string */ }
impl DefaultInArc for CStr { /* single NUL byte */ }
impl<T: DefaultInArc + ?Sized> Default for Arc<T>;
// std::sync (or maybe std::ffi)
impl DefaultInArc for OsStr { /* empty slice */ }(similar for This is the same coherence workaround that Edit: Alternately, we could just have a blanket |
This is what I assumed was going on. I'll let some others from the (Edit: I drafted one JIC, but happy to dismiss pending others thoughts.) |
|
I would propose we skip the OsStr impls for now due to this issue, and merge the rest. |
rfcbot
added
the
finished-final-comment-period
rfcbot
added
to-announce
Fix rust-lang#124275: Implemented Default for `Arc<str>` With added implementations. ``` GOOD Arc<CStr> BROKEN Arc<OsStr> // removed GOOD Rc<str> GOOD Rc<CStr> BROKEN Rc<OsStr> // removed GOOD Rc<[T]> GOOD Arc<[T]> ``` For discussion of rust-lang#124367 (comment). Key pain points currently: > I've had a guess at the best locations/feature attrs for them but they might not be correct. > However I'm unclear how to get the OsStr impl to compile, which file should they go in to avoid the error below? Is it possible, perhaps with some special std rust lib magic?
Fix #124275: Implemented Default for `Arc<str>` With added implementations. ``` GOOD Arc<CStr> BROKEN Arc<OsStr> // removed GOOD Rc<str> GOOD Rc<CStr> BROKEN Rc<OsStr> // removed GOOD Rc<[T]> GOOD Arc<[T]> ``` For discussion of rust-lang/rust#124367 (comment). Key pain points currently: > I've had a guess at the best locations/feature attrs for them but they might not be correct. > However I'm unclear how to get the OsStr impl to compile, which file should they go in to avoid the error below? Is it possible, perhaps with some special std rust lib magic?
dtolnay
removed
the
I-libs-api-nominated
This implements
DefaultforArc<str>inlibrary/alloc/src/boxed.rs, fixing #124275