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

tracing: fix macro hygiene for concat! #1918

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

teohhanhui
Copy link

Re-application of changes made in #1842 which were lost in f1cf1f1

Integration tests added for regression.


Motivation

In my library I define a macro_rules! concat macro, i.e.
callbag::concat.

When I try to call tracing::info!(...), I get error output such as
this:

error output
> RUSTFLAGS='-Z macro-backtrace' cargo +nightly clippy --features trace
    Checking callbag v0.14.0 (/home/teohhanhui/projects/teohhanhui/callbag-rs)
error[E0308]: mismatched types
  --> src/concat.rs:89:9
   |
89 |         info!("from sink: {message:?}");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `u32`

error[E0277]: the trait bound `std::sync::Arc<core::Callbag<never::Never, _>>: std::convert::From<&str>` is not satisfied
    --> src/concat.rs:58:9
     |
56   | / macro_rules! concat {
57   | |     ($($s:expr),* $(,)?) => {
58   | |         $crate::concat(::std::vec![$($s),*].into_boxed_slice())
     | |         ^^^^^^^^^^^^^^ the trait `std::convert::From<&str>` is not implemented for `std::sync::Arc<core::Callbag<never::Never, _>>`
59   | |     };
60   | | }
     | |_- in this expansion of `concat!` (https://github.com/tokio-rs/tracing/issues/5)
...
89   |           info!("from sink: {message:?}");
     |           ------------------------------- in this macro invocation (https://github.com/tokio-rs/tracing/pull/1)
     |
    ::: src/utils/tracing.rs:47:1
     |
47   | / macro_rules! info {
48   | |     ($($arg:tt)+) => {
49   | |         ::cfg_if::cfg_if! {
50   | |             if #[cfg(feature = "trace")] {
51   | |                 ::tracing::info!($($arg)+)
     | |                 -------------------------- in this macro invocation (https://github.com/tokio-rs/tracing/pull/2)
...    |
54   | |     };
55   | | }
     | |_- in this expansion of `info!` (https://github.com/tokio-rs/tracing/pull/1)
     |
    ::: /home/teohhanhui/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1
     |
586  |   macro_rules! event {
     |  _-
     | |_|
     | |
587  | |     (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> (
588  | |         $crate::__tracing_log!(
589  | |             target: $target,
...    |
644  |                   name: concat!(
     |  _______________________-
645  |                       "event ",
646  |                       file!(),
647  |                       ":",
648  |                       line!()
649  | |                 ),
     | |_________________- in this macro invocation (https://github.com/tokio-rs/tracing/issues/5)
...
667  | /         $crate::event!(
668  |               target: $target,
669  |               $lvl,
670  |               { message = format_args!($($arg)+), $($fields)* }
671  | |         )
     | |_________- in this macro invocation (https://github.com/tokio-rs/tracing/issues/4)
...
791  | |     );
792  | | }
     | | -
     | |_|
     | |_in this expansion of `$crate::event!` (https://github.com/tokio-rs/tracing/pull/3)
     |   in this expansion of `$crate::event!` (https://github.com/tokio-rs/tracing/issues/4)
...
1229 | / macro_rules! info {
1230 |        (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => (
1231 |           $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*)
1232 |       );
...
1398 | /         $crate::event!(
1399 | |             target: module_path!(),
1400 | |             $crate::Level::INFO,
1401 | |             {},
1402 | |             $($arg)+
1403 | |         )
     | |_________- in this macro invocation (https://github.com/tokio-rs/tracing/pull/3)
1404 |       );
1405 | | }
     | |_- in this expansion of `::tracing::info!` (https://github.com/tokio-rs/tracing/pull/2)
     |
     = help: the following implementations were found:
               <std::sync::Arc<B> as std::convert::From<std::borrow::Cow<'a, B>>>
               <std::sync::Arc<T> as std::convert::From<T>>
               <std::sync::Arc<T> as std::convert::From<std::boxed::Box<T>>>
               <std::sync::Arc<[T]> as std::convert::From<&[T]>>
             and 9 others
     = note: required because of the requirements on the impl of `std::convert::Into<std::sync::Arc<core::Callbag<never::Never, _>>>` for `&str`
note: required by a bound in `concat::concat`
    --> src/concat.rs:81:8
     |
72   | pub fn concat<
     |        ------ required by a bound in this
...
81   |     S: Into<Arc<Source<T>>> + Send + Sync,
     |        ^^^^^^^^^^^^^^^^^^^^ required by this bound in `concat::concat`

error[E0308]: mismatched types
    --> src/concat.rs:58:9
     |
56   | / macro_rules! concat {
57   | |     ($($s:expr),* $(,)?) => {
58   | |         $crate::concat(::std::vec![$($s),*].into_boxed_slice())
     | |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found struct `core::Callbag`
59   | |     };
60   | | }
     | |_- in this expansion of `concat!` (https://github.com/tokio-rs/tracing/issues/5)
...
89   |           info!("from sink: {message:?}");
     |           ------------------------------- in this macro invocation (https://github.com/tokio-rs/tracing/pull/1)
     |
    ::: src/utils/tracing.rs:47:1
     |
47   | / macro_rules! info {
48   | |     ($($arg:tt)+) => {
49   | |         ::cfg_if::cfg_if! {
50   | |             if #[cfg(feature = "trace")] {
51   | |                 ::tracing::info!($($arg)+)
     | |                 -------------------------- in this macro invocation (https://github.com/tokio-rs/tracing/pull/2)
...    |
54   | |     };
55   | | }
     | |_- in this expansion of `info!` (https://github.com/tokio-rs/tracing/pull/1)
     |
    ::: /home/teohhanhui/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1
     |
586  |   macro_rules! event {
     |  _-
     | |_|
     | |
587  | |     (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> (
588  | |         $crate::__tracing_log!(
589  | |             target: $target,
...    |
644  |                   name: concat!(
     |  _______________________-
645  |                       "event ",
646  |                       file!(),
647  |                       ":",
648  |                       line!()
649  | |                 ),
     | |_________________- in this macro invocation (https://github.com/tokio-rs/tracing/issues/5)
...
667  | /         $crate::event!(
668  |               target: $target,
669  |               $lvl,
670  |               { message = format_args!($($arg)+), $($fields)* }
671  | |         )
     | |_________- in this macro invocation (https://github.com/tokio-rs/tracing/issues/4)
...
791  | |     );
792  | | }
     | | -
     | |_|
     | |_in this expansion of `$crate::event!` (https://github.com/tokio-rs/tracing/pull/3)
     |   in this expansion of `$crate::event!` (https://github.com/tokio-rs/tracing/issues/4)
...
1229 | / macro_rules! info {
1230 |        (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => (
1231 |           $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*)
1232 |       );
...
1398 | /         $crate::event!(
1399 | |             target: module_path!(),
1400 | |             $crate::Level::INFO,
1401 | |             {},
1402 | |             $($arg)+
1403 | |         )
     | |_________- in this macro invocation (https://github.com/tokio-rs/tracing/pull/3)
1404 |       );
1405 | | }
     | |_- in this expansion of `::tracing::info!` (https://github.com/tokio-rs/tracing/pull/2)
     |
     = note: expected reference `&'static str`
                   found struct `core::Callbag<never::Never, _>`

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `callbag` due to 3 previous errors

This is because of my concat macro being in scope.

Solution

Change all the tracing macros to use the re-export of core::concat!
in the __macro_support module, rather than using an un-namespaced
concat!. The re-export ensures that everything still works even in a
crate that redefines the core name to something else.

@teohhanhui teohhanhui requested review from hawkw and a team as code owners February 9, 2022 20:54
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, thanks for the tests too!

@hawkw
Copy link
Member

hawkw commented Feb 9, 2022

CI failure is due to a regression in the latest nightly (rust-lang/rust#93821), nothing we can do about it, so I'm going to go ahead and merge this.

@hawkw hawkw merged commit 3e5318d into tokio-rs:v0.1.x Feb 9, 2022
@teohhanhui teohhanhui deleted the fix/macro-hygiene-again branch February 10, 2022 08:14
hawkw added a commit that referenced this pull request Feb 17, 2022
# 0.1.31 (February 17th, 2022)

This release increases the minimum supported Rust version (MSRV) to
1.49.0. In addition, it fixes some relatively rare macro bugs.

### Added

- Added `tracing-forest` to the list of related crates ([#1935])

### Changed

- Updated minimum supported Rust version (MSRV) to 1.49.0 ([#1913])

### Fixed

- Fixed the `warn!` macro incorrectly generating an event with the
  `TRACE` level ([#1930])
- Fixed macro hygiene issues when used in a crate that defines its own
  `concat!` macro, for real this time ([#1918])

Thanks to @QnnOkabayashi, @nicolaasg, and @teohhanhui for contributing
to this release!

[#1935]: #1935
[#1913]: #1913
[#1930]: #1930
[#1918]: #1918
hawkw added a commit that referenced this pull request Feb 17, 2022
# 0.1.31 (February 17th, 2022)

This release increases the minimum supported Rust version (MSRV) to
1.49.0. In addition, it fixes some relatively rare macro bugs.

### Added

- Added `tracing-forest` to the list of related crates ([#1935])

### Changed

- Updated minimum supported Rust version (MSRV) to 1.49.0 ([#1913])

### Fixed

- Fixed the `warn!` macro incorrectly generating an event with the
  `TRACE` level ([#1930])
- Fixed macro hygiene issues when used in a crate that defines its own
  `concat!` macro, for real this time ([#1918])

Thanks to @QnnOkabayashi, @nicolaasg, and @teohhanhui for contributing
to this release!

[#1935]: #1935
[#1913]: #1913
[#1930]: #1930
[#1918]: #1918
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
Re-application of changes made in tokio-rs#1842 which were lost in f1cf1f1

Integration tests added for regression.

---

## Motivation

In my library I define a `macro_rules! concat` macro, i.e.
[`callbag::concat`](https://docs.rs/callbag/latest/callbag/macro.concat.html).

When I try to call `tracing::info!(...)`, I get error output such as
this:

<details>
<summary>error output</summary>

<!-- leave a blank line above -->
```
> RUSTFLAGS='-Z macro-backtrace' cargo +nightly clippy --features trace
    Checking callbag v0.14.0 (/home/teohhanhui/projects/teohhanhui/callbag-rs)
error[E0308]: mismatched types
  --> src/concat.rs:89:9
   |
89 |         info!("from sink: {message:?}");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `u32`

error[E0277]: the trait bound `std::sync::Arc<core::Callbag<never::Never, _>>: std::convert::From<&str>` is not satisfied
    --> src/concat.rs:58:9
     |
56   | / macro_rules! concat {
57   | |     ($($s:expr),* $(,)?) => {
58   | |         $crate::concat(::std::vec![$($s),*].into_boxed_slice())
     | |         ^^^^^^^^^^^^^^ the trait `std::convert::From<&str>` is not implemented for `std::sync::Arc<core::Callbag<never::Never, _>>`
59   | |     };
60   | | }
     | |_- in this expansion of `concat!` (tokio-rs#5)
...
89   |           info!("from sink: {message:?}");
     |           ------------------------------- in this macro invocation (tokio-rs#1)
     |
    ::: src/utils/tracing.rs:47:1
     |
47   | / macro_rules! info {
48   | |     ($($arg:tt)+) => {
49   | |         ::cfg_if::cfg_if! {
50   | |             if #[cfg(feature = "trace")] {
51   | |                 ::tracing::info!($($arg)+)
     | |                 -------------------------- in this macro invocation (tokio-rs#2)
...    |
54   | |     };
55   | | }
     | |_- in this expansion of `info!` (tokio-rs#1)
     |
    ::: /home/teohhanhui/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1
     |
586  |   macro_rules! event {
     |  _-
     | |_|
     | |
587  | |     (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> (
588  | |         $crate::__tracing_log!(
589  | |             target: $target,
...    |
644  |                   name: concat!(
     |  _______________________-
645  |                       "event ",
646  |                       file!(),
647  |                       ":",
648  |                       line!()
649  | |                 ),
     | |_________________- in this macro invocation (tokio-rs#5)
...
667  | /         $crate::event!(
668  |               target: $target,
669  |               $lvl,
670  |               { message = format_args!($($arg)+), $($fields)* }
671  | |         )
     | |_________- in this macro invocation (tokio-rs#4)
...
791  | |     );
792  | | }
     | | -
     | |_|
     | |_in this expansion of `$crate::event!` (tokio-rs#3)
     |   in this expansion of `$crate::event!` (tokio-rs#4)
...
1229 | / macro_rules! info {
1230 |        (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => (
1231 |           $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*)
1232 |       );
...
1398 | /         $crate::event!(
1399 | |             target: module_path!(),
1400 | |             $crate::Level::INFO,
1401 | |             {},
1402 | |             $($arg)+
1403 | |         )
     | |_________- in this macro invocation (tokio-rs#3)
1404 |       );
1405 | | }
     | |_- in this expansion of `::tracing::info!` (tokio-rs#2)
     |
     = help: the following implementations were found:
               <std::sync::Arc<B> as std::convert::From<std::borrow::Cow<'a, B>>>
               <std::sync::Arc<T> as std::convert::From<T>>
               <std::sync::Arc<T> as std::convert::From<std::boxed::Box<T>>>
               <std::sync::Arc<[T]> as std::convert::From<&[T]>>
             and 9 others
     = note: required because of the requirements on the impl of `std::convert::Into<std::sync::Arc<core::Callbag<never::Never, _>>>` for `&str`
note: required by a bound in `concat::concat`
    --> src/concat.rs:81:8
     |
72   | pub fn concat<
     |        ------ required by a bound in this
...
81   |     S: Into<Arc<Source<T>>> + Send + Sync,
     |        ^^^^^^^^^^^^^^^^^^^^ required by this bound in `concat::concat`

error[E0308]: mismatched types
    --> src/concat.rs:58:9
     |
56   | / macro_rules! concat {
57   | |     ($($s:expr),* $(,)?) => {
58   | |         $crate::concat(::std::vec![$($s),*].into_boxed_slice())
     | |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found struct `core::Callbag`
59   | |     };
60   | | }
     | |_- in this expansion of `concat!` (tokio-rs#5)
...
89   |           info!("from sink: {message:?}");
     |           ------------------------------- in this macro invocation (tokio-rs#1)
     |
    ::: src/utils/tracing.rs:47:1
     |
47   | / macro_rules! info {
48   | |     ($($arg:tt)+) => {
49   | |         ::cfg_if::cfg_if! {
50   | |             if #[cfg(feature = "trace")] {
51   | |                 ::tracing::info!($($arg)+)
     | |                 -------------------------- in this macro invocation (tokio-rs#2)
...    |
54   | |     };
55   | | }
     | |_- in this expansion of `info!` (tokio-rs#1)
     |
    ::: /home/teohhanhui/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/tracing-0.1.29/src/macros.rs:586:1
     |
586  |   macro_rules! event {
     |  _-
     | |_|
     | |
587  | |     (target: $target:expr, parent: $parent:expr, $lvl:expr, { $($fields:tt)* } )=> (
588  | |         $crate::__tracing_log!(
589  | |             target: $target,
...    |
644  |                   name: concat!(
     |  _______________________-
645  |                       "event ",
646  |                       file!(),
647  |                       ":",
648  |                       line!()
649  | |                 ),
     | |_________________- in this macro invocation (tokio-rs#5)
...
667  | /         $crate::event!(
668  |               target: $target,
669  |               $lvl,
670  |               { message = format_args!($($arg)+), $($fields)* }
671  | |         )
     | |_________- in this macro invocation (tokio-rs#4)
...
791  | |     );
792  | | }
     | | -
     | |_|
     | |_in this expansion of `$crate::event!` (tokio-rs#3)
     |   in this expansion of `$crate::event!` (tokio-rs#4)
...
1229 | / macro_rules! info {
1230 |        (target: $target:expr, parent: $parent:expr, { $($field:tt)* }, $($arg:tt)* ) => (
1231 |           $crate::event!(target: $target, parent: $parent, $crate::Level::INFO, { $($field)* }, $($arg)*)
1232 |       );
...
1398 | /         $crate::event!(
1399 | |             target: module_path!(),
1400 | |             $crate::Level::INFO,
1401 | |             {},
1402 | |             $($arg)+
1403 | |         )
     | |_________- in this macro invocation (tokio-rs#3)
1404 |       );
1405 | | }
     | |_- in this expansion of `::tracing::info!` (tokio-rs#2)
     |
     = note: expected reference `&'static str`
                   found struct `core::Callbag<never::Never, _>`

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `callbag` due to 3 previous errors
```
</details>

This is because of my `concat` macro being in scope.

## Solution

Change all the `tracing` macros to use the re-export of `core::concat!`
in the `__macro_support` module, rather than using an un-namespaced
`concat!`. The re-export ensures that everything still works even in a
crate that redefines the `core` name to something else.
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.1.31 (February 17th, 2022)

This release increases the minimum supported Rust version (MSRV) to
1.49.0. In addition, it fixes some relatively rare macro bugs.

### Added

- Added `tracing-forest` to the list of related crates ([tokio-rs#1935])

### Changed

- Updated minimum supported Rust version (MSRV) to 1.49.0 ([tokio-rs#1913])

### Fixed

- Fixed the `warn!` macro incorrectly generating an event with the
  `TRACE` level ([tokio-rs#1930])
- Fixed macro hygiene issues when used in a crate that defines its own
  `concat!` macro, for real this time ([tokio-rs#1918])

Thanks to @QnnOkabayashi, @nicolaasg, and @teohhanhui for contributing
to this release!

[tokio-rs#1935]: tokio-rs#1935
[tokio-rs#1913]: tokio-rs#1913
[tokio-rs#1930]: tokio-rs#1930
[tokio-rs#1918]: tokio-rs#1918
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

Successfully merging this pull request may close these issues.

2 participants