From a919efad2e410eafe72d067b8a8d4cbe02ecb0df Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 27 Feb 2018 10:33:02 -0800 Subject: [PATCH] rustc: Migrate to `termcolor` crate from `term` This crate moves the compiler's error reporting to using the `termcolor` crate from crates.io. Previously rustc used a super-old version of the `term` crate in-tree which is basically unmaintained at this point, but Cargo has been using `termcolor` for some time now and tools like `rg` are using `termcolor` as well, so it seems like a good strategy to take! Note that the `term` crate remains in-tree for libtest. Changing libtest will be a bit tricky due to how the build works, but we can always tackle that later. cc #45728 --- src/Cargo.lock | 12 +- src/librustc_errors/Cargo.toml | 2 + src/librustc_errors/emitter.rs | 257 ++++++++++++++------------------- src/librustc_errors/lib.rs | 31 ++-- 4 files changed, 136 insertions(+), 166 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 1bf31b2a67c45..76bf201a94011 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -78,7 +78,7 @@ dependencies = [ [[package]] name = "atty" -version = "0.2.6" +version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "libc 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", @@ -179,7 +179,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "cargo" version = "0.26.0" dependencies = [ - "atty 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "bufstream 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "cargotest 0.1.0", "core-foundation 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -289,7 +289,7 @@ version = "2.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", - "atty 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "strsim 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "textwrap 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -643,7 +643,7 @@ name = "env_logger" version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "atty 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", + "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "humantime 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1937,9 +1937,11 @@ dependencies = [ name = "rustc_errors" version = "0.0.0" dependencies = [ + "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_data_structures 0.0.0", "serialize 0.0.0", "syntax_pos 0.0.0", + "termcolor 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-width 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2876,7 +2878,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6b3568b48b7cefa6b8ce125f9bb4989e52fbcc29ebea88df04cc7c5f12f70455" "checksum ar 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "35c7a5669cb64f085739387e1308b74e6d44022464b7f1b63bbd4ceb6379ec31" "checksum arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "a1e964f9e24d588183fcb43503abda40d288c8657dfc27311516ce2f05675aef" -"checksum atty 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "8352656fd42c30a0c3c89d26dea01e3b77c0ab2af18230835c15e2e13cd51859" +"checksum atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "af80143d6f7608d746df1520709e5d141c96f240b0e62b0aa41bdfb53374d9d4" "checksum backtrace 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "ebbbf59b1c43eefa8c3ede390fcc36820b4999f7914104015be25025e0d62af2" "checksum backtrace-sys 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "44585761d6161b0f57afc49482ab6bd067e4edef48c12a152c237eb0203f7661" "checksum bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "aad18937a628ec6abcd26d1489012cc0e18c21798210f491af69ded9b881106d" diff --git a/src/librustc_errors/Cargo.toml b/src/librustc_errors/Cargo.toml index 3e15af7558da2..e412d1749d1b2 100644 --- a/src/librustc_errors/Cargo.toml +++ b/src/librustc_errors/Cargo.toml @@ -13,3 +13,5 @@ serialize = { path = "../libserialize" } syntax_pos = { path = "../libsyntax_pos" } rustc_data_structures = { path = "../librustc_data_structures" } unicode-width = "0.1.4" +atty = "0.2" +termcolor = "0.3" diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 33fce7b1968ad..f481b36daa346 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -17,12 +17,14 @@ use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledStrin use styled_buffer::StyledBuffer; use rustc_data_structures::sync::Lrc; +use atty; use std::borrow::Cow; use std::io::prelude::*; use std::io; -use term; use std::collections::{HashMap, HashSet}; use std::cmp::min; +use termcolor::{StandardStream, ColorChoice, ColorSpec, BufferWriter}; +use termcolor::{WriteColor, Color, Buffer}; use unicode_width; const ANONYMIZED_LINE_NUM: &str = "LL"; @@ -95,11 +97,14 @@ pub enum ColorConfig { } impl ColorConfig { - fn use_color(&self) -> bool { + fn to_color_choice(&self) -> ColorChoice { match *self { - ColorConfig::Always => true, - ColorConfig::Never => false, - ColorConfig::Auto => stderr_isatty(), + ColorConfig::Always => ColorChoice::Always, + ColorConfig::Never => ColorChoice::Never, + ColorConfig::Auto if atty::is(atty::Stream::Stderr) => { + ColorChoice::Auto + } + ColorConfig::Auto => ColorChoice::Never, } } } @@ -123,25 +128,26 @@ impl Drop for EmitterWriter { fn drop(&mut self) { if !self.short_message && !self.error_codes.is_empty() { let mut error_codes = self.error_codes.clone().into_iter().collect::>(); + let mut dst = self.dst.writable(); error_codes.sort(); if error_codes.len() > 1 { let limit = if error_codes.len() > 9 { 9 } else { error_codes.len() }; - writeln!(self.dst, + writeln!(dst, "You've got a few errors: {}{}", error_codes[..limit].join(", "), if error_codes.len() > 9 { "..." } else { "" } ).expect("failed to give tips..."); - writeln!(self.dst, + writeln!(dst, "If you want more information on an error, try using \ \"rustc --explain {}\"", &error_codes[0]).expect("failed to give tips..."); } else { - writeln!(self.dst, + writeln!(dst, "If you want more information on this error, try using \ \"rustc --explain {}\"", &error_codes[0]).expect("failed to give tips..."); } - self.dst.flush().expect("failed to emit errors"); + dst.flush().expect("failed to emit errors"); } } } @@ -152,25 +158,14 @@ impl EmitterWriter { short_message: bool, teach: bool) -> EmitterWriter { - if color_config.use_color() { - let dst = Destination::from_stderr(); - EmitterWriter { - dst, - cm: code_map, - short_message, - teach, - error_codes: HashSet::new(), - ui_testing: false, - } - } else { - EmitterWriter { - dst: Raw(Box::new(io::stderr())), - cm: code_map, - short_message, - teach, - error_codes: HashSet::new(), - ui_testing: false, - } + let dst = Destination::from_stderr(color_config); + EmitterWriter { + dst, + cm: code_map, + short_message, + teach, + error_codes: HashSet::new(), + ui_testing: false, } } @@ -1356,10 +1351,12 @@ impl EmitterWriter { } Err(e) => panic!("failed to emit error: {}", e), } - match write!(&mut self.dst, "\n") { + + let mut dst = self.dst.writable(); + match write!(dst, "\n") { Err(e) => panic!("failed to emit error: {}", e), _ => { - match self.dst.flush() { + match dst.flush() { Err(e) => panic!("failed to emit error: {}", e), _ => (), } @@ -1424,6 +1421,8 @@ fn emit_to_destination(rendered_buffer: &Vec>, -> io::Result<()> { use lock; + let mut dst = dst.writable(); + // In order to prevent error message interleaving, where multiple error lines get intermixed // when multiple compiler processes error simultaneously, we emit errors with additional // steps. @@ -1444,7 +1443,7 @@ fn emit_to_destination(rendered_buffer: &Vec>, if !short_message && part.text.len() == 12 && part.text.starts_with("error[E") { error_codes.insert(part.text[6..11].to_owned()); } - dst.reset_attrs()?; + dst.reset()?; } if !short_message { write!(dst, "\n")?; @@ -1454,180 +1453,136 @@ fn emit_to_destination(rendered_buffer: &Vec>, Ok(()) } -#[cfg(unix)] -fn stderr_isatty() -> bool { - use libc; - unsafe { libc::isatty(libc::STDERR_FILENO) != 0 } -} -#[cfg(windows)] -fn stderr_isatty() -> bool { - type DWORD = u32; - type BOOL = i32; - type HANDLE = *mut u8; - const STD_ERROR_HANDLE: DWORD = -12i32 as DWORD; - extern "system" { - fn GetStdHandle(which: DWORD) -> HANDLE; - fn GetConsoleMode(hConsoleHandle: HANDLE, lpMode: *mut DWORD) -> BOOL; - } - unsafe { - let handle = GetStdHandle(STD_ERROR_HANDLE); - let mut out = 0; - GetConsoleMode(handle, &mut out) != 0 - } -} - -pub type BufferedStderr = term::Terminal + Send; - pub enum Destination { - Terminal(Box), - BufferedTerminal(Box), + Terminal(StandardStream), + Buffered(BufferWriter), Raw(Box), } -/// Buffered writer gives us a way on Unix to buffer up an entire error message before we output -/// it. This helps to prevent interleaving of multiple error messages when multiple compiler -/// processes error simultaneously -pub struct BufferedWriter { - buffer: Vec, -} - -impl BufferedWriter { - // note: we use _new because the conditional compilation at its use site may make this - // this function unused on some platforms - fn _new() -> BufferedWriter { - BufferedWriter { buffer: vec![] } - } -} - -impl Write for BufferedWriter { - fn write(&mut self, buf: &[u8]) -> io::Result { - for b in buf { - self.buffer.push(*b); - } - Ok(buf.len()) - } - fn flush(&mut self) -> io::Result<()> { - let mut stderr = io::stderr(); - let result = stderr.write_all(&self.buffer) - .and_then(|_| stderr.flush()); - self.buffer.clear(); - result - } +pub enum WritableDst<'a> { + Terminal(&'a mut StandardStream), + Buffered(&'a mut BufferWriter, Buffer), + Raw(&'a mut Box), } impl Destination { - #[cfg(not(windows))] - /// When not on Windows, prefer the buffered terminal so that we can buffer an entire error - /// to be emitted at one time. - fn from_stderr() -> Destination { - let stderr: Option> = - term::TerminfoTerminal::new(BufferedWriter::_new()) - .map(|t| Box::new(t) as Box); - - match stderr { - Some(t) => BufferedTerminal(t), - None => Raw(Box::new(io::stderr())), + fn from_stderr(color: ColorConfig) -> Destination { + let choice = color.to_color_choice(); + // On Windows we'll be performing global synchronization on the entire + // system for emitting rustc errors, so there's no need to buffer + // anything. + // + // On non-Windows we rely on the atomicity of `write` to ensure errors + // don't get all jumbled up. + if cfg!(windows) { + Terminal(StandardStream::stderr(choice)) + } else { + Buffered(BufferWriter::stderr(choice)) } } - #[cfg(windows)] - /// Return a normal, unbuffered terminal when on Windows. - fn from_stderr() -> Destination { - let stderr: Option> = term::TerminfoTerminal::new(io::stderr()) - .map(|t| Box::new(t) as Box) - .or_else(|| { - term::WinConsole::new(io::stderr()) - .ok() - .map(|t| Box::new(t) as Box) - }); - - match stderr { - Some(t) => Terminal(t), - None => Raw(Box::new(io::stderr())), + fn writable<'a>(&'a mut self) -> WritableDst<'a> { + match *self { + Destination::Terminal(ref mut t) => WritableDst::Terminal(t), + Destination::Buffered(ref mut t) => { + let buf = t.buffer(); + WritableDst::Buffered(t, buf) + } + Destination::Raw(ref mut t) => WritableDst::Raw(t), } } +} +impl<'a> WritableDst<'a> { fn apply_style(&mut self, lvl: Level, style: Style) -> io::Result<()> { + let mut spec = ColorSpec::new(); match style { Style::LineAndColumn => {} Style::LineNumber => { - self.start_attr(term::Attr::Bold)?; + spec.set_bold(true); + spec.set_intense(true); if cfg!(windows) { - self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_CYAN))?; + spec.set_fg(Some(Color::Cyan)); } else { - self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_BLUE))?; + spec.set_fg(Some(Color::Blue)); } } Style::Quotation => {} Style::OldSchoolNoteText | Style::HeaderMsg => { - self.start_attr(term::Attr::Bold)?; + spec.set_bold(true); if cfg!(windows) { - self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_WHITE))?; + spec.set_intense(true) + .set_fg(Some(Color::White)); } } Style::UnderlinePrimary | Style::LabelPrimary => { - self.start_attr(term::Attr::Bold)?; - self.start_attr(term::Attr::ForegroundColor(lvl.color()))?; + spec = lvl.color(); + spec.set_bold(true); } Style::UnderlineSecondary | Style::LabelSecondary => { - self.start_attr(term::Attr::Bold)?; + spec.set_bold(true) + .set_intense(true); if cfg!(windows) { - self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_CYAN))?; + spec.set_fg(Some(Color::Cyan)); } else { - self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_BLUE))?; + spec.set_fg(Some(Color::Blue)); } } Style::NoStyle => {} - Style::Level(l) => { - self.start_attr(term::Attr::Bold)?; - self.start_attr(term::Attr::ForegroundColor(l.color()))?; + Style::Level(lvl) => { + spec = lvl.color(); + spec.set_bold(true); + } + Style::Highlight => { + spec.set_bold(true); } - Style::Highlight => self.start_attr(term::Attr::Bold)?, } - Ok(()) + self.set_color(&spec) } - fn start_attr(&mut self, attr: term::Attr) -> io::Result<()> { + fn set_color(&mut self, color: &ColorSpec) -> io::Result<()> { match *self { - Terminal(ref mut t) => { - t.attr(attr)?; - } - BufferedTerminal(ref mut t) => { - t.attr(attr)?; - } - Raw(_) => {} + WritableDst::Terminal(ref mut t) => t.set_color(color), + WritableDst::Buffered(_, ref mut t) => t.set_color(color), + WritableDst::Raw(_) => Ok(()) } - Ok(()) } - fn reset_attrs(&mut self) -> io::Result<()> { + fn reset(&mut self) -> io::Result<()> { match *self { - Terminal(ref mut t) => { - t.reset()?; - } - BufferedTerminal(ref mut t) => { - t.reset()?; - } - Raw(_) => {} + WritableDst::Terminal(ref mut t) => t.reset(), + WritableDst::Buffered(_, ref mut t) => t.reset(), + WritableDst::Raw(_) => Ok(()), } - Ok(()) } } -impl Write for Destination { +impl<'a> Write for WritableDst<'a> { fn write(&mut self, bytes: &[u8]) -> io::Result { match *self { - Terminal(ref mut t) => t.write(bytes), - BufferedTerminal(ref mut t) => t.write(bytes), - Raw(ref mut w) => w.write(bytes), + WritableDst::Terminal(ref mut t) => t.write(bytes), + WritableDst::Buffered(_, ref mut buf) => buf.write(bytes), + WritableDst::Raw(ref mut w) => w.write(bytes), } } + fn flush(&mut self) -> io::Result<()> { match *self { - Terminal(ref mut t) => t.flush(), - BufferedTerminal(ref mut t) => t.flush(), - Raw(ref mut w) => w.flush(), + WritableDst::Terminal(ref mut t) => t.flush(), + WritableDst::Buffered(_, ref mut buf) => buf.flush(), + WritableDst::Raw(ref mut w) => w.flush(), + } + } +} + +impl<'a> Drop for WritableDst<'a> { + fn drop(&mut self) { + match *self { + WritableDst::Buffered(ref mut dst, ref mut buf) => { + drop(dst.print(buf)); + } + _ => {} } } } diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 3eea311a5af5e..924ed71ef0d65 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -21,7 +21,8 @@ #![feature(i128_type)] #![feature(optin_builtin_traits)] -extern crate term; +extern crate atty; +extern crate termcolor; #[cfg(unix)] extern crate libc; extern crate rustc_data_structures; @@ -47,6 +48,8 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering::SeqCst; use std::panic; +use termcolor::{ColorSpec, Color}; + mod diagnostic; mod diagnostic_builder; pub mod emitter; @@ -660,20 +663,28 @@ impl fmt::Display for Level { } impl Level { - fn color(self) -> term::color::Color { + fn color(self) -> ColorSpec { + let mut spec = ColorSpec::new(); match self { - Bug | Fatal | PhaseFatal | Error => term::color::BRIGHT_RED, + Bug | Fatal | PhaseFatal | Error => { + spec.set_fg(Some(Color::Red)) + .set_intense(true); + } Warning => { - if cfg!(windows) { - term::color::BRIGHT_YELLOW - } else { - term::color::YELLOW - } + spec.set_fg(Some(Color::Yellow)) + .set_intense(cfg!(windows)); + } + Note => { + spec.set_fg(Some(Color::Green)) + .set_intense(true); + } + Help => { + spec.set_fg(Some(Color::Cyan)) + .set_intense(true); } - Note => term::color::BRIGHT_GREEN, - Help => term::color::BRIGHT_CYAN, Cancelled => unreachable!(), } + return spec } pub fn to_str(self) -> &'static str {