tracing: support bare display and debug in macros

tracing-attributes/tests/fields.rs

@@ -77,6 +77,33 @@ impl HasField {
     fn self_expr_field(&self) {}
 }
 
+struct Disp {
+    bar: u64,
+}
+
+impl ::std::fmt::Display for Disp {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+        ::std::write!(f, "Disp.bar={bar}", bar = self.bar)
+    }
+}
+
+#[derive(Debug)]
+struct Deb {
+    _val: u64,
+}
+
+#[instrument(fields(foo = display(Disp { bar: 6, })))]
+fn fn_bare_display() {}
+
+#[instrument(skip_all, fields(foo = display(param)))]
+fn fn_bare_display_param(param: Disp) {}
+
+#[instrument(fields(foo = debug(Deb { _val: 100, })))]
+fn fn_bare_debug() {}
+
+#[instrument(skip_all, fields(foo = debug(param)))]
+fn fn_bare_debug_param(param: Deb) {}
+
 #[test]
 fn fields() {
     let span = expect::span().with_fields(
@@ -236,6 +263,52 @@ fn clashy_const_field_name() {
     });
 }
 
+#[test]
+fn bare_display() {
+    use tracing::field::display;
+
+    let span = expect::span().with_fields(expect::field("foo").with_value(&display("Disp.bar=6")));
+
+    run_test(span, || {
+        fn_bare_display();
+    });
+}
+
+#[test]
+fn bare_display_param() {
+    use tracing::field::display;
+
+    let span = expect::span().with_fields(expect::field("foo").with_value(&display("Disp.bar=6")));
+
+    run_test(span, || {
+        fn_bare_display_param(Disp { bar: 6 });
+    });
+}
+
+#[test]
+fn bare_debug() {
+    use tracing::field::debug;
+
+    let span =
+        expect::span().with_fields(expect::field("foo").with_value(&debug(Deb { _val: 100 })));
+
+    run_test(span, || {
+        fn_bare_debug();
+    });
+}
+
+#[test]
+fn bare_debug_param() {
+    use tracing::field::debug;
+
+    let span =
+        expect::span().with_fields(expect::field("foo").with_value(&debug(Deb { _val: 100 })));
+
+    run_test(span, || {
+        fn_bare_debug_param(Deb { _val: 100 });
+    });
+}
+
 fn run_test<F: FnOnce() -> T, T>(span: NewSpan, fun: F) {
     let (subscriber, handle) = subscriber::mock()
         .new_span(span)

tracing/Cargo.toml

@@ -35,6 +35,8 @@ criterion = { version = "0.3.6", default-features = false }
 futures = { version = "0.3.21", default-features = false }
 log = "0.4.17"
 tracing-mock = { path = "../tracing-mock" }
+trybuild = "1.0.64"
+rustversion = "1.0.9"
 
 [target.'cfg(target_arch = "wasm32")'.dev-dependencies]
 wasm-bindgen-test = "0.3.38"

tracing/src/macros.rs

@@ -2831,12 +2831,24 @@ macro_rules! valueset {
             $($rest)*
         )
     };
+    (@ { $(,)* $($out:expr),* }, $($k:ident).+ = debug($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $($k:ident).+ = %$val:expr, $($rest:tt)*) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
             $($rest)*
         )
     };
+    (@ { $(,)* $($out:expr),* }, $($k:ident).+ = display($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $($k:ident).+ = $val:expr, $($rest:tt)*) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
@@ -2866,11 +2878,21 @@ macro_rules! valueset {
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
         )
     };
+    (@ { $(,)* $($out:expr),* }, $($k:ident).+ = debug($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $($k:ident).+ = %$val:expr) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
         )
     };
+    (@ { $(,)* $($out:expr),* }, $($k:ident).+ = display($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $($k:ident).+ = $val:expr) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
@@ -2899,12 +2921,24 @@ macro_rules! valueset {
             $($rest)*
         )
     };
+    (@ { $(,)* $($out:expr),* }, $k:literal = debug($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $k:literal = %$val:expr, $($rest:tt)*) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
             $($rest)*
         )
     };
+    (@ { $(,)* $($out:expr),* }, $k:literal = display($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $k:literal = $val:expr, $($rest:tt)*) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
@@ -2916,11 +2950,21 @@ macro_rules! valueset {
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
         )
     };
+    (@ { $(,)* $($out:expr),* }, $k:literal = debug($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $k:literal = %$val:expr) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
         )
     };
+    (@ { $(,)* $($out:expr),* }, $k:literal = display($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
     (@ { $(,)* $($out:expr),* }, $k:literal = $val:expr) => {
         $crate::valueset!(
             @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
@@ -2930,35 +2974,57 @@ macro_rules! valueset {
     // Handle constant names
     (@ { $(,)* $($out:expr),* }, { $k:expr } = ?$val:expr, $($rest:tt)*) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
+    (@ { $(,)* $($out:expr),* }, { $k:expr } = debug($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
             $($rest)*
         )
     };
     (@ { $(,)* $($out:expr),* }, { $k:expr } = %$val:expr, $($rest:tt)*) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+            $($rest)*
+        )
+    };
+    (@ { $(,)* $($out:expr),* }, { $k:expr } = display($val:expr), $($rest:tt)*) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
             $($rest)*
         )
     };
     (@ { $(,)* $($out:expr),* }, { $k:expr } = $val:expr, $($rest:tt)*) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$val as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
             $($rest)*
         )
     };
     (@ { $(,)* $($out:expr),* }, { $k:expr } = ?$val:expr) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
+    (@ { $(,)* $($out:expr),* }, { $k:expr } = debug($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) },
         )
     };
     (@ { $(,)* $($out:expr),* }, { $k:expr } = %$val:expr) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
+        )
+    };
+    (@ { $(,)* $($out:expr),* }, { $k:expr } = display($val:expr)) => {
+        $crate::valueset!(
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) },
         )
     };
     (@ { $(,)* $($out:expr),* }, { $k:expr } = $val:expr) => {
         $crate::valueset!(
-            @ { $($out),*, (Some(&$val as &dyn $crate::field::Value)) },
+            @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) },
         )
     };
 
@@ -2973,7 +3039,10 @@ macro_rules! valueset {
             #[allow(unused_imports)]
             // This import statement CANNOT be removed as it will break existing use cases.
             // See #831, #2332, #3424 for the last times we tried.
-            use $crate::field::{debug, display, Value};
+            // It previously contained `display` and `debug` as well, but those have been
+            // moved into the macro patterns instead, still no idea how to remove `Value`
+            // without breaking though.
+            use $crate::field::Value;
             $fields.value_set_all($crate::valueset!(
                 @ { },
                 $($kvs)+

tracing/tests/macros.rs

@@ -20,6 +20,21 @@ impl ::std::fmt::Display for DisplayDebug {
     }
 }
 
+struct Disp {
+    bar: u64,
+}
+
+impl ::std::fmt::Display for Disp {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+        ::std::write!(f, "Disp.bar={bar}", bar = self.bar)
+    }
+}
+
+#[derive(Debug)]
+struct Deb {
+    _val: u64,
+}
+
 // Tests that macros work across various invocation syntax.
 //
 // These are quite repetitive, and _could_ be generated by a macro. However,
@@ -1398,6 +1413,91 @@ fn borrow_val_spans() {
     zero.push_str("hello world");
 }
 
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+#[test]
+fn bare_display_debug() {
+    error!(foo = display("foo"));
+    error!(foo = display(Disp { bar: 4 }));
+    error!(quux = debug(Deb { _val: 1024 }));
+    error!(quux = debug(4_u8 as u64));
+
+    error!(mog = 5, foo = display("foo"));
+    error!(mog = 5, foo = display(Disp { bar: 4 }));
+    error!(mog = 5, quux = debug(Deb { _val: 1024 }));
+    error!(mog = 5, quux = debug(4_u8 as u64));
+
+    error!(foo = display("foo"), mog = 5);
+    error!(foo = display(Disp { bar: 4 }), mog = 5);
+    error!(quux = debug(Deb { _val: 1024 }), mog = 5);
+    error!(quux = debug(4_u8 as u64), mog = 5);
+
+    error!({ foo = display("foo") }, "gom = {:?}", 8);
+    error!({ foo = display(Disp { bar: 4 }) }, "gom = {:?}", 8);
+    error!({ quux = debug(Deb { _val: 1024 }) }, "gom = {:?}", 8);
+    error!({ quux = debug(4_u8 as u64) }, "gom = {:?}", 8);
+
+    error!("foo" = display("foo"));
+    error!("foo" = display(Disp { bar: 4 }));
+    error!("quux" = debug(Deb { _val: 1024 }));
+    error!("quux" = debug(4_u8 as u64));
+
+    error!(mog = 5, "foo" = display("foo"));
+    error!(mog = 5, "foo" = display(Disp { bar: 4 }));
+    error!(mog = 5, "quux" = debug(Deb { _val: 1024 }));
+    error!(mog = 5, "quux" = debug(4_u8 as u64));
+
+    error!("foo" = display("foo"), mog = 5);
+    error!("foo" = display(Disp { bar: 4 }), mog = 5);
+    error!("quux" = debug(Deb { _val: 1024 }), mog = 5);
+    error!("quux" = debug(4_u8 as u64), mog = 5);
+
+    error!({ "foo" = display("foo") }, "gom = {:?}", 8);
+    error!({ "foo" = display(Disp { bar: 4 }) }, "gom = {:?}", 8);
+    error!({ "quux" = debug(Deb { _val: 1024 }) }, "gom = {:?}", 8);
+    error!({ "quux" = debug(4_u8 as u64) }, "gom = {:?}", 8);
+
+    const FOO: &str = "FOO";
+    const QUUX: &str = "QUUX";
+    error!({ FOO } = display("foo"));
+    error!({ FOO } = display(Disp { bar: 4 }));
+    error!({ QUUX } = debug(Deb { _val: 1024 }));
+    error!({ QUUX } = debug(4_u8 as u64));
+
+    error!(mog = 5, { FOO } = display("foo"));
+    error!(mog = 5, { FOO } = display(Disp { bar: 4 }));
+    error!(mog = 5, { QUUX } = debug(Deb { _val: 1024 }));
+    error!(mog = 5, { QUUX } = debug(4_u8 as u64));
+
+    error!({ FOO } = display("foo"), mog = 5);
+    error!({ FOO } = display(Disp { bar: 4 }), mog = 5);
+    error!({ QUUX } = debug(Deb { _val: 1024 }), mog = 5);
+    error!({ QUUX } = debug(4_u8 as u64), mog = 5);
+
+    error!({ {FOO} = display("foo") }, "gom = {:?}", 8);
+    error!({ {FOO} = display(Disp { bar: 4, }) }, "gom = {:?}", 8);
+    error!({ {QUUX} = debug(Deb { _val: 1024, }) }, "gom = {:?}", 8);
+    error!({ {QUUX} = debug(4_u8 as u64) }, "gom = {:?}", 8);
+
+    let display = Disp { bar: 4 };
+    let debug = Deb { _val: 96 };
+    error!("{debug:?} {display}");
+    error!(%display, ?debug);
+    error!(display = %display, debug = ?debug);
+
+    let display = "str also implements Value";
+    let debug = "str also implements Value";
+    error!("{debug:?} {display}");
+    error!(display, debug);
+    error!(display = display, debug = debug);
+}
+
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+#[test]
+fn bare_value() {
+    let num = ::std::option::Option::Some(52);
+    error!(bar = &num as &dyn Value);
+}
+
 #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
 #[test]
 fn callsite_macro_api() {

tracing/tests/ui.rs

@@ -0,0 +1,8 @@
+// Only test on stable, since UI tests are bound to change over time
+
+#[rustversion::stable]
+#[test]
+fn compile_fail() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/ui/fail/*.rs");
+}

tracing/tests/ui/fail/debug_without_key.rs

@@ -0,0 +1,9 @@
+//! We support bare `debug` function in macros, but only when a key is
+//! specified (e.g. `foo = debug(foo)` is supported but just `debug(foo)`
+//! isn't).
+
+fn main() {
+    let foo = "foo";
+    tracing::info!(debug(foo));
+}
+