Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error-stack: move fmt::HookContext into hook::HookContext #1693

Merged
merged 6 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
360 changes: 20 additions & 340 deletions packages/libs/error-stack/src/fmt/hook.rs

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions packages/libs/error-stack/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ use core::{
#[cfg(any(feature = "std", feature = "hooks"))]
pub use hook::HookContext;
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) use hook::{install_builtin_hooks, Hooks};
pub(crate) use hook::{install_builtin_hooks, Format, Hooks};
#[cfg(feature = "pretty-print")]
use owo_colors::{OwoColorize, Stream, Style as OwOStyle};

Expand Down Expand Up @@ -947,7 +947,7 @@ fn debug_frame(
impl<C> Debug for Report<C> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
#[cfg(any(feature = "std", feature = "hooks"))]
let mut context = HookContext::new(fmt.alternate());
let mut context = HookContext::new(Format::new(fmt.alternate()));

#[cfg_attr(not(any(feature = "std", feature = "hooks")), allow(unused_mut))]
let mut lines = self
Expand All @@ -958,7 +958,7 @@ impl<C> Debug for Report<C> {
frame,
&[],
#[cfg(any(feature = "std", feature = "hooks"))]
&mut context,
context.cast(),
)
})
.enumerate()
Expand Down
366 changes: 366 additions & 0 deletions packages/libs/error-stack/src/hook/context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,366 @@
use alloc::{boxed::Box, collections::BTreeMap};
use core::{
any::{Any, TypeId},
marker::PhantomData,
};

type Storage = BTreeMap<TypeId, BTreeMap<TypeId, Box<dyn Any>>>;

/// Private struct which is used to hold the information about the current count for every type.
/// This is used so that others cannot interfere with the counter and ensure that there's no
/// unexpected behavior.
struct Counter(isize);

impl Counter {
const fn new(value: isize) -> Self {
Self(value)
}

const fn as_inner(&self) -> isize {
self.0
}

fn increment(&mut self) {
self.0 += 1;
}

fn decrement(&mut self) {
self.0 -= 1;
}
}

pub(crate) struct Inner<T> {
storage: Storage,
extra: T,
}

impl<T> Inner<T> {
pub(crate) fn new(extra: T) -> Self {
Self {
storage: Storage::new(),
extra,
}
}
}

impl<T> Inner<T> {
pub(crate) const fn storage(&self) -> &Storage {
&self.storage
}

pub(crate) fn storage_mut(&mut self) -> &mut Storage {
&mut self.storage
}

pub(crate) const fn extra(&self) -> &T {
&self.extra
}

pub(crate) fn extra_mut(&mut self) -> &mut T {
&mut self.extra
}
}

/// Internal [`HookContext`]
///
/// This is an implementation detail and cannot be used directly. This is only exposed for
/// documentation purposes and cannot be directly imported.
///
/// Instead use [`crate::fmt::HookContext`] for [`Debug`] hooks.
// TODO: add link to serde hooks once implemented
// TODO: ideally we would want to make `HookContextInner` private, as it is an implementation
// detail, but "attribute privacy" as outlined in https://github.com/rust-lang/rust/pull/61969
// is currently not implemented for repr(transparent).
#[cfg_attr(not(doc), repr(transparent))]
pub struct HookContext<Extra, T> {
inner: Inner<Extra>,
_marker: PhantomData<fn(&T)>,
}

impl<T> HookContext<T, ()> {
pub(crate) fn new(extra: T) -> Self {
Self {
inner: Inner::new(extra),
_marker: PhantomData,
}
}
}

impl<Extra, T> HookContext<Extra, T> {
pub(crate) const fn inner(&self) -> &Inner<Extra> {
&self.inner
}

pub(crate) fn inner_mut(&mut self) -> &mut Inner<Extra> {
&mut self.inner
}

fn storage(&self) -> &Storage {
self.inner().storage()
}

fn storage_mut(&mut self) -> &mut Storage {
self.inner_mut().storage_mut()
}
}

impl<Extra, T> HookContext<Extra, T> {
/// Cast the [`HookContext`] to a new type `U`.
///
/// The storage of [`HookContext`] is partitioned, meaning that if `T` and `U` are different
/// types the values stored in [`HookContext<_, T>`] will be separated from values in
/// [`HookContext<_, U>`].
///
/// In most situations this functions isn't needed, as it transparently casts between different
/// partitions of the storage. Only hooks that share storage with hooks of different types
/// should need to use this function.
///
/// ### Example
///
/// ```rust
/// # // we only test with Rust 1.65, which means that `render()` is unused on earlier version
/// # #![cfg_attr(not(rust_1_65), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::ErrorKind;
///
/// use error_stack::Report;
///
/// struct Warning(&'static str);
/// struct Error(&'static str);
///
/// Report::install_debug_hook::<Error>(|Error(frame), context| {
/// let idx = context.increment_counter() + 1;
///
/// context.push_body(format!("[{idx}] [ERROR] {frame}"));
/// });
/// Report::install_debug_hook::<Warning>(|Warning(frame), context| {
/// // We want to share the same counter with `Error`, so that we're able to have
/// // a global counter to keep track of all errors and warnings in order, this means
/// // we need to access the storage of `Error` using `cast()`.
/// let context = context.cast::<Error>();
/// let idx = context.increment_counter() + 1;
/// context.push_body(format!("[{idx}] [WARN] {frame}"))
/// });
///
/// let report = Report::new(std::io::Error::from(ErrorKind::InvalidInput))
/// .attach(Error("unable to reach remote host"))
/// .attach(Warning("disk nearly full"))
/// .attach(Error("cannot resolve example.com: unknown host"));
///
/// # owo_colors::set_override(true);
/// # fn render(value: String) -> String {
/// # let backtrace = regex::Regex::new(r"backtrace no\. (\d+)\n(?: .*\n)* .*").unwrap();
/// # let backtrace_info = regex::Regex::new(r"backtrace( with (\d+) frames)? \((\d+)\)").unwrap();
/// #
/// # let value = backtrace.replace_all(&value, "backtrace no. $1\n [redacted]");
/// # let value = backtrace_info.replace_all(value.as_ref(), "backtrace ($3)");
/// #
/// # ansi_to_html::convert_escaped(value.as_ref()).unwrap()
/// # }
/// #
/// # #[cfg(rust_1_65)]
/// # expect_test::expect_file![concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_cast.snap")].assert_eq(&render(format!("{report:?}")));
/// #
/// println!("{report:?}");
/// ```
///
/// <pre>
#[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_cast.snap"))]
/// </pre>
#[must_use]
pub fn cast<U>(&mut self) -> &mut HookContext<Extra, U> {
// SAFETY: `HookContext` is marked as repr(transparent) and the changed generic is only used
// inside of the `PhantomData`
unsafe { &mut *(self as *mut Self).cast::<HookContext<Extra, U>>() }
}
Comment on lines +170 to +174
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take the opportunity to ask this question as we touched this code: Do you see a way to remove the unsafe here? I know, why we have, I'm just wondering if there is a way to avoid it (this question is unrelated to this PR).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, if we would instead transmute mut self instead we could use RFC 2528 may be of help.

}

impl<I, T: 'static> HookContext<I, T> {
/// Return a reference to a value of type `U`, if a value of that type exists.
///
/// Values returned are isolated and "bound" to `T`, this means that [`HookContext<_, Warning>`]
/// and [`HookContext<_, Error>`] do not share the same values. Values are only valid during the
/// invocation of the corresponding call (e.g. [`Debug`]).
///
/// [`Debug`]: core::fmt::Debug
#[must_use]
pub fn get<U: 'static>(&self) -> Option<&U> {
self.storage()
.get(&TypeId::of::<T>())?
.get(&TypeId::of::<U>())?
.downcast_ref()
}

/// Return a mutable reference to a value of type `U`, if a value of that type exists.
///
/// Values returned are isolated and "bound" to `T`, this means that [`HookContext<_, Warning>`]
/// and [`HookContext<_, Error>`] do not share the same values. Values are only valid during the
/// invocation of the corresponding call (e.g. [`Debug`]).
///
/// [`Debug`]: core::fmt::Debug
pub fn get_mut<U: 'static>(&mut self) -> Option<&mut U> {
self.storage_mut()
.get_mut(&TypeId::of::<T>())?
.get_mut(&TypeId::of::<U>())?
.downcast_mut()
}

/// Insert a new value of type `U` into the storage of [`HookContext`].
///
/// The returned value will the previously stored value of the same type `U` scoped over type
/// `T`, if it existed, did no such value exist it will return [`None`].
pub fn insert<U: 'static>(&mut self, value: U) -> Option<U> {
self.storage_mut()
.entry(TypeId::of::<T>())
.or_default()
.insert(TypeId::of::<U>(), Box::new(value))?
.downcast()
.map(|boxed| *boxed)
.ok()
}

/// Remove the value of type `U` from the storage of [`HookContext`] if it existed.
///
/// The returned value will be the previously stored value of the same type `U` if it existed in
/// the scope of `T`, did no such value exist, it will return [`None`].
pub fn remove<U: 'static>(&mut self) -> Option<U> {
self.storage_mut()
.get_mut(&TypeId::of::<T>())?
.remove(&TypeId::of::<U>())?
.downcast()
.map(|boxed| *boxed)
.ok()
}

/// One of the most common interactions with [`HookContext`] is a counter to reference previous
/// frames in an entry to the appendix that was added using [`HookContext::push_appendix`].
///
/// This is a utility method, which uses the other primitive methods provided to automatically
/// increment a counter, if the counter wasn't initialized this method will return `0`.
///
/// ```rust
/// # // we only test with Rust 1.65, which means that `render()` is unused on earlier version
/// # #![cfg_attr(not(rust_1_65), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::ErrorKind;
///
/// use error_stack::Report;
///
/// struct Suggestion(&'static str);
///
/// Report::install_debug_hook::<Suggestion>(|Suggestion(value), context| {
/// let idx = context.increment_counter();
/// context.push_body(format!("suggestion {idx}: {value}"));
/// });
///
/// let report = Report::new(std::io::Error::from(ErrorKind::InvalidInput))
/// .attach(Suggestion("use a file you can read next time!"))
/// .attach(Suggestion("don't press any random keys!"));
///
/// # owo_colors::set_override(true);
/// # fn render(value: String) -> String {
/// # let backtrace = regex::Regex::new(r"backtrace no\. (\d+)\n(?: .*\n)* .*").unwrap();
/// # let backtrace_info = regex::Regex::new(r"backtrace( with (\d+) frames)? \((\d+)\)").unwrap();
/// #
/// # let value = backtrace.replace_all(&value, "backtrace no. $1\n [redacted]");
/// # let value = backtrace_info.replace_all(value.as_ref(), "backtrace ($3)");
/// #
/// # ansi_to_html::convert_escaped(value.as_ref()).unwrap()
/// # }
/// #
/// # #[cfg(rust_1_65)]
/// # expect_test::expect_file![concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_increment.snap")].assert_eq(&render(format!("{report:?}")));
/// #
/// println!("{report:?}");
/// ```
///
/// <pre>
#[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_increment.snap"))]
/// </pre>
///
/// [`Debug`]: core::fmt::Debug
pub fn increment_counter(&mut self) -> isize {
let counter = self.get_mut::<Counter>();

// reason: This would fail as we cannot move out of `self` because it is borrowed
#[allow(clippy::option_if_let_else)]
match counter {
None => {
// if the counter hasn't been set yet, default to `0`
self.insert(Counter::new(0));

0
}
Some(ctr) => {
ctr.increment();

ctr.as_inner()
}
}
}

/// One of the most common interactions with [`HookContext`] is a counter to reference previous
/// frames in an entry to the appendix that was added using [`HookContext::push_appendix`].
///
/// This is a utility method, which uses the other primitive method provided to automatically
/// decrement a counter, if the counter wasn't initialized this method will return `-1` to stay
/// consistent with [`HookContext::increment_counter`].
///
/// ```rust
/// # // we only test with Rust 1.65, which means that `render()` is unused on earlier version
/// # #![cfg_attr(not(rust_1_65), allow(dead_code, unused_variables, unused_imports))]
/// use std::io::ErrorKind;
///
/// use error_stack::Report;
///
/// struct Suggestion(&'static str);
///
/// Report::install_debug_hook::<Suggestion>(|Suggestion(value), context| {
/// let idx = context.decrement_counter();
/// context.push_body(format!("suggestion {idx}: {value}"));
/// });
///
/// let report = Report::new(std::io::Error::from(ErrorKind::InvalidInput))
/// .attach(Suggestion("use a file you can read next time!"))
/// .attach(Suggestion("don't press any random keys!"));
///
/// # owo_colors::set_override(true);
/// # fn render(value: String) -> String {
/// # let backtrace = regex::Regex::new(r"backtrace no\. (\d+)\n(?: .*\n)* .*").unwrap();
/// # let backtrace_info = regex::Regex::new(r"backtrace( with (\d+) frames)? \((\d+)\)").unwrap();
/// #
/// # let value = backtrace.replace_all(&value, "backtrace no. $1\n [redacted]");
/// # let value = backtrace_info.replace_all(value.as_ref(), "backtrace ($3)");
/// #
/// # ansi_to_html::convert_escaped(value.as_ref()).unwrap()
/// # }
/// #
/// # #[cfg(rust_1_65)]
/// # expect_test::expect_file![concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_decrement.snap")].assert_eq(&render(format!("{report:?}")));
/// #
/// println!("{report:?}");
/// ```
///
/// <pre>
#[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots/doc/fmt__hookcontext_decrement.snap"))]
/// </pre>
pub fn decrement_counter(&mut self) -> isize {
let counter = self.get_mut::<Counter>();

// reason: This would fail as we cannot move out of `self` because it is borrowed
#[allow(clippy::option_if_let_else)]
match counter {
None => {
// given that increment starts with `0` (which is therefore the implicit default
// value) decrementing the default value results in `-1`,
// which is why we output that value.
self.insert(Counter::new(-1));

-1
}
Some(ctr) => {
ctr.decrement();

ctr.as_inner()
}
}
}
}