From 72e96604c0115ee77b0817d8bb053bcfd4625b70 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 Nov 2020 20:48:25 +0100 Subject: [PATCH] Remove io::LocalOutput and use Arc> for local streams. --- compiler/rustc_interface/src/util.rs | 21 ++-------- library/std/src/io/impls.rs | 14 ------- library/std/src/io/mod.rs | 2 +- library/std/src/io/stdio.rs | 57 ++++++++++------------------ library/std/src/panicking.rs | 27 +++++++++++-- library/test/src/bench.rs | 8 +--- library/test/src/helpers/mod.rs | 1 - library/test/src/helpers/sink.rs | 31 --------------- library/test/src/lib.rs | 8 +--- 9 files changed, 51 insertions(+), 118 deletions(-) delete mode 100644 library/test/src/helpers/sink.rs diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index d9ec6d51cdf..d90fff3bae5 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -25,7 +25,7 @@ use smallvec::SmallVec; use std::env; use std::env::consts::{DLL_PREFIX, DLL_SUFFIX}; -use std::io::{self, Write}; +use std::io; use std::lazy::SyncOnceCell; use std::mem; use std::ops::DerefMut; @@ -106,21 +106,6 @@ fn get_stack_size() -> Option { env::var_os("RUST_MIN_STACK").is_none().then_some(STACK_SIZE) } -struct Sink(Arc>>); -impl Write for Sink { - fn write(&mut self, data: &[u8]) -> io::Result { - Write::write(&mut *self.0.lock().unwrap(), data) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} -impl io::LocalOutput for Sink { - fn clone_box(&self) -> Box { - Box::new(Self(self.0.clone())) - } -} - /// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need /// for `'static` bounds. #[cfg(not(parallel_compiler))] @@ -164,7 +149,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se let main_handler = move || { rustc_span::with_session_globals(edition, || { if let Some(stderr) = stderr { - io::set_panic(Some(box Sink(stderr.clone()))); + io::set_panic(Some(stderr.clone())); } f() }) @@ -204,7 +189,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se let main_handler = move |thread: rayon::ThreadBuilder| { rustc_span::SESSION_GLOBALS.set(session_globals, || { if let Some(stderr) = stderr { - io::set_panic(Some(box Sink(stderr.clone()))); + io::set_panic(Some(stderr.clone())); } thread.run() }) diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index 66426101d27..6b3c86cb0df 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -209,20 +209,6 @@ fn read_line(&mut self, buf: &mut String) -> io::Result { } } -// Used by panicking::default_hook -#[cfg(test)] -/// This impl is only used by printing logic, so any error returned is always -/// of kind `Other`, and should be ignored. -impl Write for dyn ::realstd::io::LocalOutput { - fn write(&mut self, buf: &[u8]) -> io::Result { - (*self).write(buf).map_err(|_| ErrorKind::Other.into()) - } - - fn flush(&mut self) -> io::Result<()> { - (*self).flush().map_err(|_| ErrorKind::Other.into()) - } -} - // ============================================================================= // In-memory buffer implementations diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e6efe6ec57e..e6b9314fd88 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -277,7 +277,7 @@ pub use self::stdio::{_eprint, _print}; #[unstable(feature = "libstd_io_internals", issue = "42788")] #[doc(no_inline, hidden)] -pub use self::stdio::{set_panic, set_print, LocalOutput}; +pub use self::stdio::{set_panic, set_print}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::util::{copy, empty, repeat, sink, Empty, Repeat, Sink}; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 2eb5fb45286..e3e8a763591 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -10,22 +10,24 @@ use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter}; use crate::lazy::SyncOnceCell; use crate::sync::atomic::{AtomicBool, Ordering}; -use crate::sync::{Mutex, MutexGuard}; +use crate::sync::{Arc, Mutex, MutexGuard}; use crate::sys::stdio; use crate::sys_common; use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use crate::thread::LocalKey; +type LocalStream = Arc>; + thread_local! { /// Used by the test crate to capture the output of the print! and println! macros. - static LOCAL_STDOUT: RefCell>> = { + static LOCAL_STDOUT: RefCell> = { RefCell::new(None) } } thread_local! { /// Used by the test crate to capture the output of the eprint! and eprintln! macros, and panics. - static LOCAL_STDERR: RefCell>> = { + static LOCAL_STDERR: RefCell> = { RefCell::new(None) } } @@ -888,18 +890,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { } } -/// A writer than can be cloned to new threads. -#[unstable( - feature = "set_stdio", - reason = "this trait may disappear completely or be replaced \ - with a more general mechanism", - issue = "none" -)] -#[doc(hidden)] -pub trait LocalOutput: Write + Send { - fn clone_box(&self) -> Box; -} - /// Resets the thread-local stderr handle to the specified writer /// /// This will replace the current thread's stderr handle, returning the old @@ -915,18 +905,17 @@ pub trait LocalOutput: Write + Send { issue = "none" )] #[doc(hidden)] -pub fn set_panic(sink: Option>) -> Option> { +pub fn set_panic(sink: Option) -> Option { use crate::mem; if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { // LOCAL_STDERR is definitely None since LOCAL_STREAMS is false. return None; } - let s = LOCAL_STDERR.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then( - |mut s| { - let _ = s.flush(); + let s = + LOCAL_STDERR.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then(|s| { + let _ = s.lock().unwrap_or_else(|e| e.into_inner()).flush(); Some(s) - }, - ); + }); LOCAL_STREAMS.store(true, Ordering::Relaxed); s } @@ -946,35 +935,29 @@ pub fn set_panic(sink: Option>) -> Option>) -> Option> { +pub fn set_print(sink: Option) -> Option { use crate::mem; if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { // LOCAL_STDOUT is definitely None since LOCAL_STREAMS is false. return None; } - let s = LOCAL_STDOUT.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then( - |mut s| { - let _ = s.flush(); + let s = + LOCAL_STDOUT.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)).and_then(|s| { + let _ = s.lock().unwrap_or_else(|e| e.into_inner()).flush(); Some(s) - }, - ); + }); LOCAL_STREAMS.store(true, Ordering::Relaxed); s } -pub(crate) fn clone_io() -> (Option>, Option>) { +pub(crate) fn clone_io() -> (Option, Option) { // Don't waste time when LOCAL_{STDOUT,STDERR} are definitely None. if !LOCAL_STREAMS.load(Ordering::Relaxed) { return (None, None); } LOCAL_STDOUT.with(|stdout| { - LOCAL_STDERR.with(|stderr| { - ( - stdout.borrow().as_ref().map(|o| o.clone_box()), - stderr.borrow().as_ref().map(|o| o.clone_box()), - ) - }) + LOCAL_STDERR.with(|stderr| (stdout.borrow().clone(), stderr.borrow().clone())) }) } @@ -990,7 +973,7 @@ pub(crate) fn clone_io() -> (Option>, Option( args: fmt::Arguments<'_>, - local_s: &'static LocalKey>>>, + local_s: &'static LocalKey>>, global_s: fn() -> T, label: &str, ) where @@ -1005,8 +988,8 @@ fn print_to( // our printing recursively panics/prints, so the recursive // panic/print goes to the global sink instead of our local sink. let prev = s.borrow_mut().take(); - if let Some(mut w) = prev { - let result = w.write_fmt(args); + if let Some(w) = prev { + let result = w.lock().unwrap_or_else(|e| e.into_inner()).write_fmt(args); *s.borrow_mut() = Some(w); return result; } diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index fbbc61f4e60..9e9584baeb3 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -218,10 +218,29 @@ fn default_hook(info: &PanicInfo<'_>) { } }; - if let Some(mut local) = set_panic(None) { - // NB. In `cfg(test)` this uses the forwarding impl - // for `dyn ::realstd::io::LocalOutput`. - write(&mut local); + if let Some(local) = set_panic(None) { + let mut stream = local.lock().unwrap_or_else(|e| e.into_inner()); + + #[cfg(test)] + { + use crate::io; + struct Wrapper<'a>(&'a mut (dyn ::realstd::io::Write + Send)); + impl io::Write for Wrapper<'_> { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf).map_err(|_| io::ErrorKind::Other.into()) + } + fn flush(&mut self) -> io::Result<()> { + self.0.flush().map_err(|_| io::ErrorKind::Other.into()) + } + } + write(&mut Wrapper(&mut *stream)); + } + + #[cfg(not(test))] + write(&mut *stream); + + drop(stream); + set_panic(Some(local)); } else if let Some(mut out) = panic_output() { write(&mut out); diff --git a/library/test/src/bench.rs b/library/test/src/bench.rs index 10546de1764..396dd8d3436 100644 --- a/library/test/src/bench.rs +++ b/library/test/src/bench.rs @@ -2,8 +2,7 @@ pub use std::hint::black_box; use super::{ - event::CompletedTest, helpers::sink::Sink, options::BenchMode, test_result::TestResult, - types::TestDesc, Sender, + event::CompletedTest, options::BenchMode, test_result::TestResult, types::TestDesc, Sender, }; use crate::stats; @@ -186,10 +185,7 @@ pub fn benchmark(desc: TestDesc, monitor_ch: Sender, nocapture let data = Arc::new(Mutex::new(Vec::new())); let oldio = if !nocapture { - Some(( - io::set_print(Some(Sink::new_boxed(&data))), - io::set_panic(Some(Sink::new_boxed(&data))), - )) + Some((io::set_print(Some(data.clone())), io::set_panic(Some(data.clone())))) } else { None }; diff --git a/library/test/src/helpers/mod.rs b/library/test/src/helpers/mod.rs index eb416b10150..b7f00c4c86c 100644 --- a/library/test/src/helpers/mod.rs +++ b/library/test/src/helpers/mod.rs @@ -5,4 +5,3 @@ pub mod exit_code; pub mod isatty; pub mod metrics; -pub mod sink; diff --git a/library/test/src/helpers/sink.rs b/library/test/src/helpers/sink.rs deleted file mode 100644 index dfbf0a3b72f..00000000000 --- a/library/test/src/helpers/sink.rs +++ /dev/null @@ -1,31 +0,0 @@ -//! Module providing a helper structure to capture output in subprocesses. - -use std::{ - io, - io::prelude::Write, - sync::{Arc, Mutex}, -}; - -#[derive(Clone)] -pub struct Sink(Arc>>); - -impl Sink { - pub fn new_boxed(data: &Arc>>) -> Box { - Box::new(Self(data.clone())) - } -} - -impl io::LocalOutput for Sink { - fn clone_box(&self) -> Box { - Box::new(self.clone()) - } -} - -impl Write for Sink { - fn write(&mut self, data: &[u8]) -> io::Result { - Write::write(&mut *self.0.lock().unwrap(), data) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 9c5bb8957b5..7d0ce6dfbd1 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -89,7 +89,6 @@ pub mod test { use event::{CompletedTest, TestEvent}; use helpers::concurrency::get_concurrency; use helpers::exit_code::get_exit_code; -use helpers::sink::Sink; use options::{Concurrent, RunStrategy}; use test_result::*; use time::TestExecTime; @@ -532,10 +531,7 @@ fn run_test_in_process( let data = Arc::new(Mutex::new(Vec::new())); let oldio = if !nocapture { - Some(( - io::set_print(Some(Sink::new_boxed(&data))), - io::set_panic(Some(Sink::new_boxed(&data))), - )) + Some((io::set_print(Some(data.clone())), io::set_panic(Some(data.clone())))) } else { None }; @@ -556,7 +552,7 @@ fn run_test_in_process( Ok(()) => calc_result(&desc, Ok(()), &time_opts, &exec_time), Err(e) => calc_result(&desc, Err(e.as_ref()), &time_opts, &exec_time), }; - let stdout = data.lock().unwrap().to_vec(); + let stdout = data.lock().unwrap_or_else(|e| e.into_inner()).to_vec(); let message = CompletedTest::new(desc, test_result, exec_time, stdout); monitor_ch.send(message).unwrap(); } -- 2.44.0