From 2ecbc56de984fdfffa3885c4e8bd344d4acd86d8 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Fri, 1 Mar 2024 19:42:43 -0600 Subject: [PATCH] Change MainThread abstraction Don't force the internal use of `RefCell`, let the caller place that into `MainThread<>` manually. This lets us remove the reference to `MainThread<>` from the definition of `Screen` again and reduces the number of `assert_is_main_thread()` calls. --- src/output.rs | 9 +++++---- src/reader.rs | 29 +++++++++++++++-------------- src/screen.rs | 48 +++++++++++++++++++++++------------------------- src/threads.rs | 20 ++++---------------- 4 files changed, 47 insertions(+), 59 deletions(-) diff --git a/src/output.rs b/src/output.rs index c93aed11c..35554b35c 100644 --- a/src/output.rs +++ b/src/output.rs @@ -6,6 +6,7 @@ use crate::env::EnvVar; use crate::threads::MainThread; use crate::wchar::prelude::*; use bitflags::bitflags; +use std::cell::RefCell; use std::ffi::CStr; use std::io::{Result, Write}; use std::os::fd::RawFd; @@ -444,10 +445,10 @@ impl Outputter { /// Access the outputter for stdout. /// This should only be used from the main thread. - pub fn stdoutput() -> &'static MainThread { - static STDOUTPUT: MainThread = - MainThread::new(Outputter::new_from_fd(libc::STDOUT_FILENO)); - &STDOUTPUT + pub fn stdoutput() -> &'static RefCell { + static STDOUTPUT: MainThread> = + MainThread::new(RefCell::new(Outputter::new_from_fd(libc::STDOUT_FILENO))); + STDOUTPUT.get() } } diff --git a/src/reader.rs b/src/reader.rs index dd7089ed0..e0ab72f20 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -1902,7 +1902,8 @@ impl ReaderData { perror("tcsetattr"); // return to previous mode } Outputter::stdoutput() - .with_mut(|output| output.set_color(RgbColor::RESET, RgbColor::RESET)); + .borrow_mut() + .set_color(RgbColor::RESET, RgbColor::RESET); } rls.finished.then(|| zelf.command_line.text().to_owned()) } @@ -2941,9 +2942,9 @@ impl ReaderData { el.end_edit_group(); } rl::DisableMouseTracking => { - Outputter::stdoutput().with_mut(|outp| { - outp.write_wstr(L!("\x1B[?1000l")); - }); + Outputter::stdoutput() + .borrow_mut() + .write_wstr(L!("\x1B[?1000l")); } rl::ClearScreenAndRepaint => { self.parser().libdata_mut().pods.is_repaint = true; @@ -2954,9 +2955,7 @@ impl ReaderData { // and *then* reexecute the prompt and overdraw it. // This removes the flicker, // while keeping the prompt up-to-date. - Outputter::stdoutput().with_mut(|outp| { - outp.write_wstr(&clear); - }); + Outputter::stdoutput().borrow_mut().write_wstr(&clear); self.screen.reset_line(/*repaint_prompt=*/ true); self.layout_and_repaint(L!("readline")); } @@ -3488,9 +3487,9 @@ fn reader_interactive_init(parser: &Parser) { /// Destroy data for interactive use. fn reader_interactive_destroy() { - Outputter::stdoutput().with_mut(|outp| { - outp.set_color(RgbColor::RESET, RgbColor::RESET); - }); + Outputter::stdoutput() + .borrow_mut() + .set_color(RgbColor::RESET, RgbColor::RESET); } /// \return whether fish is currently unwinding the stack in preparation to exit. @@ -3571,9 +3570,9 @@ pub fn reader_write_title( let _ = write_loop(&STDOUT_FILENO, &narrow); } - Outputter::stdoutput().with_mut(|outp| { - outp.set_color(RgbColor::RESET, RgbColor::RESET); - }); + Outputter::stdoutput() + .borrow_mut() + .set_color(RgbColor::RESET, RgbColor::RESET); if reset_cursor_position && !lst.is_empty() { // Put the cursor back at the beginning of the line (issue #2453). let _ = write_to_fd(b"\r", STDOUT_FILENO); @@ -4585,7 +4584,9 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { } reader_write_title(cmd, parser, true); - Outputter::stdoutput().with_mut(|outp| outp.set_color(RgbColor::NORMAL, RgbColor::NORMAL)); + Outputter::stdoutput() + .borrow_mut() + .set_color(RgbColor::NORMAL, RgbColor::NORMAL); term_donate(false); let time_before = Instant::now(); diff --git a/src/screen.rs b/src/screen.rs index 648f48eb4..4458fae8c 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -8,7 +8,7 @@ //! of text around to handle text insertion. use crate::pager::{PageRendering, Pager}; -use crate::threads::MainThread; +use std::cell::RefCell; use std::collections::LinkedList; use std::ffi::{CStr, CString}; use std::io::Write; @@ -177,7 +177,7 @@ pub struct Screen { pub autosuggestion_is_truncated: bool, /// Receiver for our output. - outp: &'static MainThread, + outp: &'static RefCell, /// The internal representation of the desired screen contents. desired: ScreenData, @@ -638,9 +638,9 @@ impl Screen { // Either issue a cr to go back to the beginning of this line, or a nl to go to the // beginning of the next one, depending on what we think is more efficient. if new_y <= zelf.actual.cursor.y { - zelf.outp.with_mut(|outp| outp.push(b'\r')); + zelf.outp.borrow_mut().push(b'\r'); } else { - zelf.outp.with_mut(|outp| outp.push(b'\n')); + zelf.outp.borrow_mut().push(b'\n'); zelf.actual.cursor.y += 1; } // Either way we're not in the first column. @@ -674,16 +674,14 @@ impl Screen { None }; - zelf.outp.with_mut(|outp| { - for _ in 0..y_steps.abs_diff(0) { - outp.tputs_if_some(&s); - } - }); + for _ in 0..y_steps.abs_diff(0) { + zelf.outp.borrow_mut().tputs_if_some(&s); + } let mut x_steps = isize::try_from(new_x).unwrap() - isize::try_from(zelf.actual.cursor.x).unwrap(); if x_steps != 0 && new_x == 0 { - zelf.outp.with_mut(|outp| outp.push(b'\r')); + zelf.outp.borrow_mut().push(b'\r'); x_steps = 0; } @@ -703,10 +701,10 @@ impl Screen { multi_str.as_ref().unwrap(), i32::try_from(x_steps.abs_diff(0)).unwrap(), ); - zelf.outp.with_mut(|outp| outp.tputs_if_some(&multi_param)); + zelf.outp.borrow_mut().tputs_if_some(&multi_param); } else { for _ in 0..x_steps.abs_diff(0) { - zelf.outp.with_mut(|outp| outp.tputs_if_some(&s)); + zelf.outp.borrow_mut().tputs_if_some(&s); } } @@ -718,7 +716,7 @@ impl Screen { fn write_char(&mut self, c: char, width: isize) { let mut zelf = self.scoped_buffer(); zelf.actual.cursor.x = zelf.actual.cursor.x.wrapping_add(width as usize); - zelf.outp.with_mut(|outp| outp.writech(c)); + zelf.outp.borrow_mut().writech(c); if Some(zelf.actual.cursor.x) == zelf.actual.screen_width && allow_soft_wrap() { zelf.soft_wrap_location = Some(Cursor { x: 0, @@ -735,16 +733,16 @@ impl Screen { /// Send the specified string through tputs and append the output to the screen's outputter. fn write_mbs(&mut self, s: &CStr) { - self.outp.with_mut(|outp| outp.tputs(s)); + self.outp.borrow_mut().tputs(s); } fn write_mbs_if_some(&mut self, s: &Option>) -> bool { - self.outp.with_mut(|outp| outp.tputs_if_some(s)) + self.outp.borrow_mut().tputs_if_some(s) } /// Convert a wide string to a multibyte string and append it to the buffer. fn write_str(&mut self, s: &wstr) { - self.outp.with_mut(|outp| outp.write_wstr(s)); + self.outp.borrow_mut().write_wstr(s); } /// Update the cursor as if soft wrapping had been performed. @@ -769,9 +767,9 @@ impl Screen { } fn scoped_buffer(&mut self) -> impl ScopeGuarding { - self.outp.with_mut(Outputter::begin_buffering); + self.outp.borrow_mut().begin_buffering(); ScopeGuard::new(self, |zelf| { - zelf.outp.with_mut(Outputter::end_buffering); + zelf.outp.borrow_mut().end_buffering(); }) } @@ -782,7 +780,7 @@ impl Screen { let mut set_color = |zelf: &mut Self, c| { let fg = color_resolver.resolve_spec(&c, false, vars); let bg = color_resolver.resolve_spec(&c, true, vars); - zelf.outp.with_mut(|outp| outp.set_color(fg, bg)); + zelf.outp.borrow_mut().set_color(fg, bg); }; let mut cached_layouts = LAYOUT_CACHE_SHARED.lock().unwrap(); @@ -837,9 +835,9 @@ impl Screen { let mut start = 0; for line_break in left_prompt_layout.line_breaks { zelf.write_str(&left_prompt[start..line_break]); - zelf.outp.with_mut(|outp| { - outp.tputs_if_some(&term.and_then(|term| term.clr_eol.as_ref())); - }); + zelf.outp + .borrow_mut() + .tputs_if_some(&term.and_then(|term| term.clr_eol.as_ref())); start = line_break; } zelf.write_str(&left_prompt[start..]); @@ -1071,9 +1069,9 @@ impl Screen { /// Issues an immediate clr_eos. pub fn screen_force_clear_to_end() { - Outputter::stdoutput().with_mut(|outp| { - outp.tputs_if_some(&term().unwrap().clr_eos); - }); + Outputter::stdoutput() + .borrow_mut() + .tputs_if_some(&term().unwrap().clr_eos); } /// Information about the layout of a prompt. diff --git a/src/threads.rs b/src/threads.rs index 0e7c7905c..0c5d27708 100644 --- a/src/threads.rs +++ b/src/threads.rs @@ -4,7 +4,6 @@ use crate::flog::{FloggableDebug, FLOG}; use crate::reader::ReaderData; use once_cell::race::OnceBox; -use std::cell::RefCell; use std::marker::PhantomData; use std::num::NonZeroU64; use std::sync::atomic::{AtomicBool, Ordering}; @@ -363,7 +362,7 @@ impl ThreadPool { /// A `Sync` and `Send` wrapper for non-`Sync`/`Send` types. /// Only allows access from the main thread. pub struct MainThread { - data: RefCell, + data: T, // Make type !Send and !Sync by default _marker: PhantomData<*const ()>, } @@ -376,25 +375,14 @@ unsafe impl Sync for MainThread {} impl MainThread { pub const fn new(value: T) -> Self { Self { - data: RefCell::new(value), + data: value, _marker: PhantomData, } } - pub fn with(&self, f: F) -> R - where - F: FnOnce(&T) -> R, - { + pub fn get(&self) -> &T { assert_is_main_thread(); - f(&self.data.borrow()) - } - - pub fn with_mut(&self, f: F) -> R - where - F: FnOnce(&mut T) -> R, - { - assert_is_main_thread(); - f(&mut self.data.borrow_mut()) + &self.data } }