From 72e96604c0115ee77b0817d8bb053bcfd4625b70 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 Nov 2020 20:48:25 +0100 Subject: [PATCH 1/5] 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 rustc_span::symbol::{sym, Symbol}; 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 @@ impl BufRead for Box { } } -// 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::{StderrLock, StdinLock, StdoutLock}; 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::fmt; 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 @@ impl fmt::Debug for StderrLock<'_> { } } -/// 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 @@ where 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 concurrency; 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 @@ mod tests; 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(); } From ccbce1d3b2b9f74619d19c6d3377d20dd06e0050 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 Nov 2020 20:49:02 +0100 Subject: [PATCH 2/5] Update tests for updated set_panic. --- src/test/ui/panic-while-printing.rs | 11 ++++----- src/test/ui/threads-sendsync/task-stderr.rs | 26 ++++++--------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/test/ui/panic-while-printing.rs b/src/test/ui/panic-while-printing.rs index 21fc12759f8..555b90fbd19 100644 --- a/src/test/ui/panic-while-printing.rs +++ b/src/test/ui/panic-while-printing.rs @@ -5,7 +5,8 @@ use std::fmt; use std::fmt::{Display, Formatter}; -use std::io::{self, set_panic, LocalOutput, Write}; +use std::io::{self, set_panic, Write}; +use std::sync::{Arc, Mutex}; pub struct A; @@ -16,6 +17,7 @@ impl Display for A { } struct Sink; + impl Write for Sink { fn write(&mut self, buf: &[u8]) -> io::Result { Ok(buf.len()) @@ -24,14 +26,9 @@ impl Write for Sink { Ok(()) } } -impl LocalOutput for Sink { - fn clone_box(&self) -> Box { - Box::new(Sink) - } -} fn main() { - set_panic(Some(Box::new(Sink))); + set_panic(Some(Arc::new(Mutex::new(Sink)))); assert!(std::panic::catch_unwind(|| { eprintln!("{}", A); }) diff --git a/src/test/ui/threads-sendsync/task-stderr.rs b/src/test/ui/threads-sendsync/task-stderr.rs index bc4bedac196..8bd78158b54 100644 --- a/src/test/ui/threads-sendsync/task-stderr.rs +++ b/src/test/ui/threads-sendsync/task-stderr.rs @@ -1,33 +1,21 @@ // run-pass // ignore-emscripten no threads support -#![feature(box_syntax, set_stdio)] +#![feature(set_stdio)] -use std::io::prelude::*; use std::io; use std::str; use std::sync::{Arc, Mutex}; use std::thread; -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(Sink(self.0.clone())) - } -} - fn main() { let data = Arc::new(Mutex::new(Vec::new())); - let sink = Sink(data.clone()); - let res = thread::Builder::new().spawn(move|| -> () { - io::set_panic(Some(Box::new(sink))); - panic!("Hello, world!") + let res = thread::Builder::new().spawn({ + let data = data.clone(); + move || { + io::set_panic(Some(data)); + panic!("Hello, world!") + } }).unwrap().join(); assert!(res.is_err()); From f534b75f050f2daca87c15f6c8d04bf9dc5b68a6 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 Nov 2020 21:44:21 +0100 Subject: [PATCH 3/5] Use Vec for LOCAL_STD{OUT,ERR} instead of dyn Write. It was only ever used with Vec anyway. This simplifies some things. - It no longer needs to be flushed, because that's a no-op anyway for a Vec. - Writing to a Vec never fails. - No #[cfg(test)] code is needed anymore to use `realstd` instead of `std`, because Vec comes from alloc, not std (like Write). --- compiler/rustc_interface/src/util.rs | 8 +--- library/std/src/io/stdio.rs | 59 +++++++++++----------------- library/std/src/lib.rs | 1 + library/std/src/panicking.rs | 23 +---------- src/test/ui/panic-while-printing.rs | 15 +------ 5 files changed, 28 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index d90fff3bae5..94a7a5ba793 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -148,9 +148,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(stderr.clone())); - } + io::set_panic(stderr.clone()); f() }) }; @@ -188,9 +186,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se // on the new threads. let main_handler = move |thread: rayon::ThreadBuilder| { rustc_span::SESSION_GLOBALS.set(session_globals, || { - if let Some(stderr) = stderr { - io::set_panic(Some(stderr.clone())); - } + io::set_panic(stderr.clone()); thread.run() }) }; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index e3e8a763591..f6038f4ce11 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -16,7 +16,7 @@ use crate::sys_common; use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use crate::thread::LocalKey; -type LocalStream = Arc>; +type LocalStream = Arc>>; thread_local! { /// Used by the test crate to capture the output of the print! and println! macros. @@ -911,13 +911,8 @@ pub fn set_panic(sink: Option) -> Option { // 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(|s| { - let _ = s.lock().unwrap_or_else(|e| e.into_inner()).flush(); - Some(s) - }); LOCAL_STREAMS.store(true, Ordering::Relaxed); - s + LOCAL_STDERR.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)) } /// Resets the thread-local stdout handle to the specified writer @@ -941,13 +936,8 @@ pub fn set_print(sink: Option) -> Option { // 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(|s| { - let _ = s.lock().unwrap_or_else(|e| e.into_inner()).flush(); - Some(s) - }); LOCAL_STREAMS.store(true, Ordering::Relaxed); - s + LOCAL_STDOUT.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)) } pub(crate) fn clone_io() -> (Option, Option) { @@ -956,9 +946,10 @@ pub(crate) fn clone_io() -> (Option, Option) { return (None, None); } - LOCAL_STDOUT.with(|stdout| { - LOCAL_STDERR.with(|stderr| (stdout.borrow().clone(), stderr.borrow().clone())) - }) + ( + LOCAL_STDOUT.with(|s| s.borrow().clone()), + LOCAL_STDERR.with(|s| s.borrow().clone()), + ) } /// Write `args` to output stream `local_s` if possible, `global_s` @@ -979,28 +970,22 @@ fn print_to( ) where T: Write, { - let result = LOCAL_STREAMS - .load(Ordering::Relaxed) - .then(|| { - local_s - .try_with(|s| { - // Note that we completely remove a local sink to write to in case - // 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(w) = prev { - let result = w.lock().unwrap_or_else(|e| e.into_inner()).write_fmt(args); - *s.borrow_mut() = Some(w); - return result; - } - global_s().write_fmt(args) - }) - .ok() - }) - .flatten() - .unwrap_or_else(|| global_s().write_fmt(args)); + if LOCAL_STREAMS.load(Ordering::Relaxed) + && local_s.try_with(|s| { + // Note that we completely remove a local sink to write to in case + // our printing recursively panics/prints, so the recursive + // panic/print goes to the global sink instead of our local sink. + s.take().map(|w| { + let _ = w.lock().unwrap_or_else(|e| e.into_inner()).write_fmt(args); + *s.borrow_mut() = Some(w); + }) + }) == Ok(Some(())) + { + // Succesfully wrote to local stream. + return; + } - if let Err(e) = result { + if let Err(e) = global_s().write_fmt(args) { panic!("failed printing to {}: {}", label, e); } } diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index bc218b77c87..a0c2a27438a 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -297,6 +297,7 @@ #![feature(raw)] #![feature(raw_ref_macros)] #![feature(ready_macro)] +#![feature(refcell_take)] #![feature(rustc_attrs)] #![feature(rustc_private)] #![feature(shrink_to)] diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 9e9584baeb3..a5d4d72b00d 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -219,28 +219,7 @@ fn default_hook(info: &PanicInfo<'_>) { }; 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); - + write(&mut *local.lock().unwrap_or_else(|e| e.into_inner())); set_panic(Some(local)); } else if let Some(mut out) = panic_output() { write(&mut out); diff --git a/src/test/ui/panic-while-printing.rs b/src/test/ui/panic-while-printing.rs index 555b90fbd19..fa740c5baa8 100644 --- a/src/test/ui/panic-while-printing.rs +++ b/src/test/ui/panic-while-printing.rs @@ -5,7 +5,7 @@ use std::fmt; use std::fmt::{Display, Formatter}; -use std::io::{self, set_panic, Write}; +use std::io::set_panic; use std::sync::{Arc, Mutex}; pub struct A; @@ -16,19 +16,8 @@ impl Display for A { } } -struct Sink; - -impl Write for Sink { - fn write(&mut self, buf: &[u8]) -> io::Result { - Ok(buf.len()) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} - fn main() { - set_panic(Some(Arc::new(Mutex::new(Sink)))); + set_panic(Some(Arc::new(Mutex::new(Vec::new())))); assert!(std::panic::catch_unwind(|| { eprintln!("{}", A); }) From 08b7cb79e0d76aab4d3a68335595c07b238c17b8 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 3 Nov 2020 22:59:04 +0100 Subject: [PATCH 4/5] Use Cell instead of RefCell for LOCAL_{STDOUT,STDERR}. --- library/std/src/io/stdio.rs | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index f6038f4ce11..2e71ad29584 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -5,7 +5,7 @@ mod tests; use crate::io::prelude::*; -use crate::cell::RefCell; +use crate::cell::{Cell, RefCell}; use crate::fmt; use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter}; use crate::lazy::SyncOnceCell; @@ -20,15 +20,15 @@ type LocalStream = Arc>>; thread_local! { /// Used by the test crate to capture the output of the print! and println! macros. - static LOCAL_STDOUT: RefCell> = { - RefCell::new(None) + static LOCAL_STDOUT: Cell> = { + Cell::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> = { - RefCell::new(None) + static LOCAL_STDERR: Cell> = { + Cell::new(None) } } @@ -906,13 +906,12 @@ impl fmt::Debug for StderrLock<'_> { )] #[doc(hidden)] 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; } LOCAL_STREAMS.store(true, Ordering::Relaxed); - LOCAL_STDERR.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)) + LOCAL_STDERR.with(move |slot| slot.replace(sink)) } /// Resets the thread-local stdout handle to the specified writer @@ -931,13 +930,12 @@ pub fn set_panic(sink: Option) -> Option { )] #[doc(hidden)] 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; } LOCAL_STREAMS.store(true, Ordering::Relaxed); - LOCAL_STDOUT.with(move |slot| mem::replace(&mut *slot.borrow_mut(), sink)) + LOCAL_STDOUT.with(move |slot| slot.replace(sink)) } pub(crate) fn clone_io() -> (Option, Option) { @@ -946,10 +944,13 @@ pub(crate) fn clone_io() -> (Option, Option) { return (None, None); } - ( - LOCAL_STDOUT.with(|s| s.borrow().clone()), - LOCAL_STDERR.with(|s| s.borrow().clone()), - ) + let clone = |cell: &Cell>| { + let s = cell.take(); + cell.set(s.clone()); + s + }; + + (LOCAL_STDOUT.with(clone), LOCAL_STDERR.with(clone)) } /// Write `args` to output stream `local_s` if possible, `global_s` @@ -964,7 +965,7 @@ pub(crate) fn clone_io() -> (Option, Option) { /// However, if the actual I/O causes an error, this function does panic. fn print_to( args: fmt::Arguments<'_>, - local_s: &'static LocalKey>>, + local_s: &'static LocalKey>>, global_s: fn() -> T, label: &str, ) where @@ -977,7 +978,7 @@ fn print_to( // panic/print goes to the global sink instead of our local sink. s.take().map(|w| { let _ = w.lock().unwrap_or_else(|e| e.into_inner()).write_fmt(args); - *s.borrow_mut() = Some(w); + s.set(Some(w)); }) }) == Ok(Some(())) { From aff7bd66e8b97a41d34c221007e12e4bbe535322 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 4 Nov 2020 00:11:14 +0100 Subject: [PATCH 5/5] Merge set_panic and set_print into set_output_capture. There were no use cases for setting them separately. Merging them simplifies some things. --- compiler/rustc_interface/src/lib.rs | 2 +- compiler/rustc_interface/src/util.rs | 4 +- library/std/src/io/mod.rs | 8 +- library/std/src/io/stdio.rs | 112 +++++------------- library/std/src/lib.rs | 2 +- library/std/src/panicking.rs | 8 +- library/std/src/thread/mod.rs | 6 +- library/test/src/bench.rs | 14 +-- library/test/src/lib.rs | 15 +-- ...nternals.md => internal-output-capture.md} | 2 +- .../src/library-features/set-stdio.md | 5 - src/test/ui/panic-while-printing.rs | 6 +- src/test/ui/threads-sendsync/task-stderr.rs | 4 +- 13 files changed, 57 insertions(+), 131 deletions(-) rename src/doc/unstable-book/src/library-features/{libstd-io-internals.md => internal-output-capture.md} (79%) delete mode 100644 src/doc/unstable-book/src/library-features/set-stdio.md diff --git a/compiler/rustc_interface/src/lib.rs b/compiler/rustc_interface/src/lib.rs index 88d2efe96d1..0935eb2bd71 100644 --- a/compiler/rustc_interface/src/lib.rs +++ b/compiler/rustc_interface/src/lib.rs @@ -1,6 +1,6 @@ #![feature(bool_to_option)] #![feature(box_syntax)] -#![feature(set_stdio)] +#![feature(internal_output_capture)] #![feature(nll)] #![feature(generator_trait)] #![feature(generators)] diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 94a7a5ba793..20a7b47313e 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -148,7 +148,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se let main_handler = move || { rustc_span::with_session_globals(edition, || { - io::set_panic(stderr.clone()); + io::set_output_capture(stderr.clone()); f() }) }; @@ -186,7 +186,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se // on the new threads. let main_handler = move |thread: rayon::ThreadBuilder| { rustc_span::SESSION_GLOBALS.set(session_globals, || { - io::set_panic(stderr.clone()); + io::set_output_capture(stderr.clone()); thread.run() }) }; diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e6b9314fd88..d1e5942cba8 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -269,20 +269,18 @@ pub use self::buffered::{BufReader, BufWriter, LineWriter}; pub use self::cursor::Cursor; #[stable(feature = "rust1", since = "1.0.0")] pub use self::error::{Error, ErrorKind, Result}; +#[unstable(feature = "internal_output_capture", issue = "none")] +#[doc(no_inline, hidden)] +pub use self::stdio::set_output_capture; #[stable(feature = "rust1", since = "1.0.0")] pub use self::stdio::{stderr, stdin, stdout, Stderr, Stdin, Stdout}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::stdio::{StderrLock, StdinLock, StdoutLock}; #[unstable(feature = "print_internals", issue = "none")] 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}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::util::{copy, empty, repeat, sink, Empty, Repeat, Sink}; -pub(crate) use self::stdio::clone_io; - mod buffered; mod cursor; mod error; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 2e71ad29584..f1dfb4c7730 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -14,37 +14,29 @@ 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: Cell> = { + /// Used by the test crate to capture the output of the print macros and panics. + static OUTPUT_CAPTURE: Cell> = { Cell::new(None) } } -thread_local! { - /// Used by the test crate to capture the output of the eprint! and eprintln! macros, and panics. - static LOCAL_STDERR: Cell> = { - Cell::new(None) - } -} - -/// Flag to indicate LOCAL_STDOUT and/or LOCAL_STDERR is used. +/// Flag to indicate OUTPUT_CAPTURE is used. /// -/// If both are None and were never set on any thread, this flag is set to -/// false, and both LOCAL_STDOUT and LOCAL_STDOUT can be safely ignored on all -/// threads, saving some time and memory registering an unused thread local. +/// If it is None and was never set on any thread, this flag is set to false, +/// and OUTPUT_CAPTURE can be safely ignored on all threads, saving some time +/// and memory registering an unused thread local. /// -/// Note about memory ordering: This contains information about whether two -/// thread local variables might be in use. Although this is a global flag, the +/// Note about memory ordering: This contains information about whether a +/// thread local variable might be in use. Although this is a global flag, the /// memory ordering between threads does not matter: we only want this flag to -/// have a consistent order between set_print/set_panic and print_to *within +/// have a consistent order between set_output_capture and print_to *within /// the same thread*. Within the same thread, things always have a perfectly /// consistent order. So Ordering::Relaxed is fine. -static LOCAL_STREAMS: AtomicBool = AtomicBool::new(false); +static OUTPUT_CAPTURE_USED: AtomicBool = AtomicBool::new(false); /// A handle to a raw instance of the standard input stream of this process. /// @@ -890,70 +882,24 @@ impl fmt::Debug for StderrLock<'_> { } } -/// Resets the thread-local stderr handle to the specified writer -/// -/// This will replace the current thread's stderr handle, returning the old -/// handle. All future calls to `panic!` and friends will emit their output to -/// this specified handle. -/// -/// Note that this does not need to be called for all new threads; the default -/// output handle is to the process's stderr stream. +/// Sets the thread-local output capture buffer and returns the old one. #[unstable( - feature = "set_stdio", - reason = "this function may disappear completely or be replaced \ - with a more general mechanism", + feature = "internal_output_capture", + reason = "this function is meant for use in the test crate \ + and may disappear in the future", issue = "none" )] #[doc(hidden)] -pub fn set_panic(sink: Option) -> Option { - if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { - // LOCAL_STDERR is definitely None since LOCAL_STREAMS is false. +pub fn set_output_capture(sink: Option) -> Option { + if sink.is_none() && !OUTPUT_CAPTURE_USED.load(Ordering::Relaxed) { + // OUTPUT_CAPTURE is definitely None since OUTPUT_CAPTURE_USED is false. return None; } - LOCAL_STREAMS.store(true, Ordering::Relaxed); - LOCAL_STDERR.with(move |slot| slot.replace(sink)) + OUTPUT_CAPTURE_USED.store(true, Ordering::Relaxed); + OUTPUT_CAPTURE.with(move |slot| slot.replace(sink)) } -/// Resets the thread-local stdout handle to the specified writer -/// -/// This will replace the current thread's stdout handle, returning the old -/// handle. All future calls to `print!` and friends will emit their output to -/// this specified handle. -/// -/// Note that this does not need to be called for all new threads; the default -/// output handle is to the process's stdout stream. -#[unstable( - feature = "set_stdio", - reason = "this function may disappear completely or be replaced \ - with a more general mechanism", - issue = "none" -)] -#[doc(hidden)] -pub fn set_print(sink: Option) -> Option { - if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { - // LOCAL_STDOUT is definitely None since LOCAL_STREAMS is false. - return None; - } - LOCAL_STREAMS.store(true, Ordering::Relaxed); - LOCAL_STDOUT.with(move |slot| slot.replace(sink)) -} - -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); - } - - let clone = |cell: &Cell>| { - let s = cell.take(); - cell.set(s.clone()); - s - }; - - (LOCAL_STDOUT.with(clone), LOCAL_STDERR.with(clone)) -} - -/// Write `args` to output stream `local_s` if possible, `global_s` +/// Write `args` to the capture buffer if enabled and possible, or `global_s` /// otherwise. `label` identifies the stream in a panic message. /// /// This function is used to print error messages, so it takes extra @@ -963,16 +909,12 @@ pub(crate) fn clone_io() -> (Option, Option) { /// thread, it will just fall back to the global stream. /// /// However, if the actual I/O causes an error, this function does panic. -fn print_to( - args: fmt::Arguments<'_>, - local_s: &'static LocalKey>>, - global_s: fn() -> T, - label: &str, -) where +fn print_to(args: fmt::Arguments<'_>, global_s: fn() -> T, label: &str) +where T: Write, { - if LOCAL_STREAMS.load(Ordering::Relaxed) - && local_s.try_with(|s| { + if OUTPUT_CAPTURE_USED.load(Ordering::Relaxed) + && OUTPUT_CAPTURE.try_with(|s| { // Note that we completely remove a local sink to write to in case // our printing recursively panics/prints, so the recursive // panic/print goes to the global sink instead of our local sink. @@ -982,7 +924,7 @@ fn print_to( }) }) == Ok(Some(())) { - // Succesfully wrote to local stream. + // Succesfully wrote to capture buffer. return; } @@ -999,7 +941,7 @@ fn print_to( #[doc(hidden)] #[cfg(not(test))] pub fn _print(args: fmt::Arguments<'_>) { - print_to(args, &LOCAL_STDOUT, stdout, "stdout"); + print_to(args, stdout, "stdout"); } #[unstable( @@ -1010,7 +952,7 @@ pub fn _print(args: fmt::Arguments<'_>) { #[doc(hidden)] #[cfg(not(test))] pub fn _eprint(args: fmt::Arguments<'_>) { - print_to(args, &LOCAL_STDERR, stderr, "stderr"); + print_to(args, stderr, "stderr"); } #[cfg(test)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index a0c2a27438a..334757aafd7 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -207,7 +207,7 @@ // std may use features in a platform-specific way #![allow(unused_features)] #![cfg_attr(not(bootstrap), feature(rustc_allow_const_fn_unstable))] -#![cfg_attr(test, feature(print_internals, set_stdio, update_panic_count))] +#![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count))] #![cfg_attr( all(target_vendor = "fortanix", target_env = "sgx"), feature(slice_index_methods, coerce_unsized, sgx_platform) diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index a5d4d72b00d..8ba3feccb6b 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -24,11 +24,11 @@ use crate::sys_common::{thread_info, util}; use crate::thread; #[cfg(not(test))] -use crate::io::set_panic; +use crate::io::set_output_capture; // make sure to use the stderr output configured // by libtest in the real copy of std #[cfg(test)] -use realstd::io::set_panic; +use realstd::io::set_output_capture; // Binary interface to the panic runtime that the standard library depends on. // @@ -218,9 +218,9 @@ fn default_hook(info: &PanicInfo<'_>) { } }; - if let Some(local) = set_panic(None) { + if let Some(local) = set_output_capture(None) { write(&mut *local.lock().unwrap_or_else(|e| e.into_inner())); - set_panic(Some(local)); + set_output_capture(Some(local)); } else if let Some(mut out) = panic_output() { write(&mut out); } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index fefaa77a2a1..5d65f960fcd 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -456,15 +456,15 @@ impl Builder { let my_packet: Arc>>> = Arc::new(UnsafeCell::new(None)); let their_packet = my_packet.clone(); - let (stdout, stderr) = crate::io::clone_io(); + let output_capture = crate::io::set_output_capture(None); + crate::io::set_output_capture(output_capture.clone()); let main = move || { if let Some(name) = their_thread.cname() { imp::Thread::set_name(name); } - crate::io::set_print(stdout); - crate::io::set_panic(stderr); + crate::io::set_output_capture(output_capture); // SAFETY: the stack guard passed is the one for the current thread. // This means the current thread's stack and the new thread's stack diff --git a/library/test/src/bench.rs b/library/test/src/bench.rs index 396dd8d3436..d4b37284ea7 100644 --- a/library/test/src/bench.rs +++ b/library/test/src/bench.rs @@ -184,18 +184,14 @@ where let mut bs = Bencher { mode: BenchMode::Auto, summary: None, bytes: 0 }; let data = Arc::new(Mutex::new(Vec::new())); - let oldio = if !nocapture { - Some((io::set_print(Some(data.clone())), io::set_panic(Some(data.clone())))) - } else { - None - }; + + if !nocapture { + io::set_output_capture(Some(data.clone())); + } let result = catch_unwind(AssertUnwindSafe(|| bs.bench(f))); - if let Some((printio, panicio)) = oldio { - io::set_print(printio); - io::set_panic(panicio); - } + io::set_output_capture(None); let test_result = match result { //bs.bench(f) { diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 7d0ce6dfbd1..816b4d51188 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -25,7 +25,7 @@ #![feature(nll)] #![feature(bool_to_option)] #![feature(available_concurrency)] -#![feature(set_stdio)] +#![feature(internal_output_capture)] #![feature(panic_unwind)] #![feature(staged_api)] #![feature(termination_trait_lib)] @@ -530,11 +530,9 @@ fn run_test_in_process( // Buffer for capturing standard I/O let data = Arc::new(Mutex::new(Vec::new())); - let oldio = if !nocapture { - Some((io::set_print(Some(data.clone())), io::set_panic(Some(data.clone())))) - } else { - None - }; + if !nocapture { + io::set_output_capture(Some(data.clone())); + } let start = report_time.then(Instant::now); let result = catch_unwind(AssertUnwindSafe(testfn)); @@ -543,10 +541,7 @@ fn run_test_in_process( TestExecTime(duration) }); - if let Some((printio, panicio)) = oldio { - io::set_print(printio); - io::set_panic(panicio); - } + io::set_output_capture(None); let test_result = match result { Ok(()) => calc_result(&desc, Ok(()), &time_opts, &exec_time), diff --git a/src/doc/unstable-book/src/library-features/libstd-io-internals.md b/src/doc/unstable-book/src/library-features/internal-output-capture.md similarity index 79% rename from src/doc/unstable-book/src/library-features/libstd-io-internals.md rename to src/doc/unstable-book/src/library-features/internal-output-capture.md index 8bcc2769db7..7e1241fce98 100644 --- a/src/doc/unstable-book/src/library-features/libstd-io-internals.md +++ b/src/doc/unstable-book/src/library-features/internal-output-capture.md @@ -1,4 +1,4 @@ -# `libstd_io_internals` +# `internal_output_capture` This feature is internal to the Rust compiler and is not intended for general use. diff --git a/src/doc/unstable-book/src/library-features/set-stdio.md b/src/doc/unstable-book/src/library-features/set-stdio.md deleted file mode 100644 index 7dbdcdaa1a2..00000000000 --- a/src/doc/unstable-book/src/library-features/set-stdio.md +++ /dev/null @@ -1,5 +0,0 @@ -# `set_stdio` - -This feature is internal to the Rust compiler and is not intended for general use. - ------------------------- diff --git a/src/test/ui/panic-while-printing.rs b/src/test/ui/panic-while-printing.rs index fa740c5baa8..96b35e45535 100644 --- a/src/test/ui/panic-while-printing.rs +++ b/src/test/ui/panic-while-printing.rs @@ -1,11 +1,11 @@ // run-pass // ignore-emscripten no subprocess support -#![feature(set_stdio)] +#![feature(internal_output_capture)] use std::fmt; use std::fmt::{Display, Formatter}; -use std::io::set_panic; +use std::io::set_output_capture; use std::sync::{Arc, Mutex}; pub struct A; @@ -17,7 +17,7 @@ impl Display for A { } fn main() { - set_panic(Some(Arc::new(Mutex::new(Vec::new())))); + set_output_capture(Some(Arc::new(Mutex::new(Vec::new())))); assert!(std::panic::catch_unwind(|| { eprintln!("{}", A); }) diff --git a/src/test/ui/threads-sendsync/task-stderr.rs b/src/test/ui/threads-sendsync/task-stderr.rs index 8bd78158b54..78145e337da 100644 --- a/src/test/ui/threads-sendsync/task-stderr.rs +++ b/src/test/ui/threads-sendsync/task-stderr.rs @@ -1,7 +1,7 @@ // run-pass // ignore-emscripten no threads support -#![feature(set_stdio)] +#![feature(internal_output_capture)] use std::io; use std::str; @@ -13,7 +13,7 @@ fn main() { let res = thread::Builder::new().spawn({ let data = data.clone(); move || { - io::set_panic(Some(data)); + io::set_output_capture(Some(data)); panic!("Hello, world!") } }).unwrap().join();