From 454882dcb7fdb03867d695a88335e2d2c8f7561a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Feb 2014 19:02:10 -0800 Subject: Remove std::condition This has been a long time coming. Conditions in rust were initially envisioned as being a good alternative to error code return pattern. The idea is that all errors are fatal-by-default, and you can opt-in to handling the error by registering an error handler. While sounding nice, conditions ended up having some unforseen shortcomings: * Actually handling an error has some very awkward syntax: let mut result = None; let mut answer = None; io::io_error::cond.trap(|e| { result = Some(e) }).inside(|| { answer = Some(some_io_operation()); }); match result { Some(err) => { /* hit an I/O error */ } None => { let answer = answer.unwrap(); /* deal with the result of I/O */ } } This pattern can certainly use functions like io::result, but at its core actually handling conditions is fairly difficult * The "zero value" of a function is often confusing. One of the main ideas behind using conditions was to change the signature of I/O functions. Instead of read_be_u32() returning a result, it returned a u32. Errors were notified via a condition, and if you caught the condition you understood that the "zero value" returned is actually a garbage value. These zero values are often difficult to understand, however. One case of this is the read_bytes() function. The function takes an integer length of the amount of bytes to read, and returns an array of that size. The array may actually be shorter, however, if an error occurred. Another case is fs::stat(). The theoretical "zero value" is a blank stat struct, but it's a little awkward to create and return a zero'd out stat struct on a call to stat(). In general, the return value of functions that can raise error are much more natural when using a Result as opposed to an always-usable zero-value. * Conditions impose a necessary runtime requirement on *all* I/O. In theory I/O is as simple as calling read() and write(), but using conditions imposed the restriction that a rust local task was required if you wanted to catch errors with I/O. While certainly an surmountable difficulty, this was always a bit of a thorn in the side of conditions. * Functions raising conditions are not always clear that they are raising conditions. This suffers a similar problem to exceptions where you don't actually know whether a function raises a condition or not. The documentation likely explains, but if someone retroactively adds a condition to a function there's nothing forcing upstream users to acknowledge a new point of task failure. * Libaries using I/O are not guaranteed to correctly raise on conditions when an error occurs. In developing various I/O libraries, it's much easier to just return `None` from a read rather than raising an error. The silent contract of "don't raise on EOF" was a little difficult to understand and threw a wrench into the answer of the question "when do I raise a condition?" Many of these difficulties can be overcome through documentation, examples, and general practice. In the end, all of these difficulties added together ended up being too overwhelming and improving various aspects didn't end up helping that much. A result-based I/O error handling strategy also has shortcomings, but the cognitive burden is much smaller. The tooling necessary to make this strategy as usable as conditions were is much smaller than the tooling necessary for conditions. Perhaps conditions may manifest themselves as a future entity, but for now we're going to remove them from the standard library. Closes #9795 Closes #8968 --- src/libstd/path/mod.rs | 85 ++++++++++++-------------------------------- src/libstd/path/posix.rs | 86 +++++++------------------------------------- src/libstd/path/windows.rs | 88 +++++++--------------------------------------- 3 files changed, 48 insertions(+), 211 deletions(-) (limited to 'src/libstd/path') diff --git a/src/libstd/path/mod.rs b/src/libstd/path/mod.rs index 4aa4a3feab1..f3f70c263ec 100644 --- a/src/libstd/path/mod.rs +++ b/src/libstd/path/mod.rs @@ -147,12 +147,6 @@ pub use is_sep_byte = self::windows::is_sep_byte; pub mod posix; pub mod windows; -// Condition that is raised when a NUL is found in a byte vector given to a Path function -condition! { - // this should be a &[u8] but there's a lifetime issue - null_byte: ~[u8] -> ~[u8]; -} - /// A trait that represents the generic operations available on paths pub trait GenericPath: Clone + GenericPathUnsafe { /// Creates a new Path from a byte vector or string. @@ -160,18 +154,13 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the path contains a NUL. + /// Fails the task if the path contains a NUL. /// /// See individual Path impls for additional restrictions. #[inline] fn new(path: T) -> Self { - if contains_nul(path.container_as_bytes()) { - let path = self::null_byte::cond.raise(path.container_into_owned_bytes()); - assert!(!contains_nul(path)); - unsafe { GenericPathUnsafe::new_unchecked(path) } - } else { - unsafe { GenericPathUnsafe::new_unchecked(path) } - } + assert!(!contains_nul(path.container_as_bytes())); + unsafe { GenericPathUnsafe::new_unchecked(path) } } /// Creates a new Path from a byte vector or string, if possible. @@ -283,16 +272,11 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the filename contains a NUL. + /// Fails the task if the filename contains a NUL. #[inline] fn set_filename(&mut self, filename: T) { - if contains_nul(filename.container_as_bytes()) { - let filename = self::null_byte::cond.raise(filename.container_into_owned_bytes()); - assert!(!contains_nul(filename)); - unsafe { self.set_filename_unchecked(filename) } - } else { - unsafe { self.set_filename_unchecked(filename) } - } + assert!(!contains_nul(filename.container_as_bytes())); + unsafe { self.set_filename_unchecked(filename) } } /// Replaces the extension with the given byte vector or string. /// If there is no extension in `self`, this adds one. @@ -301,8 +285,9 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the extension contains a NUL. + /// Fails the task if the extension contains a NUL. fn set_extension(&mut self, extension: T) { + assert!(!contains_nul(extension.container_as_bytes())); // borrowck causes problems here too let val = { match self.filename() { @@ -315,21 +300,11 @@ pub trait GenericPath: Clone + GenericPathUnsafe { None } else { let mut v; - if contains_nul(extension.container_as_bytes()) { - let ext = extension.container_into_owned_bytes(); - let extension = self::null_byte::cond.raise(ext); - assert!(!contains_nul(extension)); - v = vec::with_capacity(name.len() + extension.len() + 1); - v.push_all(name); - v.push(dot); - v.push_all(extension); - } else { - let extension = extension.container_as_bytes(); - v = vec::with_capacity(name.len() + extension.len() + 1); - v.push_all(name); - v.push(dot); - v.push_all(extension); - } + let extension = extension.container_as_bytes(); + v = vec::with_capacity(name.len() + extension.len() + 1); + v.push_all(name); + v.push(dot); + v.push_all(extension); Some(v) } } @@ -338,19 +313,10 @@ pub trait GenericPath: Clone + GenericPathUnsafe { Some(name.slice_to(idx).to_owned()) } else { let mut v; - if contains_nul(extension.container_as_bytes()) { - let ext = extension.container_into_owned_bytes(); - let extension = self::null_byte::cond.raise(ext); - assert!(!contains_nul(extension)); - v = vec::with_capacity(idx + extension.len() + 1); - v.push_all(name.slice_to(idx+1)); - v.push_all(extension); - } else { - let extension = extension.container_as_bytes(); - v = vec::with_capacity(idx + extension.len() + 1); - v.push_all(name.slice_to(idx+1)); - v.push_all(extension); - } + let extension = extension.container_as_bytes(); + v = vec::with_capacity(idx + extension.len() + 1); + v.push_all(name.slice_to(idx+1)); + v.push_all(extension); Some(v) } } @@ -370,7 +336,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the filename contains a NUL. + /// Fails the task if the filename contains a NUL. #[inline] fn with_filename(&self, filename: T) -> Self { let mut p = self.clone(); @@ -383,7 +349,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the extension contains a NUL. + /// Fails the task if the extension contains a NUL. #[inline] fn with_extension(&self, extension: T) -> Self { let mut p = self.clone(); @@ -408,16 +374,11 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the path contains a NUL. + /// Fails the task if the path contains a NUL. #[inline] fn push(&mut self, path: T) { - if contains_nul(path.container_as_bytes()) { - let path = self::null_byte::cond.raise(path.container_into_owned_bytes()); - assert!(!contains_nul(path)); - unsafe { self.push_unchecked(path) } - } else { - unsafe { self.push_unchecked(path) } - } + assert!(!contains_nul(path.container_as_bytes())); + unsafe { self.push_unchecked(path) } } /// Pushes multiple paths (as byte vectors or strings) onto `self`. /// See `push` for details. @@ -445,7 +406,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe { /// /// # Failure /// - /// Raises the `null_byte` condition if the path contains a NUL. + /// Fails the task if the path contains a NUL. #[inline] fn join(&self, path: T) -> Self { let mut p = self.clone(); diff --git a/src/libstd/path/posix.rs b/src/libstd/path/posix.rs index ba0cd0bb521..6970ebfb15d 100644 --- a/src/libstd/path/posix.rs +++ b/src/libstd/path/posix.rs @@ -318,7 +318,7 @@ impl Path { /// /// # Failure /// - /// Raises the `null_byte` condition if the vector contains a NUL. + /// Fails the task if the vector contains a NUL. #[inline] pub fn new(path: T) -> Path { GenericPath::new(path) @@ -527,83 +527,21 @@ mod tests { #[test] fn test_null_byte() { - use path::null_byte::cond; - - let mut handled = false; - let mut p = cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("foo/bar", 0)); - (b!("/bar").to_owned()) - }).inside(|| { + use task; + let result = task::try(proc() { Path::new(b!("foo/bar", 0)) }); - assert!(handled); - assert_eq!(p.as_vec(), b!("/bar")); - - handled = false; - cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("f", 0, "o")); - (b!("foo").to_owned()) - }).inside(|| { - p.set_filename(b!("f", 0, "o")) - }); - assert!(handled); - assert_eq!(p.as_vec(), b!("/foo")); - - handled = false; - cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("f", 0, "o")); - (b!("foo").to_owned()) - }).inside(|| { - p.push(b!("f", 0, "o")); - }); - assert!(handled); - assert_eq!(p.as_vec(), b!("/foo/foo")); - } - - #[test] - fn test_null_byte_fail() { - use path::null_byte::cond; - use task; + assert!(result.is_err()); - macro_rules! t( - ($name:expr => $code:expr) => ( - { - let mut t = task::task(); - t.name($name); - let res = t.try(proc() $code); - assert!(res.is_err()); - } - ) - ) + let result = task::try(proc() { + Path::new("test").set_filename(b!("f", 0, "o")) + }); + assert!(result.is_err()); - t!(~"new() w/nul" => { - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - Path::new(b!("foo/bar", 0)) - }); - }) - - t!(~"set_filename w/nul" => { - let mut p = Path::new(b!("foo/bar")); - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - p.set_filename(b!("foo", 0)) - }); - }) - - t!(~"push w/nul" => { - let mut p = Path::new(b!("foo/bar")); - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - p.push(b!("foo", 0)) - }); - }) + let result = task::try(proc() { + Path::new("test").push(b!("f", 0, "o")); + }); + assert!(result.is_err()); } #[test] diff --git a/src/libstd/path/windows.rs b/src/libstd/path/windows.rs index eec6f37b627..90154adb7fe 100644 --- a/src/libstd/path/windows.rs +++ b/src/libstd/path/windows.rs @@ -590,7 +590,7 @@ impl Path { /// /// # Failure /// - /// Raises the `null_byte` condition if the vector contains a NUL. + /// Fails the task if the vector contains a NUL. /// Fails if invalid UTF-8. #[inline] pub fn new(path: T) -> Path { @@ -1248,83 +1248,21 @@ mod tests { #[test] fn test_null_byte() { - use path::null_byte::cond; - - let mut handled = false; - let mut p = cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("foo\\bar", 0)); - (b!("\\bar").to_owned()) - }).inside(|| { - Path::new(b!("foo\\bar", 0)) - }); - assert!(handled); - assert_eq!(p.as_vec(), b!("\\bar")); - - handled = false; - cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("f", 0, "o")); - (b!("foo").to_owned()) - }).inside(|| { - p.set_filename(b!("f", 0, "o")) - }); - assert!(handled); - assert_eq!(p.as_vec(), b!("\\foo")); - - handled = false; - cond.trap(|v| { - handled = true; - assert_eq!(v.as_slice(), b!("f", 0, "o")); - (b!("foo").to_owned()) - }).inside(|| { - p.push(b!("f", 0, "o")); - }); - assert!(handled); - assert_eq!(p.as_vec(), b!("\\foo\\foo")); - } - - #[test] - fn test_null_byte_fail() { - use path::null_byte::cond; use task; + let result = task::try(proc() { + Path::new(b!("foo/bar", 0)) + }); + assert!(result.is_err()); - macro_rules! t( - ($name:expr => $code:expr) => ( - { - let mut t = task::task(); - t.name($name); - let res = t.try(proc() $code); - assert!(res.is_err()); - } - ) - ) - - t!(~"from_vec() w\\nul" => { - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - Path::new(b!("foo\\bar", 0)) - }); - }) - - t!(~"set_filename w\\nul" => { - let mut p = Path::new(b!("foo\\bar")); - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - p.set_filename(b!("foo", 0)) - }); - }) + let result = task::try(proc() { + Path::new("test").set_filename(b!("f", 0, "o")) + }); + assert!(result.is_err()); - t!(~"push w\\nul" => { - let mut p = Path::new(b!("foo\\bar")); - cond.trap(|_| { - (b!("null", 0).to_owned()) - }).inside(|| { - p.push(b!("foo", 0)) - }); - }) + let result = task::try(proc() { + Path::new("test").push(b!("f", 0, "o")); + }); + assert!(result.is_err()); } #[test] -- cgit 1.4.1-3-g733a5