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

Figure out macro invocation syntax for the new release #218

Closed
sfackler opened this issue Aug 14, 2017 · 11 comments
Closed

Figure out macro invocation syntax for the new release #218

sfackler opened this issue Aug 14, 2017 · 11 comments

Comments

@sfackler
Copy link
Member

There are a couple of constraints here, IMO:

  • It must be possible to turn println!(...) into debug!(...) with no changes to the inner bit.
  • It must be possible to optionally specify a target string.
  • It must be possible to optionally specify a set of parameters.
    • We may or may not actually support this in the initial release, but I want to make sure the macro syntax will work out for it.

The current syntax is as println with an optional target: $expr, at the start to specify the target. I don't really think that sticking things before the formatting part is going to work for parameters, so we'll need some other way of working with that.

One possibility is that string interpolation isn't supported when providing parameters, since all of the dynamic info should be provided in the parameters. Not sure how people would feel about that though.

cc @alexcrichton @dtolnay @dpc

@alexcrichton
Copy link
Member

Is there a downside to the current syntax today? If so mind listing that out?

@sfackler
Copy link
Member Author

The primary concern is how to fit parameters in. The matcher for the format arguments has to be (I think?) $($t:tt)* so it has to be the last thing in the macro, at least at its level of paren nesting. Right now the only other thing we have is the target so it's either info!("foo: {}", foo) or info!(target: "some_target", "foo: {}", foo). Once we add parameters though, this seems pretty gross: info!(target: "some_target", params: { foo: foo, bar: "hello", }, "foo: {}", foo).

@alexcrichton
Copy link
Member

Oh right yes sorry! I think I misunderstood.

@dtolnay
Copy link
Member

dtolnay commented Aug 14, 2017

Not proposing this as a solution, but it's perfectly possible to parse format args that are not the end of a level of nesting.

macro_rules! info {
    // public
    (target: $t:expr, $fmt:expr) => {
        info!([$fmt]);
    };
    (target: $t:expr, $fmt:expr, $($rest:tt)*) => {
        info!([$fmt] $($rest)*);
    };

    // private
    ([$($fmt:tt)*]) => {
        println!($($fmt)*);
    };
    ([$($fmt:tt)*] params: { $($k:ident : $v:expr),* }) => {
        println!($($fmt)*);
        $(
            println!("params[{}] = {:?}", stringify!($k), $v);
        )*
    };
    ([$($fmt:tt)*] params: { $($k:ident : $v:expr,)* }) => {
        info!([$($fmt)*] params: { $($k : $v),* });
    };
    ([$($fmt:tt)*] $n:ident = $v:expr) => {
        info!([$($fmt)*, $n = $v]);
    };
    ([$($fmt:tt)*] $n:ident = $v:expr, $($rest:tt)*) => {
        info!([$($fmt)*, $n = $v] $($rest)*);
    };
    ([$($fmt:tt)*] $v:expr) => {
        info!([$($fmt)*, $v]);
    };
    ([$($fmt:tt)*] $v:expr, $($rest:tt)*) => {
        info!([$($fmt)*, $v] $($rest)*);
    };
}

fn main() {
    let foo = 0;
    info!(target: "some_target", "foo: {}", foo, params: { foo: foo, bar: "hello" });
}

@sfackler
Copy link
Member Author

Wow, you are a magician! I'm guessing we can move target after the format args via the same technique as well - that could be a pretty good syntax.

@alexcrichton
Copy link
Member

Nice! That actually seems like a pretty sweet API to me

@dpc
Copy link

dpc commented Aug 23, 2017

It seems to me that additional params: and { ... } makes the structured logging a bit more tedious. I really liked the syntax proposed here: #149 (comment)

Pasted for conviniance:

info!("{} {msg}", "Hello", msg="world!"; k: v);

The main point being that the first part (until ;) behaves like println!, with named arguments being included as key-values for the the structured logging part itself.

@sfackler
Copy link
Member Author

I believe this is the last blocker on an 0.4 release. Are people okay with the syntax as it exists today? It seems like whether we go for params { ... } or ; ... they should both be extensions of the current syntax, right?

@alexcrichton
Copy link
Member

Sounds plausible to me!

@KodrAus
Copy link
Contributor

KodrAus commented Oct 2, 2017

It looks like the case. Are we happy to close this now or would you like to keep it open for a while longer?

@sfackler
Copy link
Member Author

sfackler commented Oct 2, 2017

I think we're good to go.

@sfackler sfackler closed this as completed Oct 2, 2017
EFanZh pushed a commit to EFanZh/log that referenced this issue Jul 23, 2023
Use `opt-level = "z"` for release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants