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

macros use fields more than once #1739

Closed
Skepfyr opened this issue Nov 21, 2021 · 2 comments
Closed

macros use fields more than once #1739

Skepfyr opened this issue Nov 21, 2021 · 2 comments
Labels
crate/tracing Related to the `tracing` crate kind/bug Something isn't working

Comments

@Skepfyr
Copy link
Contributor

Skepfyr commented Nov 21, 2021

Bug Report

Some tracing macros can use the fields provided more than once causing confusing and non-local error messages.

Version

Discovered on 0.1.29, from a code read also looks to be present on master.

Crates

tracing

Description

#[derive(Debug)]
struct Foo;

#[derive(Debug)]
struct Bar(Foo);

pub fn foo(foo: Foo) {
    tracing::error!("{:?}", Bar(foo))
}

when the above code is compiled with the log feature enabled, you get the following compiler error:

error[E0382]: use of moved value: `foo`
 --> src/lib.rs:8:33
  |
7 | pub fn foo(foo: Foo) {
  |            --- move occurs because `foo` has type `Foo`, which does not implement the `Copy` trait
8 |     tracing::error!("{:?}", Bar(foo))
  |                                 ^^^
  |                                 |
  |                                 value moved here
  |                                 value used here after move

For more information about this error, try `rustc --explain E0382`.

(You can also extend this to weird and wonderful effects, such as a counter that depends on the feature flags and log level.)

This error is confusing by itself, but is made worse by the fact that it can be triggered indirectly; this (A) compiles fine without the log feature enabled, but if a crate (B) depends on A and (even indirectly) enables the log feature then A will fail to compile as part of B, but will compile fine by itself.

From a quick code read I think there may be other macros affected by this, but I haven't investigated any further.

@hawkw hawkw added crate/tracing Related to the `tracing` crate kind/bug Something isn't working labels Nov 21, 2021
@hawkw
Copy link
Member

hawkw commented Nov 21, 2021

Yeah, this has --- unfortunately --- been a known issue for quite a while now (see #196). I'd really, really like to fix this, but I wasn't able to find a good solution last time I spent time looking into this.

If you're interested in helping out, a fresh pair of eyes could be useful! I'm happy to answer any questions if you do take a look at it.

@Skepfyr
Copy link
Contributor Author

Skepfyr commented Nov 22, 2021

Staring at this I agree it's a pain. My only thought at the moment would be to use the ValueSet to produce the log. I think ValueSet has all the information needed to produce the formatted log string, the only change would be to always construct it and pass it to log rather than just when that trace level is enabled.

Skepfyr added a commit to Skepfyr/tracing that referenced this issue Jan 9, 2022
This cheanges the event macro (and all the upstream macros like info!),
so that it only uses each field once when the log feature is enabled.
Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once. This change generates the log using the
`ValueSet` meaning the token trees are only substituted once.

Fixes: tokio-rs#196, tokio-rs#1739
Skepfyr added a commit to Skepfyr/tracing that referenced this issue Jan 9, 2022
This changes the event macro (and all the upstream macros like info!),
so that it only uses each field once when the log feature is enabled.
Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once. This change generates the log using the
`ValueSet` meaning the token trees are only substituted once.

Fixes: tokio-rs#196, tokio-rs#1739
hawkw pushed a commit that referenced this issue Jan 29, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once. 


## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
hawkw pushed a commit that referenced this issue Feb 3, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
hawkw pushed a commit that referenced this issue Feb 3, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
hawkw pushed a commit that referenced this issue Feb 3, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
hawkw pushed a commit that referenced this issue Feb 3, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
hawkw pushed a commit that referenced this issue Feb 4, 2022
## Motivation

Fixes: #196, #1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
@Skepfyr Skepfyr closed this as completed Mar 19, 2022
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
## Motivation

Fixes: tokio-rs#196, tokio-rs#1739

Previously, the `field` token trees would be substituted into the log
macro invocation and the `ValueSet`, potentially meaning they are
executed more than once.

## Solution

This changes the `event!` macro (and all the upstream macros like `info!`),
so that it only uses each field once when the log feature is enabled. It
does this by generating the log using the `ValueSet` meaning the token
trees are only substituted once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/tracing Related to the `tracing` crate kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants