Skip to content

Commit

Permalink
auto merge of #18457 : japaric/rust/tocstr, r=alexcrichton
Browse files Browse the repository at this point in the history
Methods that used to take `ToCStr` implementors by value, now take them by reference. In particular, this breaks some uses of `Command`:

``` rust
Command::new("foo");  // Still works
Command::new(path) -> Command::new(&path)
cmd.arg(string) -> cmd.arg(&string) or cmd.arg(string.as_slice())
```

[breaking-change]

---

It may be sensible to remove `impl ToCstr for String` since:
- We're getting `impl Deref<str> for String`, so `string.to_cstr()` would still work
- `Command` methods would still be able to use `cmd.arg(string[..])` instead of `cmd.arg(&string)`.

But, I'm leaving that up to the library stabilization process.

r? @aturon 
cc #16918
  • Loading branch information
bors committed Nov 1, 2014
2 parents 51a25c7 + dd9dda7 commit 0547a40
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 39 deletions.
44 changes: 29 additions & 15 deletions src/librustrt/c_str.rs
Expand Up @@ -74,7 +74,7 @@ fn main() {
use collections::string::String;
use collections::hash;
use core::fmt;
use core::kinds::marker;
use core::kinds::{Sized, marker};
use core::mem;
use core::prelude::{Clone, Collection, Drop, Eq, ImmutableSlice, Iterator};
use core::prelude::{MutableSlice, None, Option, Ordering, PartialEq};
Expand Down Expand Up @@ -286,7 +286,7 @@ impl fmt::Show for CString {
}

/// A generic trait for converting a value to a CString.
pub trait ToCStr {
pub trait ToCStr for Sized? {
/// Copy the receiver into a CString.
///
/// # Failure
Expand Down Expand Up @@ -329,15 +329,7 @@ pub trait ToCStr {
}
}

// FIXME (#12938): Until DST lands, we cannot decompose &str into &
// and str, so we cannot usefully take ToCStr arguments by reference
// (without forcing an additional & around &str). So we are instead
// temporarily adding an instance for ~str and String, so that we can
// take ToCStr as owned. When DST lands, the string instances should
// be revisited, and arguments bound by ToCStr should be passed by
// reference.

impl<'a> ToCStr for &'a str {
impl ToCStr for str {
#[inline]
fn to_c_str(&self) -> CString {
self.as_bytes().to_c_str()
Expand Down Expand Up @@ -384,10 +376,10 @@ impl ToCStr for String {
// The length of the stack allocated buffer for `vec.with_c_str()`
const BUF_LEN: uint = 128;

impl<'a> ToCStr for &'a [u8] {
impl ToCStr for [u8] {
fn to_c_str(&self) -> CString {
let mut cs = unsafe { self.to_c_str_unchecked() };
check_for_null(*self, cs.as_mut_ptr());
check_for_null(self, cs.as_mut_ptr());
cs
}

Expand All @@ -403,11 +395,33 @@ impl<'a> ToCStr for &'a [u8] {
}

fn with_c_str<T>(&self, f: |*const libc::c_char| -> T) -> T {
unsafe { with_c_str(*self, true, f) }
unsafe { with_c_str(self, true, f) }
}

unsafe fn with_c_str_unchecked<T>(&self, f: |*const libc::c_char| -> T) -> T {
with_c_str(self, false, f)
}
}

impl<'a, Sized? T: ToCStr> ToCStr for &'a T {
#[inline]
fn to_c_str(&self) -> CString {
(**self).to_c_str()
}

#[inline]
unsafe fn to_c_str_unchecked(&self) -> CString {
(**self).to_c_str_unchecked()
}

#[inline]
fn with_c_str<T>(&self, f: |*const libc::c_char| -> T) -> T {
(**self).with_c_str(f)
}

#[inline]
unsafe fn with_c_str_unchecked<T>(&self, f: |*const libc::c_char| -> T) -> T {
with_c_str(*self, false, f)
(**self).with_c_str_unchecked(f)
}
}

Expand Down
12 changes: 0 additions & 12 deletions src/libstd/path/posix.rs
Expand Up @@ -106,18 +106,6 @@ impl ToCStr for Path {
}
}

impl<'a> ToCStr for &'a Path {
#[inline]
fn to_c_str(&self) -> CString {
(*self).to_c_str()
}

#[inline]
unsafe fn to_c_str_unchecked(&self) -> CString {
(*self).to_c_str_unchecked()
}
}

impl<S: hash::Writer> hash::Hash<S> for Path {
#[inline]
fn hash(&self, state: &mut S) {
Expand Down
12 changes: 0 additions & 12 deletions src/libstd/path/windows.rs
Expand Up @@ -130,18 +130,6 @@ impl ToCStr for Path {
}
}

impl<'a> ToCStr for &'a Path {
#[inline]
fn to_c_str(&self) -> CString {
(*self).to_c_str()
}

#[inline]
unsafe fn to_c_str_unchecked(&self) -> CString {
(*self).to_c_str_unchecked()
}
}

impl<S: hash::Writer> hash::Hash<S> for Path {
#[cfg(not(test))]
#[inline]
Expand Down

0 comments on commit 0547a40

Please sign in to comment.