Skip to content

Commit

Permalink
Pass Dispatch as owned value
Browse files Browse the repository at this point in the history
As an alternative to tokio-rs#1045, this PR switches `set_default` and `with_default` to take an owned `Dispatch` instead of a ref.

See [comment here](tokio-rs#1045 (comment)) for motivation.

Closes tokio-rs#455
Part of tokio-rs#922
  • Loading branch information
dvdplm committed Oct 19, 2020
1 parent a8cc977 commit c2fa02e
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 29 deletions.
12 changes: 6 additions & 6 deletions tracing-core/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
//! // no default subscriber
//!
//! # #[cfg(feature = "std")]
//! dispatcher::with_default(&my_dispatch, || {
//! dispatcher::with_default(my_dispatch, || {
//! // my_subscriber is the default
//! });
//!
Expand Down Expand Up @@ -227,7 +227,7 @@ pub struct DefaultGuard(Option<Dispatch>);
/// [`set_global_default`]: ../fn.set_global_default.html
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn with_default<T>(dispatcher: &Dispatch, f: impl FnOnce() -> T) -> T {
pub fn with_default<T>(dispatcher: Dispatch, f: impl FnOnce() -> T) -> T {
// When this guard is dropped, the default dispatcher will be reset to the
// prior default. Using this (rather than simply resetting after calling
// `f`) ensures that we always reset to the prior dispatcher even if `f`
Expand All @@ -253,7 +253,7 @@ pub fn with_default<T>(dispatcher: &Dispatch, f: impl FnOnce() -> T) -> T {
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
#[must_use = "Dropping the guard unregisters the dispatcher."]
pub fn set_default(dispatcher: &Dispatch) -> DefaultGuard {
pub fn set_default(dispatcher: Dispatch) -> DefaultGuard {
// When this guard is dropped, the default dispatcher will be reset to the
// prior default. Using this ensures that we always reset to the prior
// dispatcher even if the thread calling this function panics.
Expand Down Expand Up @@ -856,7 +856,7 @@ mod test {
fn exit(&self, _: &span::Id) {}
}

with_default(&Dispatch::new(TestSubscriber), || {
with_default(Dispatch::new(TestSubscriber), || {
Event::dispatch(&TEST_META, &TEST_META.fields().value_set(&[]))
})
}
Expand Down Expand Up @@ -904,7 +904,7 @@ mod test {
fn exit(&self, _: &span::Id) {}
}

with_default(&Dispatch::new(TestSubscriber), mk_span)
with_default(Dispatch::new(TestSubscriber), mk_span)
}

#[test]
Expand Down Expand Up @@ -936,7 +936,7 @@ mod test {

fn exit(&self, _: &span::Id) {}
}
let guard = set_default(&Dispatch::new(TestSubscriber));
let guard = set_default(Dispatch::new(TestSubscriber));
let default_dispatcher = Dispatch::default();
assert!(default_dispatcher.is::<TestSubscriber>());

Expand Down
6 changes: 3 additions & 3 deletions tracing-core/tests/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn set_default_dispatch() {
)
});

let guard = set_default(&Dispatch::new(TestSubscriberB));
let guard = set_default(Dispatch::new(TestSubscriberB));
get_default(|current| assert!(current.is::<TestSubscriberB>(), "set_default get failed"));

// Drop the guard, setting the dispatch back to the global dispatch
Expand All @@ -31,15 +31,15 @@ fn set_default_dispatch() {
#[cfg(feature = "std")]
#[test]
fn nested_set_default() {
let _guard = set_default(&Dispatch::new(TestSubscriberA));
let _guard = set_default(Dispatch::new(TestSubscriberA));
get_default(|current| {
assert!(
current.is::<TestSubscriberA>(),
"set_default for outer subscriber failed"
)
});

let inner_guard = set_default(&Dispatch::new(TestSubscriberB));
let inner_guard = set_default(Dispatch::new(TestSubscriberB));
get_default(|current| {
assert!(
current.is::<TestSubscriberB>(),
Expand Down
2 changes: 1 addition & 1 deletion tracing-core/tests/global_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn global_dispatch() {
});

#[cfg(feature = "std")]
with_default(&Dispatch::new(TestSubscriberB), || {
with_default(Dispatch::new(TestSubscriberB), || {
get_default(|current| {
assert!(
current.is::<TestSubscriberB>(),
Expand Down
4 changes: 2 additions & 2 deletions tracing-futures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ impl<T: futures_01::Future> futures_01::Future for WithDispatch<T> {

fn poll(&mut self) -> futures_01::Poll<Self::Item, Self::Error> {
let inner = &mut self.inner;
dispatcher::with_default(&self.dispatch, || inner.poll())
dispatcher::with_default(self.dispatch.clone(), || inner.poll())
}
}

Expand All @@ -452,7 +452,7 @@ impl<T: crate::stdlib::future::Future> crate::stdlib::future::Future for WithDis
let this = self.project();
let dispatch = this.dispatch;
let future = this.inner;
dispatcher::with_default(dispatch, || future.poll(cx))
dispatcher::with_default(dispatch.clone(), || future.poll(cx))
}
}

Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/fmt/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ mod test {
};
let dispatch = Dispatch::from(subscriber);

dispatcher::with_default(&dispatch, || {
dispatcher::with_default(dispatch, || {
error!("{}", msg);
});

Expand Down
14 changes: 5 additions & 9 deletions tracing-subscriber/src/registry/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ mod tests {
// registry.
let dispatch = dispatcher::Dispatch::new(subscriber);

dispatcher::with_default(&dispatch, || {
dispatcher::with_default(dispatch, || {
let span = tracing::debug_span!("span1");
drop(span);
let span = tracing::info_span!("span2");
Expand All @@ -604,8 +604,6 @@ mod tests {
state.assert_removed("span1");
state.assert_removed("span2");

// Ensure the registry itself outlives the span.
drop(dispatch);
}

#[test]
Expand All @@ -622,7 +620,7 @@ mod tests {
// registry.
let dispatch = dispatcher::Dispatch::new(subscriber);

let span2 = dispatcher::with_default(&dispatch, || {
let span2 = dispatcher::with_default(dispatch, || {
let span = tracing::debug_span!("span1");
drop(span);
let span2 = tracing::info_span!("span2");
Expand All @@ -637,8 +635,6 @@ mod tests {
drop(span2);
state.assert_removed("span1");

// Ensure the registry itself outlives the span.
drop(dispatch);
}

#[test]
Expand All @@ -655,7 +651,7 @@ mod tests {
// registry.
let dispatch = dispatcher::Dispatch::new(subscriber);

dispatcher::with_default(&dispatch, || {
dispatcher::with_default(dispatch, || {
let span1 = tracing::debug_span!("span1");
let span2 = tracing::info_span!("span2");

Expand Down Expand Up @@ -688,7 +684,7 @@ mod tests {

let dispatch = dispatcher::Dispatch::new(subscriber);

dispatcher::with_default(&dispatch, || {
dispatcher::with_default(dispatch, || {
let span1 = tracing::info_span!("parent");
let span2 = tracing::info_span!(parent: &span1, "child");

Expand All @@ -715,7 +711,7 @@ mod tests {

let dispatch = dispatcher::Dispatch::new(subscriber);

dispatcher::with_default(&dispatch, || {
dispatcher::with_default(dispatch, || {
let span1 = tracing::info_span!("grandparent");
let span2 = tracing::info_span!(parent: &span1, "parent");
let span3 = tracing::info_span!(parent: &span2, "child");
Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ where
#[cfg(feature = "tracing-log")]
let _ = tracing_log::LogTracer::init();

dispatcher::set_default(&self.into())
dispatcher::set_default(self.into())
}

/// Attempts to set `self` as the [global default subscriber] in the current
Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/tests/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn reload_handle() {

let subscriber = tracing_core::dispatcher::Dispatch::new(layer.with_subscriber(NopSubscriber));

tracing_core::dispatcher::with_default(&subscriber, || {
tracing_core::dispatcher::with_default(subscriber, || {
assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 0);
assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 0);

Expand Down
2 changes: 1 addition & 1 deletion tracing/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
//! // no default subscriber
//!
//! # #[cfg(feature = "std")]
//! dispatcher::with_default(&my_dispatch, || {
//! dispatcher::with_default(my_dispatch, || {
//! // my_subscriber is the default
//! });
//!
Expand Down
4 changes: 2 additions & 2 deletions tracing/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn with_default<T, S>(subscriber: S, f: impl FnOnce() -> T) -> T
where
S: Subscriber + Send + Sync + 'static,
{
crate::dispatcher::with_default(&crate::Dispatch::new(subscriber), f)
crate::dispatcher::with_default(crate::Dispatch::new(subscriber), f)
}

/// Sets this subscriber as the global default for the duration of the entire program.
Expand Down Expand Up @@ -62,7 +62,7 @@ pub fn set_default<S>(subscriber: S) -> DefaultGuard
where
S: Subscriber + Send + Sync + 'static,
{
crate::dispatcher::set_default(&crate::Dispatch::new(subscriber))
crate::dispatcher::set_default(crate::Dispatch::new(subscriber))
}

pub use tracing_core::dispatcher::SetGlobalDefaultError;
4 changes: 2 additions & 2 deletions tracing/tests/multiple_max_level_hints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ fn multiple_max_level_hints() {

let dispatch1 = tracing::Dispatch::new(subscriber1);

tracing::dispatcher::with_default(&dispatch1, do_events);
tracing::dispatcher::with_default(dispatch1, do_events);
handle1.assert_finished();

let dispatch2 = tracing::Dispatch::new(subscriber2);
tracing::dispatcher::with_default(&dispatch2, do_events);
tracing::dispatcher::with_default(dispatch2, do_events);
handle2.assert_finished();
}

0 comments on commit c2fa02e

Please sign in to comment.