From d97c80be63f7cdd46f7bd8d9a17e92a698c51a89 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Sun, 27 Aug 2017 10:14:25 -0400 Subject: termcolor: make StandardStream be Send This commit fixes a bug where the `StandardStream` type isn't `Send` on Windows. This can cause some surprising compile breakage, and the only motivation for it being non-Send was dubious. Namely, it was a result of trying to eliminate code duplication. This refactoring also eliminates at least one "unreachable" panic case that was a result of trying to eliminate code reuse, so that's a nice benefit as well. Fixes #503 --- termcolor/src/lib.rs | 125 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 37 deletions(-) diff --git a/termcolor/src/lib.rs b/termcolor/src/lib.rs index 987bc32e..b07071e9 100644 --- a/termcolor/src/lib.rs +++ b/termcolor/src/lib.rs @@ -265,7 +265,7 @@ impl<'a> io::Write for IoStandardStreamLock<'a> { /// Satisfies `io::Write` and `WriteColor`, and supports optional coloring /// to either of the standard output streams, stdout and stderr. pub struct StandardStream { - wtr: LossyStandardStream>, + wtr: LossyStandardStream>, } /// `StandardStreamLock` is a locked reference to a `StandardStream`. @@ -276,12 +276,21 @@ pub struct StandardStream { /// The lifetime `'a` refers to the lifetime of the corresponding /// `StandardStream`. pub struct StandardStreamLock<'a> { - wtr: LossyStandardStream>>, + wtr: LossyStandardStream>>, } /// WriterInner is a (limited) generic representation of a writer. It is /// limited because W should only ever be stdout/stderr on Windows. -enum WriterInner<'a, W> { +enum WriterInner { + NoColor(NoColor), + Ansi(Ansi), + #[cfg(windows)] + Windows { wtr: W, console: Mutex }, +} + +/// WriterInnerLock is a (limited) generic representation of a writer. It is +/// limited because W should only ever be stdout/stderr on Windows. +enum WriterInnerLock<'a, W> { NoColor(NoColor), Ansi(Ansi), /// What a gross hack. On Windows, we need to specify a lifetime for the @@ -291,9 +300,7 @@ enum WriterInner<'a, W> { #[allow(dead_code)] Unreachable(::std::marker::PhantomData<&'a ()>), #[cfg(windows)] - Windows { wtr: W, console: Mutex }, - #[cfg(windows)] - WindowsLocked { wtr: W, console: MutexGuard<'a, wincolor::Console> }, + Windows { wtr: W, console: MutexGuard<'a, wincolor::Console> }, } impl StandardStream { @@ -386,12 +393,11 @@ impl<'a> StandardStreamLock<'a> { #[cfg(not(windows))] fn from_stream(stream: &StandardStream) -> StandardStreamLock { let locked = match *stream.wtr.get_ref() { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref w) => { - WriterInner::NoColor(NoColor(w.0.lock())) + WriterInnerLock::NoColor(NoColor(w.0.lock())) } WriterInner::Ansi(ref w) => { - WriterInner::Ansi(Ansi(w.0.lock())) + WriterInnerLock::Ansi(Ansi(w.0.lock())) } }; StandardStreamLock { wtr: stream.wtr.wrap(locked) } @@ -400,25 +406,19 @@ impl<'a> StandardStreamLock<'a> { #[cfg(windows)] fn from_stream(stream: &StandardStream) -> StandardStreamLock { let locked = match *stream.wtr.get_ref() { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref w) => { - WriterInner::NoColor(NoColor(w.0.lock())) + WriterInnerLock::NoColor(NoColor(w.0.lock())) } WriterInner::Ansi(ref w) => { - WriterInner::Ansi(Ansi(w.0.lock())) + WriterInnerLock::Ansi(Ansi(w.0.lock())) } #[cfg(windows)] WriterInner::Windows { ref wtr, ref console } => { - WriterInner::WindowsLocked { + WriterInnerLock::Windows { wtr: wtr.lock(), console: console.lock().unwrap(), } } - #[cfg(windows)] - WriterInner::WindowsLocked{..} => { - panic!("cannot call StandardStream.lock while a \ - StandardStreamLock is alive"); - } }; StandardStreamLock { wtr: stream.wtr.wrap(locked) } } @@ -450,48 +450,38 @@ impl<'a> WriteColor for StandardStreamLock<'a> { fn reset(&mut self) -> io::Result<()> { self.wtr.reset() } } -impl<'a, W: io::Write> io::Write for WriterInner<'a, W> { +impl io::Write for WriterInner { fn write(&mut self, buf: &[u8]) -> io::Result { match *self { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref mut wtr) => wtr.write(buf), WriterInner::Ansi(ref mut wtr) => wtr.write(buf), #[cfg(windows)] WriterInner::Windows { ref mut wtr, .. } => wtr.write(buf), - #[cfg(windows)] - WriterInner::WindowsLocked { ref mut wtr, .. } => wtr.write(buf), } } fn flush(&mut self) -> io::Result<()> { match *self { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref mut wtr) => wtr.flush(), WriterInner::Ansi(ref mut wtr) => wtr.flush(), #[cfg(windows)] WriterInner::Windows { ref mut wtr, .. } => wtr.flush(), - #[cfg(windows)] - WriterInner::WindowsLocked { ref mut wtr, .. } => wtr.flush(), } } } -impl<'a, W: io::Write> WriteColor for WriterInner<'a, W> { +impl WriteColor for WriterInner { fn supports_color(&self) -> bool { match *self { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(_) => false, WriterInner::Ansi(_) => true, #[cfg(windows)] WriterInner::Windows { .. } => true, - #[cfg(windows)] - WriterInner::WindowsLocked { .. } => true, } } fn set_color(&mut self, spec: &ColorSpec) -> io::Result<()> { match *self { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref mut wtr) => wtr.set_color(spec), WriterInner::Ansi(ref mut wtr) => wtr.set_color(spec), #[cfg(windows)] @@ -500,17 +490,11 @@ impl<'a, W: io::Write> WriteColor for WriterInner<'a, W> { let mut console = console.lock().unwrap(); spec.write_console(&mut *console) } - #[cfg(windows)] - WriterInner::WindowsLocked { ref mut wtr, ref mut console } => { - try!(wtr.flush()); - spec.write_console(console) - } } } fn reset(&mut self) -> io::Result<()> { match *self { - WriterInner::Unreachable(_) => unreachable!(), WriterInner::NoColor(ref mut wtr) => wtr.reset(), WriterInner::Ansi(ref mut wtr) => wtr.reset(), #[cfg(windows)] @@ -519,8 +503,63 @@ impl<'a, W: io::Write> WriteColor for WriterInner<'a, W> { try!(console.lock().unwrap().reset()); Ok(()) } + } + } +} + +impl<'a, W: io::Write> io::Write for WriterInnerLock<'a, W> { + fn write(&mut self, buf: &[u8]) -> io::Result { + match *self { + WriterInnerLock::Unreachable(_) => unreachable!(), + WriterInnerLock::NoColor(ref mut wtr) => wtr.write(buf), + WriterInnerLock::Ansi(ref mut wtr) => wtr.write(buf), #[cfg(windows)] - WriterInner::WindowsLocked { ref mut wtr, ref mut console } => { + WriterInnerLock::Windows { ref mut wtr, .. } => wtr.write(buf), + } + } + + fn flush(&mut self) -> io::Result<()> { + match *self { + WriterInnerLock::Unreachable(_) => unreachable!(), + WriterInnerLock::NoColor(ref mut wtr) => wtr.flush(), + WriterInnerLock::Ansi(ref mut wtr) => wtr.flush(), + #[cfg(windows)] + WriterInnerLock::Windows { ref mut wtr, .. } => wtr.flush(), + } + } +} + +impl<'a, W: io::Write> WriteColor for WriterInnerLock<'a, W> { + fn supports_color(&self) -> bool { + match *self { + WriterInnerLock::Unreachable(_) => unreachable!(), + WriterInnerLock::NoColor(_) => false, + WriterInnerLock::Ansi(_) => true, + #[cfg(windows)] + WriterInnerLock::Windows { .. } => true, + } + } + + fn set_color(&mut self, spec: &ColorSpec) -> io::Result<()> { + match *self { + WriterInnerLock::Unreachable(_) => unreachable!(), + WriterInnerLock::NoColor(ref mut wtr) => wtr.set_color(spec), + WriterInnerLock::Ansi(ref mut wtr) => wtr.set_color(spec), + #[cfg(windows)] + WriterInnerLock::Windows { ref mut wtr, ref mut console } => { + try!(wtr.flush()); + spec.write_console(console) + } + } + } + + fn reset(&mut self) -> io::Result<()> { + match *self { + WriterInnerLock::Unreachable(_) => unreachable!(), + WriterInnerLock::NoColor(ref mut wtr) => wtr.reset(), + WriterInnerLock::Ansi(ref mut wtr) => wtr.reset(), + #[cfg(windows)] + WriterInnerLock::Windows { ref mut wtr, ref mut console } => { try!(wtr.flush()); try!(console.reset()); Ok(()) @@ -1335,3 +1374,15 @@ fn write_lossy_utf8(mut w: W, buf: &[u8]) -> io::Result { Err(e) => w.write(&buf[..e.valid_up_to()]), } } + +#[cfg(test)] +mod tests { + use super::StandardStream; + + fn assert_is_send() {} + + #[test] + fn standard_stream_is_send() { + assert_is_send::(); + } +} -- cgit v1.2.3