Implement `Rc`/`Arc` conversions for string-like types #45990

murarth · 2017-11-14T20:22:21Z

Provides the following conversion implementations:

From<{CString,&CStr}> for {Arc,Rc}<CStr>
From<{OsString,&OsStr}> for {Arc,Rc}<OsStr>
From<{PathBuf,&Path}> for {Arc,Rc}<Path>

kennytm · 2017-11-14T20:31:38Z

src/libstd/ffi/c_str.rs

+    #[inline]
+    fn from(s: CString) -> Arc<CStr> {
+        let arc: Arc<[u8]> = Arc::from(s.into_inner());
+        unsafe { mem::transmute(arc) }


Could we do this via Arc::into_raw + x as *const CStr + Arc::from_raw instead of transmute?

Sure. I've now replaced each instance of transmute.

murarth · 2017-11-14T23:38:35Z

Tests passed. r? @kennytm

kennytm · 2017-11-15T05:36:46Z

r? @BurntSushi

BurntSushi · 2017-11-15T11:58:26Z

src/libstd/ffi/c_str.rs

+    #[inline]
+    fn from(s: &CStr) -> Arc<CStr> {
+        let arc: Arc<[u8]> = Arc::from(s.to_bytes_with_nul());
+        unsafe { Arc::from_raw(Arc::into_raw(arc) as *const CStr) }


Why is this safe? What guarantees that &'a CStr outlives the resulting Arc<CStr>?

Why would &'a CStr need to outlive Arc<CStr>? Arc::from does a new heap allocation and copies over all the contents of s.
The unsafe block just does Arc<[u8]> -> Arc<CStr>. This is safe since the invariants of CStr are maintained.

Oh I see. I didn't realize there was a copy here. In hindsight, the Arc<[u8]> type should have told me that. And of course this impl: https://doc.rust-lang.org/std/sync/struct.Arc.html#impl-From<&'a [T]>

Centril · 2017-11-15T14:23:24Z

This is continuation of RFC 1845, shared_from_slice, which motivates this PR =)

BurntSushi · 2017-11-15T14:25:59Z

This seems reasonable to me!

@rfcbot merge

kennytm · 2017-11-15T14:37:38Z

@BurntSushi The correct command is @rfcbot fcp merge...

BurntSushi · 2017-11-15T14:51:49Z

Let's try it again...

@rfcbot fcp merge

rfcbot · 2017-11-15T14:51:49Z

Team member @BurntSushi has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, 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.

kennytm · 2017-11-22T11:00:27Z

Ping @Kimundi, ticky boxes for you!

rfcbot · 2017-11-22T19:35:57Z

🔔 This is now entering its final comment period, as per the review above. 🔔

alexcrichton · 2017-11-25T15:36:11Z

@bors: r+

bors · 2017-11-25T15:36:12Z

📌 Commit 3c1d59f has been approved by alexcrichton

bors · 2017-11-25T18:54:20Z

⌛ Testing commit 3c1d59f61817c4470ebbaa17c925633c71cb1fd3 with merge b199f97bdde51ed7ad4c16799c3ef05a354428e1...

bors · 2017-11-25T19:00:42Z

💔 Test failed - status-appveyor

kennytm · 2017-11-25T19:26:28Z

Could not compile stage0-libstd on all Windows, missing imports.

error[E0433]: failed to resolve. Use of undeclared type or module `Arc`
   --> src\libstd\sys\windows\os_str.rs:162:18
    |
162 |         unsafe { Arc::from_raw(Arc::into_raw(arc) as *const Slice) }
    |                  ^^^ Use of undeclared type or module `Arc`
error[E0433]: failed to resolve. Use of undeclared type or module `Arc`
   --> src\libstd\sys\windows\os_str.rs:162:32
    |
162 |         unsafe { Arc::from_raw(Arc::into_raw(arc) as *const Slice) }
    |                                ^^^ Use of undeclared type or module `Arc`
error[E0412]: cannot find type `Arc` in this scope
   --> src\libstd\sys\windows\os_str.rs:121:31
    |
121 |     pub fn into_arc(&self) -> Arc<Slice> {
    |                               ^^^ not found in this scope
    |
help: possible candidate is found in another module, you can import it into scope
    |
14  | use alloc::arc::Arc;
    |
error[E0412]: cannot find type `Arc` in this scope
   --> src\libstd\sys\windows\os_str.rs:160:31
    |
160 |     pub fn into_arc(&self) -> Arc<Slice> {
    |                               ^^^ not found in this scope
    |
help: possible candidate is found in another module, you can import it into scope
    |
14  | use alloc::arc::Arc;
    |
error: aborting due to 4 previous errors
error: Could not compile `std`.

murarth · 2017-11-25T19:53:07Z

@kennytm: Fixed. re-r+?

kennytm · 2017-11-25T19:55:13Z

@bors r=alexcrichton

bors · 2017-11-25T19:55:14Z

📌 Commit 85e6421 has been approved by alexcrichton

Implement `Rc`/`Arc` conversions for string-like types Provides the following conversion implementations: * `From<`{`CString`,`&CStr`}`>` for {`Arc`,`Rc`}`<CStr>` * `From<`{`OsString`,`&OsStr`}`>` for {`Arc`,`Rc`}`<OsStr>` * `From<`{`PathBuf`,`&Path`}`>` for {`Arc`,`Rc`}`<Path>` Closes rust-lang#45008

bors · 2017-11-26T03:30:57Z

⌛ Testing commit 85e6421 with merge 5600b272ddaffa84cdbbdf4872a89a2680d72ccf...

bors · 2017-11-26T04:26:15Z

💔 Test failed - status-travis

Provides the following conversion implementations: * `From<`{`CString`,`&CStr`}`>` for {`Arc`,`Rc`}`<CStr>` * `From<`{`OsString`,`&OsStr`}`>` for {`Arc`,`Rc`}`<OsStr>` * `From<`{`PathBuf`,`&Path`}`>` for {`Arc`,`Rc`}`<Path>`

murarth · 2017-11-26T05:16:18Z

Psh. The wasm stuff merged and added a new OsString implementation. It's a straight copy of the Unix implementation, so I copied over the changes.

kennytm · 2017-11-26T06:25:29Z

@bors r=alexcrichton

bors · 2017-11-26T06:25:30Z

📌 Commit 1bbc776 has been approved by alexcrichton

bors · 2017-11-26T06:49:49Z

⌛ Testing commit 1bbc776 with merge d4dc289...

Implement `Rc`/`Arc` conversions for string-like types Provides the following conversion implementations: * `From<`{`CString`,`&CStr`}`>` for {`Arc`,`Rc`}`<CStr>` * `From<`{`OsString`,`&OsStr`}`>` for {`Arc`,`Rc`}`<OsStr>` * `From<`{`PathBuf`,`&Path`}`>` for {`Arc`,`Rc`}`<Path>` Closes #45008

bors · 2017-11-26T09:15:00Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d4dc289 to master...

cf. <rust-lang/rust#45990>

Correct a few stability attributes * The extra impls for `ManuallyDrop` were added in rust-lang#44310 which was only stabilised in 1.22.0. * The impls for `SliceIndex` were stabilised in rust-lang#43373 but as `RangeInclusive` and `RangeToInclusive` are still unstable the impls should remain unstable. * The `From` impls for atomic integers were added in rust-lang#45610 but most atomic integers are still unstable. * The `shared_from_slice2` impls were added in rust-lang#45990 but they won't be stable until 1.24.0. * The `Mutex` and `RwLock` impls were added in rust-lang#46082 but won't be stable until 1.24.0.

kennytm reviewed Nov 14, 2017

View reviewed changes

rust-highfive assigned kennytm Nov 14, 2017

rust-highfive assigned BurntSushi and unassigned kennytm Nov 15, 2017

BurntSushi reviewed Nov 15, 2017

View reviewed changes

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

kennytm added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 15, 2017

rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Nov 15, 2017

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Nov 22, 2017

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Nov 22, 2017

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 25, 2017

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 25, 2017

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 25, 2017

kennytm mentioned this pull request Nov 25, 2017

Rollup of 7 pull requests #46266

Closed

Implement Rc/Arc conversions for string-like types

1bbc776

Provides the following conversion implementations: * `From<`{`CString`,`&CStr`}`>` for {`Arc`,`Rc`}`<CStr>` * `From<`{`OsString`,`&OsStr`}`>` for {`Arc`,`Rc`}`<OsStr>` * `From<`{`PathBuf`,`&Path`}`>` for {`Arc`,`Rc`}`<Path>`

bors merged commit 1bbc776 into rust-lang:master Nov 26, 2017

murarth deleted the rc-from-strs branch November 26, 2017 17:13

kennytm mentioned this pull request Nov 28, 2017

Updated RELEASES.md for 1.23.0 #46327

Merged

lo48576 pushed a commit to lo48576/opaque_typedef that referenced this pull request Dec 2, 2017

Implement derive for IntoArc and IntoRc

bd4d19f

cf. <rust-lang/rust#45990>

ollie27 mentioned this pull request Dec 27, 2017

Correct a few stability attributes #47030

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `Rc`/`Arc` conversions for string-like types #45990

Implement `Rc`/`Arc` conversions for string-like types #45990

murarth commented Nov 14, 2017

kennytm Nov 14, 2017

murarth Nov 14, 2017

murarth commented Nov 14, 2017

kennytm commented Nov 15, 2017

BurntSushi Nov 15, 2017

Centril Nov 15, 2017 •

edited

Loading

BurntSushi Nov 15, 2017 •

edited

Loading

Centril commented Nov 15, 2017

BurntSushi commented Nov 15, 2017

kennytm commented Nov 15, 2017

BurntSushi commented Nov 15, 2017

rfcbot commented Nov 15, 2017 •

edited by Kimundi

Loading

kennytm commented Nov 22, 2017

rfcbot commented Nov 22, 2017

alexcrichton commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 25, 2017

kennytm commented Nov 25, 2017

murarth commented Nov 25, 2017

kennytm commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

murarth commented Nov 26, 2017

kennytm commented Nov 26, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

Implement Rc/Arc conversions for string-like types #45990

Implement Rc/Arc conversions for string-like types #45990

Conversation

murarth commented Nov 14, 2017

kennytm Nov 14, 2017

Choose a reason for hiding this comment

murarth Nov 14, 2017

Choose a reason for hiding this comment

murarth commented Nov 14, 2017

kennytm commented Nov 15, 2017

BurntSushi Nov 15, 2017

Choose a reason for hiding this comment

Centril Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

BurntSushi Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

Centril commented Nov 15, 2017

BurntSushi commented Nov 15, 2017

kennytm commented Nov 15, 2017

BurntSushi commented Nov 15, 2017

rfcbot commented Nov 15, 2017 • edited by Kimundi Loading

kennytm commented Nov 22, 2017

rfcbot commented Nov 22, 2017

alexcrichton commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 25, 2017

kennytm commented Nov 25, 2017

murarth commented Nov 25, 2017

kennytm commented Nov 25, 2017

bors commented Nov 25, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

murarth commented Nov 26, 2017

kennytm commented Nov 26, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

bors commented Nov 26, 2017

Implement `Rc`/`Arc` conversions for string-like types #45990

Implement `Rc`/`Arc` conversions for string-like types #45990

Centril Nov 15, 2017 •

edited

Loading

BurntSushi Nov 15, 2017 •

edited

Loading

rfcbot commented Nov 15, 2017 •

edited by Kimundi

Loading