diff options
| author | bors <bors@rust-lang.org> | 2014-01-29 09:46:34 -0800 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2014-01-29 09:46:34 -0800 |
| commit | c3ae182d5ce005e15d3d1f2906bd67cb65c61f8d (patch) | |
| tree | c45f623e349d8773561d27883dcdd7e60c627fa3 /src/libnative | |
| parent | e1580f6d09f0cd990c3eed55b1d6181af3258791 (diff) | |
| parent | c13a62593c218243b4d3b0e8be0b903c0315571b (diff) | |
| download | rust-c3ae182d5ce005e15d3d1f2906bd67cb65c61f8d.tar.gz rust-c3ae182d5ce005e15d3d1f2906bd67cb65c61f8d.zip | |
auto merge of #11754 : alexcrichton/rust/unused-result, r=brson
The general consensus is that we want to move away from conditions for I/O, and I propose a two-step plan for doing so:
1. Warn about unused `Result` types. When all of I/O returns `Result`, it will require you inspect the return value for an error *only if* you have a result you want to look at. By default, for things like `write` returning `Result<(), Error>`, these will all go silently ignored. This lint will prevent blind ignorance of these return values, letting you know that there's something you should do about them.
2. Implement a `try!` macro:
```
macro_rules! try( ($e:expr) => (match $e { Ok(e) => e, Err(e) => return Err(e) }) )
```
With these two tools combined, I feel that we get almost all the benefits of conditions. The first step (the lint) is a sanity check that you're not ignoring return values at callsites. The second step is to provide a convenience method of returning early out of a sequence of computations. After thinking about this for awhile, I don't think that we need the so-called "do-notation" in the compiler itself because I think it's just *too* specialized. Additionally, the `try!` macro is super lightweight, easy to understand, and works almost everywhere. As soon as you want to do something more fancy, my answer is "use match".
Basically, with these two tools in action, I would be comfortable removing conditions. What do others think about this strategy?
----
This PR specifically implements the `unused_result` lint. I actually added two lints, `unused_result` and `unused_must_use`, and the first commit has the rationale for why `unused_result` is turned off by default.
Diffstat (limited to 'src/libnative')
| -rw-r--r-- | src/libnative/io/file.rs | 22 | ||||
| -rw-r--r-- | src/libnative/io/process.rs | 2 | ||||
| -rw-r--r-- | src/libnative/io/timer_helper.rs | 2 | ||||
| -rw-r--r-- | src/libnative/io/timer_other.rs | 2 | ||||
| -rw-r--r-- | src/libnative/io/timer_timerfd.rs | 4 |
5 files changed, 15 insertions, 17 deletions
diff --git a/src/libnative/io/file.rs b/src/libnative/io/file.rs index 53386433d53..acab7ce3a91 100644 --- a/src/libnative/io/file.rs +++ b/src/libnative/io/file.rs @@ -118,7 +118,10 @@ impl io::Reader for FileDesc { impl io::Writer for FileDesc { fn write(&mut self, buf: &[u8]) { - self.inner_write(buf); + match self.inner_write(buf) { + Ok(()) => {} + Err(e) => { io::io_error::cond.raise(e); } + } } } @@ -276,7 +279,7 @@ impl rtio::RtioFileStream for FileDesc { _ => Ok(()) } }; - self.seek(orig_pos as i64, io::SeekSet); + let _ = self.seek(orig_pos as i64, io::SeekSet); return ret; } #[cfg(unix)] @@ -383,12 +386,10 @@ impl rtio::RtioFileStream for CFile { } fn pread(&mut self, buf: &mut [u8], offset: u64) -> Result<int, IoError> { - self.flush(); - self.fd.pread(buf, offset) + self.flush().and_then(|()| self.fd.pread(buf, offset)) } fn pwrite(&mut self, buf: &[u8], offset: u64) -> Result<(), IoError> { - self.flush(); - self.fd.pwrite(buf, offset) + self.flush().and_then(|()| self.fd.pwrite(buf, offset)) } fn seek(&mut self, pos: i64, style: io::SeekStyle) -> Result<u64, IoError> { let whence = match style { @@ -412,16 +413,13 @@ impl rtio::RtioFileStream for CFile { } } fn fsync(&mut self) -> Result<(), IoError> { - self.flush(); - self.fd.fsync() + self.flush().and_then(|()| self.fd.fsync()) } fn datasync(&mut self) -> Result<(), IoError> { - self.flush(); - self.fd.fsync() + self.flush().and_then(|()| self.fd.fsync()) } fn truncate(&mut self, offset: i64) -> Result<(), IoError> { - self.flush(); - self.fd.truncate(offset) + self.flush().and_then(|()| self.fd.truncate(offset)) } } diff --git a/src/libnative/io/process.rs b/src/libnative/io/process.rs index ee4ee295055..13dd4298777 100644 --- a/src/libnative/io/process.rs +++ b/src/libnative/io/process.rs @@ -486,7 +486,7 @@ fn spawn_process_os(prog: &str, args: &[~str], (errno << 8) as u8, (errno << 0) as u8, ]; - output.inner_write(bytes); + assert!(output.inner_write(bytes).is_ok()); intrinsics::abort(); }) } diff --git a/src/libnative/io/timer_helper.rs b/src/libnative/io/timer_helper.rs index 9fc18220386..74759b467d4 100644 --- a/src/libnative/io/timer_helper.rs +++ b/src/libnative/io/timer_helper.rs @@ -101,7 +101,7 @@ mod imp { } pub fn signal(fd: libc::c_int) { - FileDesc::new(fd, false).inner_write([0]); + FileDesc::new(fd, false).inner_write([0]).unwrap(); } pub fn close(fd: libc::c_int) { diff --git a/src/libnative/io/timer_other.rs b/src/libnative/io/timer_other.rs index 4a62a400c8d..bc005f2fe8d 100644 --- a/src/libnative/io/timer_other.rs +++ b/src/libnative/io/timer_other.rs @@ -187,7 +187,7 @@ fn helper(input: libc::c_int, messages: Port<Req>) { // drain the file descriptor let mut buf = [0]; - fd.inner_read(buf); + fd.inner_read(buf).unwrap(); } -1 if os::errno() == libc::EINTR as int => {} diff --git a/src/libnative/io/timer_timerfd.rs b/src/libnative/io/timer_timerfd.rs index 2bcaf4d5c7c..1888b8578a0 100644 --- a/src/libnative/io/timer_timerfd.rs +++ b/src/libnative/io/timer_timerfd.rs @@ -98,7 +98,7 @@ fn helper(input: libc::c_int, messages: Port<Req>) { if fd == input { let mut buf = [0, ..1]; // drain the input file descriptor of its input - FileDesc::new(fd, false).inner_read(buf); + FileDesc::new(fd, false).inner_read(buf).unwrap(); incoming = true; } else { let mut bits = [0, ..8]; @@ -106,7 +106,7 @@ fn helper(input: libc::c_int, messages: Port<Req>) { // // FIXME: should this perform a send() this number of // times? - FileDesc::new(fd, false).inner_read(bits); + FileDesc::new(fd, false).inner_read(bits).unwrap(); let remove = { match map.find(&fd).expect("fd unregistered") { &(ref c, oneshot) => !c.try_send(()) || oneshot |
