1
0
mirror of https://github.com/BurntSushi/ripgrep.git synced 2025-05-13 21:26:27 +02:00

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 
This commit is contained in:
Andrew Gallant 2017-08-27 10:14:25 -04:00
parent 5213bd30ea
commit d97c80be63

@ -265,7 +265,7 @@ impl<'a> io::Write for IoStandardStreamLock<'a> {
/// Satisfies `io::Write` and `WriteColor`, and supports optional coloring /// Satisfies `io::Write` and `WriteColor`, and supports optional coloring
/// to either of the standard output streams, stdout and stderr. /// to either of the standard output streams, stdout and stderr.
pub struct StandardStream { pub struct StandardStream {
wtr: LossyStandardStream<WriterInner<'static, IoStandardStream>>, wtr: LossyStandardStream<WriterInner<IoStandardStream>>,
} }
/// `StandardStreamLock` is a locked reference to a `StandardStream`. /// `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 /// The lifetime `'a` refers to the lifetime of the corresponding
/// `StandardStream`. /// `StandardStream`.
pub struct StandardStreamLock<'a> { pub struct StandardStreamLock<'a> {
wtr: LossyStandardStream<WriterInner<'a, IoStandardStreamLock<'a>>>, wtr: LossyStandardStream<WriterInnerLock<'a, IoStandardStreamLock<'a>>>,
} }
/// WriterInner is a (limited) generic representation of a writer. It is /// WriterInner is a (limited) generic representation of a writer. It is
/// limited because W should only ever be stdout/stderr on Windows. /// limited because W should only ever be stdout/stderr on Windows.
enum WriterInner<'a, W> { enum WriterInner<W> {
NoColor(NoColor<W>),
Ansi(Ansi<W>),
#[cfg(windows)]
Windows { wtr: W, console: Mutex<wincolor::Console> },
}
/// 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<W>), NoColor(NoColor<W>),
Ansi(Ansi<W>), Ansi(Ansi<W>),
/// What a gross hack. On Windows, we need to specify a lifetime for the /// 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)] #[allow(dead_code)]
Unreachable(::std::marker::PhantomData<&'a ()>), Unreachable(::std::marker::PhantomData<&'a ()>),
#[cfg(windows)] #[cfg(windows)]
Windows { wtr: W, console: Mutex<wincolor::Console> }, Windows { wtr: W, console: MutexGuard<'a, wincolor::Console> },
#[cfg(windows)]
WindowsLocked { wtr: W, console: MutexGuard<'a, wincolor::Console> },
} }
impl StandardStream { impl StandardStream {
@ -386,12 +393,11 @@ impl<'a> StandardStreamLock<'a> {
#[cfg(not(windows))] #[cfg(not(windows))]
fn from_stream(stream: &StandardStream) -> StandardStreamLock { fn from_stream(stream: &StandardStream) -> StandardStreamLock {
let locked = match *stream.wtr.get_ref() { let locked = match *stream.wtr.get_ref() {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref w) => { WriterInner::NoColor(ref w) => {
WriterInner::NoColor(NoColor(w.0.lock())) WriterInnerLock::NoColor(NoColor(w.0.lock()))
} }
WriterInner::Ansi(ref w) => { WriterInner::Ansi(ref w) => {
WriterInner::Ansi(Ansi(w.0.lock())) WriterInnerLock::Ansi(Ansi(w.0.lock()))
} }
}; };
StandardStreamLock { wtr: stream.wtr.wrap(locked) } StandardStreamLock { wtr: stream.wtr.wrap(locked) }
@ -400,25 +406,19 @@ impl<'a> StandardStreamLock<'a> {
#[cfg(windows)] #[cfg(windows)]
fn from_stream(stream: &StandardStream) -> StandardStreamLock { fn from_stream(stream: &StandardStream) -> StandardStreamLock {
let locked = match *stream.wtr.get_ref() { let locked = match *stream.wtr.get_ref() {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref w) => { WriterInner::NoColor(ref w) => {
WriterInner::NoColor(NoColor(w.0.lock())) WriterInnerLock::NoColor(NoColor(w.0.lock()))
} }
WriterInner::Ansi(ref w) => { WriterInner::Ansi(ref w) => {
WriterInner::Ansi(Ansi(w.0.lock())) WriterInnerLock::Ansi(Ansi(w.0.lock()))
} }
#[cfg(windows)] #[cfg(windows)]
WriterInner::Windows { ref wtr, ref console } => { WriterInner::Windows { ref wtr, ref console } => {
WriterInner::WindowsLocked { WriterInnerLock::Windows {
wtr: wtr.lock(), wtr: wtr.lock(),
console: console.lock().unwrap(), console: console.lock().unwrap(),
} }
} }
#[cfg(windows)]
WriterInner::WindowsLocked{..} => {
panic!("cannot call StandardStream.lock while a \
StandardStreamLock is alive");
}
}; };
StandardStreamLock { wtr: stream.wtr.wrap(locked) } 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() } fn reset(&mut self) -> io::Result<()> { self.wtr.reset() }
} }
impl<'a, W: io::Write> io::Write for WriterInner<'a, W> { impl<W: io::Write> io::Write for WriterInner<W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> { fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match *self { match *self {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref mut wtr) => wtr.write(buf), WriterInner::NoColor(ref mut wtr) => wtr.write(buf),
WriterInner::Ansi(ref mut wtr) => wtr.write(buf), WriterInner::Ansi(ref mut wtr) => wtr.write(buf),
#[cfg(windows)] #[cfg(windows)]
WriterInner::Windows { ref mut wtr, .. } => wtr.write(buf), WriterInner::Windows { ref mut wtr, .. } => wtr.write(buf),
#[cfg(windows)]
WriterInner::WindowsLocked { ref mut wtr, .. } => wtr.write(buf),
} }
} }
fn flush(&mut self) -> io::Result<()> { fn flush(&mut self) -> io::Result<()> {
match *self { match *self {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref mut wtr) => wtr.flush(), WriterInner::NoColor(ref mut wtr) => wtr.flush(),
WriterInner::Ansi(ref mut wtr) => wtr.flush(), WriterInner::Ansi(ref mut wtr) => wtr.flush(),
#[cfg(windows)] #[cfg(windows)]
WriterInner::Windows { ref mut wtr, .. } => wtr.flush(), 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<W: io::Write> WriteColor for WriterInner<W> {
fn supports_color(&self) -> bool { fn supports_color(&self) -> bool {
match *self { match *self {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(_) => false, WriterInner::NoColor(_) => false,
WriterInner::Ansi(_) => true, WriterInner::Ansi(_) => true,
#[cfg(windows)] #[cfg(windows)]
WriterInner::Windows { .. } => true, WriterInner::Windows { .. } => true,
#[cfg(windows)]
WriterInner::WindowsLocked { .. } => true,
} }
} }
fn set_color(&mut self, spec: &ColorSpec) -> io::Result<()> { fn set_color(&mut self, spec: &ColorSpec) -> io::Result<()> {
match *self { match *self {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref mut wtr) => wtr.set_color(spec), WriterInner::NoColor(ref mut wtr) => wtr.set_color(spec),
WriterInner::Ansi(ref mut wtr) => wtr.set_color(spec), WriterInner::Ansi(ref mut wtr) => wtr.set_color(spec),
#[cfg(windows)] #[cfg(windows)]
@ -500,17 +490,11 @@ impl<'a, W: io::Write> WriteColor for WriterInner<'a, W> {
let mut console = console.lock().unwrap(); let mut console = console.lock().unwrap();
spec.write_console(&mut *console) 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<()> { fn reset(&mut self) -> io::Result<()> {
match *self { match *self {
WriterInner::Unreachable(_) => unreachable!(),
WriterInner::NoColor(ref mut wtr) => wtr.reset(), WriterInner::NoColor(ref mut wtr) => wtr.reset(),
WriterInner::Ansi(ref mut wtr) => wtr.reset(), WriterInner::Ansi(ref mut wtr) => wtr.reset(),
#[cfg(windows)] #[cfg(windows)]
@ -519,8 +503,63 @@ impl<'a, W: io::Write> WriteColor for WriterInner<'a, W> {
try!(console.lock().unwrap().reset()); try!(console.lock().unwrap().reset());
Ok(()) Ok(())
} }
}
}
}
impl<'a, W: io::Write> io::Write for WriterInnerLock<'a, W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match *self {
WriterInnerLock::Unreachable(_) => unreachable!(),
WriterInnerLock::NoColor(ref mut wtr) => wtr.write(buf),
WriterInnerLock::Ansi(ref mut wtr) => wtr.write(buf),
#[cfg(windows)] #[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!(wtr.flush());
try!(console.reset()); try!(console.reset());
Ok(()) Ok(())
@ -1335,3 +1374,15 @@ fn write_lossy_utf8<W: io::Write>(mut w: W, buf: &[u8]) -> io::Result<usize> {
Err(e) => w.write(&buf[..e.valid_up_to()]), Err(e) => w.write(&buf[..e.valid_up_to()]),
} }
} }
#[cfg(test)]
mod tests {
use super::StandardStream;
fn assert_is_send<T: Send>() {}
#[test]
fn standard_stream_is_send() {
assert_is_send::<StandardStream>();
}
}