tracing icon indicating copy to clipboard operation
tracing copied to clipboard

Fixed macro valueset to support debug and display named parameters

Open Wonshtrum opened this issue 3 years ago • 2 comments

Signed-off-by: Eloi DEMOLIS [email protected]

Motivation

Following issue #2278 I implemented and tested the fix mentionned.

Solution

I changed the valueset macro used (among others) by the span macro which is used internally by (notably) the instrument proc macro. The important change being at this line:

use $crate::field::Value;

which doesn't overrides debug and display anymore.

I'm new to this crate so I don't know the full implications of this, but for me it mainly fixed this use case:

#[tracing::instrument]
fn test(debug: bool, display: bool) {
    todo!();
}

Wonshtrum avatar Aug 17 '22 17:08 Wonshtrum

I didn't think of this. Thank you for the review and workaround.

Wonshtrum avatar Sep 13 '22 20:09 Wonshtrum

After encountering this issue, it's been rather frustrating hunting down why this didn't work (this was using a simple info!(debug) which would trigger an error similar to #2278).

Maybe valueset! could at least be updated with cases explicitly checking for the debug and display keys / values and triggering a compile_error! with a clear message?

valueset! is a bit much for my meagre understanding of declarative macros, but I think the terminal cases could have "guard" cases checking for the explicit debug/display symbols and triggering an error along the lines of

macro_rules! thing {
    (debug) => {
        std::compile_error!("the debug symbol can't be traced");
    };
    (display) => {
        std::compile_error!("the display symbol can't be traced");
    };
    ($v:ident) => {
        println!("{}", $v);
    }
}

I think even the expansion sites could support that for the $k:ident cases (including repetitions). $k:expr and $val:expr I don't see how to fix so the namespace clash would remain, but it could at least offer some level of clarity for the simpler use cases.

xmo-odoo avatar Jul 04 '24 09:07 xmo-odoo