Auto merge of rust-lang#23316 - alexcrichton:less-question-sized, r=aturon

It is a frequent pattern among I/O functions to take `P: AsPath + ?Sized` or
`AsOsStr` instead of `AsPath`. Most of these functions do not need to take
ownership of their argument, but for libraries in general it's much more
ergonomic to not deal with `?Sized` at all and simply require an argument `P`
instead of `&P`.

This change is aimed at removing unsightly `?Sized` bounds while retaining the
same level of usability as before. All affected functions now take ownership of
their arguments instead of taking them by reference, but due to the forwarding
implementations of `AsOsStr` and `AsPath` all code should continue to work as it
did before.

This is strictly speaking a breaking change due to the signatures of these
functions changing, but normal idiomatic usage of these APIs should not break in
practice.

[breaking-change]

Author: bors

src/libstd/fs/mod.rs

@@ -124,7 +124,7 @@ impl File {
     /// This function will return an error if `path` does not already exist.
     /// Other errors may also be returned according to `OpenOptions::open`.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn open<P: AsPath + ?Sized>(path: &P) -> io::Result<File> {
+    pub fn open<P: AsPath>(path: P) -> io::Result<File> {
         OpenOptions::new().read(true).open(path)
     }
 
@@ -135,7 +135,7 @@ impl File {
     ///
     /// See the `OpenOptions::open` function for more details.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn create<P: AsPath + ?Sized>(path: &P) -> io::Result<File> {
+    pub fn create<P: AsPath>(path: P) -> io::Result<File> {
         OpenOptions::new().write(true).create(true).truncate(true).open(path)
     }
 
@@ -297,7 +297,7 @@ impl OpenOptions {
     ///   permissions for
     /// * Filesystem-level errors (full disk, etc)
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn open<P: AsPath + ?Sized>(&self, path: &P) -> io::Result<File> {
+    pub fn open<P: AsPath>(&self, path: P) -> io::Result<File> {
         let path = path.as_path();
         let inner = try!(fs_imp::File::open(path, &self.0));
         Ok(File { path: path.to_path_buf(), inner: inner })
@@ -410,7 +410,7 @@ impl DirEntry {
 /// user lacks permissions to remove the file, or if some other filesystem-level
 /// error occurs.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn remove_file<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
+pub fn remove_file<P: AsPath>(path: P) -> io::Result<()> {
     fs_imp::unlink(path.as_path())
 }
 
@@ -438,7 +438,7 @@ pub fn remove_file<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
 /// permissions to perform a `metadata` call on the given `path` or if there
 /// is no entry in the filesystem at the provided path.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn metadata<P: AsPath + ?Sized>(path: &P) -> io::Result<Metadata> {
+pub fn metadata<P: AsPath>(path: P) -> io::Result<Metadata> {
     fs_imp::stat(path.as_path()).map(Metadata)
 }
 
@@ -459,8 +459,7 @@ pub fn metadata<P: AsPath + ?Sized>(path: &P) -> io::Result<Metadata> {
 /// reside on separate filesystems, or if some other intermittent I/O error
 /// occurs.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn rename<P: AsPath + ?Sized, Q: AsPath + ?Sized>(from: &P, to: &Q)
-                                                      -> io::Result<()> {
+pub fn rename<P: AsPath, Q: AsPath>(from: P, to: Q) -> io::Result<()> {
     fs_imp::rename(from.as_path(), to.as_path())
 }
 
@@ -490,9 +489,9 @@ pub fn rename<P: AsPath + ?Sized, Q: AsPath + ?Sized>(from: &P, to: &Q)
 /// * The current process does not have the permission rights to access
 ///   `from` or write `to`
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn copy<P: AsPath + ?Sized, Q: AsPath + ?Sized>(from: &P, to: &Q)
-                                                    -> io::Result<u64> {
+pub fn copy<P: AsPath, Q: AsPath>(from: P, to: Q) -> io::Result<u64> {
     let from = from.as_path();
+    let to = to.as_path();
     if !from.is_file() {
         return Err(Error::new(ErrorKind::MismatchedFileTypeForOperation,
                               "the source path is not an existing file",
@@ -513,17 +512,15 @@ pub fn copy<P: AsPath + ?Sized, Q: AsPath + ?Sized>(from: &P, to: &Q)
 /// The `dst` path will be a link pointing to the `src` path. Note that systems
 /// often require these two paths to both be located on the same filesystem.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn hard_link<P: AsPath + ?Sized, Q: AsPath + ?Sized>(src: &P, dst: &Q)
-                                                         -> io::Result<()> {
+pub fn hard_link<P: AsPath, Q: AsPath>(src: P, dst: Q) -> io::Result<()> {
     fs_imp::link(src.as_path(), dst.as_path())
 }
 
 /// Creates a new soft link on the filesystem.
 ///
 /// The `dst` path will be a soft link pointing to the `src` path.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn soft_link<P: AsPath + ?Sized, Q: AsPath + ?Sized>(src: &P, dst: &Q)
-                                                         -> io::Result<()> {
+pub fn soft_link<P: AsPath, Q: AsPath>(src: P, dst: Q) -> io::Result<()> {
     fs_imp::symlink(src.as_path(), dst.as_path())
 }
 
@@ -535,7 +532,7 @@ pub fn soft_link<P: AsPath + ?Sized, Q: AsPath + ?Sized>(src: &P, dst: &Q)
 /// reading a file that does not exist or reading a file that is not a soft
 /// link.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn read_link<P: AsPath + ?Sized>(path: &P) -> io::Result<PathBuf> {
+pub fn read_link<P: AsPath>(path: P) -> io::Result<PathBuf> {
     fs_imp::readlink(path.as_path())
 }
 
@@ -554,7 +551,7 @@ pub fn read_link<P: AsPath + ?Sized>(path: &P) -> io::Result<PathBuf> {
 /// This function will return an error if the user lacks permissions to make a
 /// new directory at the provided `path`, or if the directory already exists.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn create_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
+pub fn create_dir<P: AsPath>(path: P) -> io::Result<()> {
     fs_imp::mkdir(path.as_path())
 }
 
@@ -568,7 +565,7 @@ pub fn create_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
 /// error conditions for when a directory is being created (after it is
 /// determined to not exist) are outlined by `fs::create_dir`.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn create_dir_all<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
+pub fn create_dir_all<P: AsPath>(path: P) -> io::Result<()> {
     let path = path.as_path();
     if path.is_dir() { return Ok(()) }
     if let Some(p) = path.parent() { try!(create_dir_all(p)) }
@@ -590,7 +587,7 @@ pub fn create_dir_all<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
 /// This function will return an error if the user lacks permissions to remove
 /// the directory at the provided `path`, or if the directory isn't empty.
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn remove_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
+pub fn remove_dir<P: AsPath>(path: P) -> io::Result<()> {
     fs_imp::rmdir(path.as_path())
 }
 
@@ -604,7 +601,7 @@ pub fn remove_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
 ///
 /// See `file::remove_file` and `fs::remove_dir`
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn remove_dir_all<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
+pub fn remove_dir_all<P: AsPath>(path: P) -> io::Result<()> {
     let path = path.as_path();
     for child in try!(read_dir(path)) {
         let child = try!(child).path();
@@ -657,7 +654,7 @@ pub fn remove_dir_all<P: AsPath + ?Sized>(path: &P) -> io::Result<()> {
 /// the process lacks permissions to view the contents or if the `path` points
 /// at a non-directory file
 #[stable(feature = "rust1", since = "1.0.0")]
-pub fn read_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<ReadDir> {
+pub fn read_dir<P: AsPath>(path: P) -> io::Result<ReadDir> {
     fs_imp::readdir(path.as_path()).map(ReadDir)
 }
 
@@ -673,7 +670,7 @@ pub fn read_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<ReadDir> {
            reason = "the precise semantics and defaults for a recursive walk \
                      may change and this may end up accounting for files such \
                      as symlinks differently")]
-pub fn walk_dir<P: AsPath + ?Sized>(path: &P) -> io::Result<WalkDir> {
+pub fn walk_dir<P: AsPath>(path: P) -> io::Result<WalkDir> {
     let start = try!(read_dir(path));
     Ok(WalkDir { cur: Some(start), stack: Vec::new() })
 }
@@ -759,8 +756,8 @@ impl PathExt for Path {
            reason = "the argument type of u64 is not quite appropriate for \
                      this function and may change if the standard library \
                      gains a type to represent a moment in time")]
-pub fn set_file_times<P: AsPath + ?Sized>(path: &P, accessed: u64,
-                                          modified: u64) -> io::Result<()> {
+pub fn set_file_times<P: AsPath>(path: P, accessed: u64,
+                                 modified: u64) -> io::Result<()> {
     fs_imp::utimes(path.as_path(), accessed, modified)
 }
 
@@ -788,8 +785,7 @@ pub fn set_file_times<P: AsPath + ?Sized>(path: &P, accessed: u64,
            reason = "a more granual ability to set specific permissions may \
                      be exposed on the Permissions structure itself and this \
                      method may not always exist")]
-pub fn set_permissions<P: AsPath + ?Sized>(path: &P, perm: Permissions)
-                                           -> io::Result<()> {
+pub fn set_permissions<P: AsPath>(path: P, perm: Permissions) -> io::Result<()> {
     fs_imp::set_perm(path.as_path(), perm.0)
 }
 

src/libstd/path.rs

@@ -877,7 +877,7 @@ impl PathBuf {
     /// Allocate a `PathBuf` with initial contents given by the
     /// argument.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn new<S: ?Sized + AsOsStr>(s: &S) -> PathBuf {
+    pub fn new<S: AsOsStr>(s: S) -> PathBuf {
         PathBuf { inner: s.as_os_str().to_os_string() }
     }
 
@@ -891,7 +891,7 @@ impl PathBuf {
     ///   replaces everything except for the prefix (if any) of `self`.
     /// * if `path` has a prefix but no root, it replaces `self.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn push<P: ?Sized>(&mut self, path: &P) where P: AsPath {
+    pub fn push<P: AsPath>(&mut self, path: P) {
         let path = path.as_path();
 
         // in general, a separator is needed if the rightmost byte is not a separator
@@ -959,7 +959,7 @@ impl PathBuf {
     /// assert!(buf == PathBuf::new("/baz.txt"));
     /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn set_file_name<S: ?Sized>(&mut self, file_name: &S) where S: AsOsStr {
+    pub fn set_file_name<S: AsOsStr>(&mut self, file_name: S) {
         if self.file_name().is_some() {
             let popped = self.pop();
             debug_assert!(popped);
@@ -974,7 +974,7 @@ impl PathBuf {
     /// Otherwise, returns `true`; if `self.extension()` is `None`, the extension
     /// is added; otherwise it is replaced.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn set_extension<S: ?Sized + AsOsStr>(&mut self, extension: &S) -> bool {
+    pub fn set_extension<S: AsOsStr>(&mut self, extension: S) -> bool {
         if self.file_name().is_none() { return false; }
 
         let mut stem = match self.file_stem() {
@@ -1000,17 +1000,17 @@ impl PathBuf {
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
-impl<'a, P: ?Sized + 'a> iter::FromIterator<&'a P> for PathBuf where P: AsPath {
-    fn from_iter<I: IntoIterator<Item = &'a P>>(iter: I) -> PathBuf {
+impl<P: AsPath> iter::FromIterator<P> for PathBuf {
+    fn from_iter<I: IntoIterator<Item = P>>(iter: I) -> PathBuf {
         let mut buf = PathBuf::new("");
         buf.extend(iter);
         buf
     }
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
-impl<'a, P: ?Sized + 'a> iter::Extend<&'a P> for PathBuf where P: AsPath {
-    fn extend<I: IntoIterator<Item = &'a P>>(&mut self, iter: I) {
+impl<P: AsPath> iter::Extend<P> for PathBuf {
+    fn extend<I: IntoIterator<Item = P>>(&mut self, iter: I) {
         for p in iter {
             self.push(p)
         }
@@ -1253,13 +1253,13 @@ impl Path {
 
     /// Determines whether `base` is a prefix of `self`.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn starts_with<P: ?Sized>(&self, base: &P) -> bool where P: AsPath {
+    pub fn starts_with<P: AsPath>(&self, base: P) -> bool {
         iter_after(self.components(), base.as_path().components()).is_some()
     }
 
     /// Determines whether `child` is a suffix of `self`.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn ends_with<P: ?Sized>(&self, child: &P) -> bool where P: AsPath {
+    pub fn ends_with<P: AsPath>(&self, child: P) -> bool {
         iter_after(self.components().rev(), child.as_path().components().rev()).is_some()
     }
 
@@ -1293,7 +1293,7 @@ impl Path {
     ///
     /// See `PathBuf::push` for more details on what it means to adjoin a path.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn join<P: ?Sized>(&self, path: &P) -> PathBuf where P: AsPath {
+    pub fn join<P: AsPath>(&self, path: P) -> PathBuf {
         let mut buf = self.to_path_buf();
         buf.push(path);
         buf
@@ -1303,7 +1303,7 @@ impl Path {
     ///
     /// See `PathBuf::set_file_name` for more details.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn with_file_name<S: ?Sized>(&self, file_name: &S) -> PathBuf where S: AsOsStr {
+    pub fn with_file_name<S: AsOsStr>(&self, file_name: S) -> PathBuf {
         let mut buf = self.to_path_buf();
         buf.set_file_name(file_name);
         buf
@@ -1313,7 +1313,7 @@ impl Path {
     ///
     /// See `PathBuf::set_extension` for more details.
     #[stable(feature = "rust1", since = "1.0.0")]
-    pub fn with_extension<S: ?Sized>(&self, extension: &S) -> PathBuf where S: AsOsStr {
+    pub fn with_extension<S: AsOsStr>(&self, extension: S) -> PathBuf {
         let mut buf = self.to_path_buf();
         buf.set_extension(extension);
         buf

src/libstd/process.rs

@@ -147,7 +147,7 @@ impl Command {
     /// Builder methods are provided to change these defaults and
     /// otherwise configure the process.
     #[stable(feature = "process", since = "1.0.0")]
-    pub fn new<S: AsOsStr + ?Sized>(program: &S) -> Command {
+    pub fn new<S: AsOsStr>(program: S) -> Command {
         Command {
             inner: CommandImp::new(program.as_os_str()),
             stdin: None,
@@ -158,7 +158,7 @@ impl Command {
 
     /// Add an argument to pass to the program.
     #[stable(feature = "process", since = "1.0.0")]
-    pub fn arg<S: AsOsStr + ?Sized>(&mut self, arg: &S) -> &mut Command {
+    pub fn arg<S: AsOsStr>(&mut self, arg: S) -> &mut Command {
         self.inner.arg(arg.as_os_str());
         self
     }
@@ -175,7 +175,7 @@ impl Command {
     /// Note that environment variable names are case-insensitive (but case-preserving) on Windows,
     /// and case-sensitive on all other platforms.
     #[stable(feature = "process", since = "1.0.0")]
-    pub fn env<K: ?Sized, V: ?Sized>(&mut self, key: &K, val: &V) -> &mut Command
+    pub fn env<K, V>(&mut self, key: K, val: V) -> &mut Command
         where K: AsOsStr, V: AsOsStr
     {
         self.inner.env(key.as_os_str(), val.as_os_str());
@@ -184,7 +184,7 @@ impl Command {
 
     /// Removes an environment variable mapping.
     #[stable(feature = "process", since = "1.0.0")]
-    pub fn env_remove<K: ?Sized + AsOsStr>(&mut self, key: &K) -> &mut Command {
+    pub fn env_remove<K: AsOsStr>(&mut self, key: K) -> &mut Command {
         self.inner.env_remove(key.as_os_str());
         self
     }
@@ -198,7 +198,7 @@ impl Command {
 
     /// Set the working directory for the child process.
     #[stable(feature = "process", since = "1.0.0")]
-    pub fn current_dir<P: AsPath + ?Sized>(&mut self, dir: &P) -> &mut Command {
+    pub fn current_dir<P: AsPath>(&mut self, dir: P) -> &mut Command {
         self.inner.cwd(dir.as_path().as_os_str());
         self
     }

src/libstd/sys/unix/os.rs

@@ -253,7 +253,7 @@ pub fn current_exe() -> io::Result<PathBuf> {
         let err = _NSGetExecutablePath(v.as_mut_ptr() as *mut i8, &mut sz);
         if err != 0 { return Err(io::Error::last_os_error()); }
         v.set_len(sz as uint - 1); // chop off trailing NUL
-        Ok(PathBuf::new::<OsString>(&OsStringExt::from_vec(v)))
+        Ok(PathBuf::new(OsString::from_vec(v)))
     }
 }