Skip to content

Commit

Permalink
auto merge of #18463 : japaric/rust/bytes2, r=alexcrichton
Browse files Browse the repository at this point in the history
- The `BytesContainer::container_into_owned_bytes` method has been removed

- Methods that used to take `BytesContainer` implementors by value, now take them by reference. In particular, this breaks some uses of Path:

``` rust
Path::new("foo")  // Still works
path.join(another_path) -> path.join(&another_path)
```

[breaking-change]

---

Re: `container_into_owned_bytes`, I've removed it because

- Nothing in the whole repository uses it
- Takes `self` by value, which is incompatible with unsized types (`str`)

The alternative to removing this method is to split `BytesContainer` into `BytesContainer for Sized?` and `SizedBytesContainer: BytesContainer + Sized`, where the second trait only contains the `container_into_owned_bytes` method. I tried this alternative [in another branch](https://github.com/japaric/rust/commits/bytes) and it works, but it seemed better not to create a new trait for an unused method.

Re: Breakage of `Path` methods

We could use the idea that @alexcrichton proposed in #18457 (add blanket `impl BytesContainer for &T where T: BytesContainer` + keep taking `T: BytesContainer` by value in `Path` methods) to avoid breaking any code.

r? @aturon 
cc #16918
  • Loading branch information
bors committed Nov 3, 2014
2 parents 851799d + fe256f8 commit b9b396c
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 59 deletions.
2 changes: 1 addition & 1 deletion src/compiletest/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ fn find_rust_src_root(config: &Config) -> Option<Path> {
let path_postfix = Path::new("src/etc/lldb_batchmode.py");

while path.pop() {
if path.join(path_postfix.clone()).is_file() {
if path.join(&path_postfix).is_file() {
return Some(path);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/driver/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ pub fn build_session_(sopts: config::Options,
if path.is_absolute() {
path.clone()
} else {
os::getcwd().join(path.clone())
os::getcwd().join(&path)
}
);

Expand Down
2 changes: 1 addition & 1 deletion src/libstd/io/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ mod tests {
let prog = pwd_cmd().cwd(&parent_dir).spawn().unwrap();

let output = String::from_utf8(prog.wait_with_output().unwrap().output).unwrap();
let child_dir = Path::new(output.as_slice().trim().into_string());
let child_dir = Path::new(output.as_slice().trim());

let parent_stat = parent_dir.stat().unwrap();
let child_stat = child_dir.stat().unwrap();
Expand Down
55 changes: 30 additions & 25 deletions src/libstd/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ println!("path exists: {}", path.exists());

#![experimental]

use core::kinds::Sized;
use c_str::CString;
use clone::Clone;
use fmt;
Expand Down Expand Up @@ -626,7 +627,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
/// ```
#[inline]
fn push_many<T: BytesContainer>(&mut self, paths: &[T]) {
let t: Option<T> = None;
let t: Option<&T> = None;
if BytesContainer::is_str(t) {
for p in paths.iter() {
self.push(p.container_as_str().unwrap())
Expand Down Expand Up @@ -791,14 +792,9 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
}

/// A trait that represents something bytes-like (e.g. a &[u8] or a &str)
pub trait BytesContainer {
pub trait BytesContainer for Sized? {
/// Returns a &[u8] representing the receiver
fn container_as_bytes<'a>(&'a self) -> &'a [u8];
/// Consumes the receiver and converts it into Vec<u8>
#[inline]
fn container_into_owned_bytes(self) -> Vec<u8> {
self.container_as_bytes().to_vec()
}
/// Returns the receiver interpreted as a utf-8 string, if possible
#[inline]
fn container_as_str<'a>(&'a self) -> Option<&'a str> {
Expand All @@ -807,7 +803,7 @@ pub trait BytesContainer {
/// Returns whether .container_as_str() is guaranteed to not fail
// FIXME (#8888): Remove unused arg once ::<for T> works
#[inline]
fn is_str(_: Option<Self>) -> bool { false }
fn is_str(_: Option<&Self>) -> bool { false }
}

/// A trait that represents the unsafe operations on GenericPaths
Expand Down Expand Up @@ -859,48 +855,44 @@ impl<'a, P: GenericPath> Display<'a, P> {
}
}

impl<'a> BytesContainer for &'a str {
impl BytesContainer for str {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
fn container_as_bytes(&self) -> &[u8] {
self.as_bytes()
}
#[inline]
fn container_as_str<'a>(&'a self) -> Option<&'a str> {
Some(*self)
fn container_as_str(&self) -> Option<&str> {
Some(self)
}
#[inline]
fn is_str(_: Option<&'a str>) -> bool { true }
fn is_str(_: Option<&str>) -> bool { true }
}

impl BytesContainer for String {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
fn container_as_bytes(&self) -> &[u8] {
self.as_bytes()
}
#[inline]
fn container_as_str<'a>(&'a self) -> Option<&'a str> {
fn container_as_str(&self) -> Option<&str> {
Some(self.as_slice())
}
#[inline]
fn is_str(_: Option<String>) -> bool { true }
fn is_str(_: Option<&String>) -> bool { true }
}

impl<'a> BytesContainer for &'a [u8] {
impl BytesContainer for [u8] {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
*self
fn container_as_bytes(&self) -> &[u8] {
self
}
}

impl BytesContainer for Vec<u8> {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
fn container_as_bytes(&self) -> &[u8] {
self.as_slice()
}
#[inline]
fn container_into_owned_bytes(self) -> Vec<u8> {
self
}
}

impl BytesContainer for CString {
Expand All @@ -920,7 +912,20 @@ impl<'a> BytesContainer for str::MaybeOwned<'a> {
Some(self.as_slice())
}
#[inline]
fn is_str(_: Option<str::MaybeOwned>) -> bool { true }
fn is_str(_: Option<&str::MaybeOwned>) -> bool { true }
}

impl<'a, Sized? T: BytesContainer> BytesContainer for &'a T {
#[inline]
fn container_as_bytes(&self) -> &[u8] {
(**self).container_as_bytes()
}
#[inline]
fn container_as_str(&self) -> Option<&str> {
(**self).container_as_str()
}
#[inline]
fn is_str(_: Option<& &'a T>) -> bool { BytesContainer::is_str(None::<&T>) }
}

#[inline(always)]
Expand Down
11 changes: 0 additions & 11 deletions src/libstd/path/posix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,6 @@ impl BytesContainer for Path {
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
self.as_vec()
}
#[inline]
fn container_into_owned_bytes(self) -> Vec<u8> {
self.into_vec()
}
}

impl<'a> BytesContainer for &'a Path {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
self.as_vec()
}
}

impl GenericPathUnsafe for Path {
Expand Down
19 changes: 1 addition & 18 deletions src/libstd/path/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,23 +144,6 @@ impl<S: hash::Writer> hash::Hash<S> for Path {
}

impl BytesContainer for Path {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
self.as_vec()
}
#[inline]
fn container_into_owned_bytes(self) -> Vec<u8> {
self.into_vec()
}
#[inline]
fn container_as_str<'a>(&'a self) -> Option<&'a str> {
self.as_str()
}
#[inline]
fn is_str(_: Option<Path>) -> bool { true }
}

impl<'a> BytesContainer for &'a Path {
#[inline]
fn container_as_bytes<'a>(&'a self) -> &'a [u8] {
self.as_vec()
Expand All @@ -170,7 +153,7 @@ impl<'a> BytesContainer for &'a Path {
self.as_str()
}
#[inline]
fn is_str(_: Option<&'a Path>) -> bool { true }
fn is_str(_: Option<&Path>) -> bool { true }
}

impl GenericPathUnsafe for Path {
Expand Down
2 changes: 1 addition & 1 deletion src/libterm/terminfo/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn get_dbpath_for_term(term: &str) -> Option<Box<Path>> {
if i == "" {
dirs_to_search.push(Path::new("/usr/share/terminfo"));
} else {
dirs_to_search.push(Path::new(i.to_string()));
dirs_to_search.push(Path::new(i));
}
},
// Found nothing in TERMINFO_DIRS, use the default paths:
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/process-spawn-with-unicode-params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn main() {

let child_filestem = Path::new(child_name);
let child_filename = child_filestem.with_extension(my_ext);
let child_path = cwd.join(child_filename.clone());
let child_path = cwd.join(child_filename);

// make a separate directory for the child
drop(fs::mkdir(&cwd, io::USER_RWX).is_ok());
Expand Down

0 comments on commit b9b396c

Please sign in to comment.