Add compile error on const fn instrumentation #2418

andrewpollack · 2022-12-19T03:18:02Z

I'm newer to using macro expansions to enforce compile_error! calls, and this is my interpretation on how to approach it. Unfortunately, the compile_error! is triggering regardless of whether I'm instrumenting a function or not, which makes me think I'm not approaching the code generation portion correctly.

Should I be forming this as a proc_macro instead of a macro_rules?

hawkw

The code you've written expands the compile_error! macro inside the implementation of the instrument proc macro, rather than including the compile_error! macro inside the output of the instrument proc macro. This means that the error occurs when compiling the tracing-attributes crate, because the compile_error! macro is present in its source.

In order for this to work correctly, the compile_error! macro needs to be generated in the instrument proc macro's output. Proc macros like instrument typically use the quote! macro to emit generated code. To illustrate this, note that whenever we use compile_error! elsewhere in tracing-attributes, it is always inside of quote!d code that will be output by the instrument macro:

tracing/tracing-attributes/src/expand.rs

Lines 168 to 170 in 02903cb

    
           return quote_spanned! {skip.span()=> 
        
               compile_error!("attempting to skip non-existent parameter") 
        
           };

You can fix this by changing your branch to return a quote!d compile_error! when encountering a const fn.

Hope that helps, and please let me know if you have any additional questions!

andrewpollack · 2022-12-20T01:07:29Z

@hawkw your explanation was fantastic and makes tons of sense, thank you so much! 😸

I've pushed the compile_error update, along with an additional doc test and a couple tests to tracing-attributes/tests/ui (I wasn't sure if these were the best places to put those updates, so I'm happy to update if there are better locations!)

hawkw

overall, this looks good to me! i had a small suggestion regarding the wording of the error message.

tracing-attributes/src/lib.rs

Co-authored-by: Eliza Weisman <eliza@buoyant.io>

hawkw

this looks great to me, and i'll merge it as soon as CI passes! thanks for working on this!

## Motivation The `#[instrument]` macro cannot be used on `const fn`s, because the generated code will perform runtime tracing behavior. However, when adding the attribute to a `const fn`, the compiler errors generated currently are somewhat unclear (see #2414). It would be better if we generated a less verbose error that simply states that `#[instrument]` is not supported on `const fn`s. ## Solution This branch changes the `#[instrument]` macro to detect when the annotated function is a `const fn`, and emit a simpler, more descritpive error message. The new error simply states that the `#[instrument]` attribute cannot be used on `const fn`s, and should be much less confusing to the user. Fixes #2414

@nitnelave

# 0.1.24 (April 24th, 2023) This release of `tracing-attributes` adds support for passing an optional `level` to the `err` and `ret` arguments to `#[instrument]`, allowing the level of the generated return-value event to be overridden. For example, ```rust #[instrument(err(level = "info"))] fn my_great_function() -> Result<(), &'static str> { // ... } ``` will emit an `INFO`-level event if the function returns an `Err`. In addition, this release updates the [`syn`] dependency to v2.x.x. ### Added - `level` argument to `err` and `ret` to override the level of the generated return value event (#2335) - Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - Updated `syn` dependency to 2.0 (#2516) ### Fixed - Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - Documented default level for `err` (#2433) - Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, and @dbidwell94 for contributign to this release! [`syn`]: https://crates.io/crates/syn

@nitnelave

# 0.1.24 (April 24th, 2023) This release of `tracing-attributes` adds support for passing an optional `level` to the `err` and `ret` arguments to `#[instrument]`, allowing the level of the generated return-value event to be overridden. For example, ```rust #[instrument(err(level = "info"))] fn my_great_function() -> Result<(), &'static str> { // ... } ``` will emit an `INFO`-level event if the function returns an `Err`. In addition, this release updates the [`syn`] dependency to v2.x.x. ### Added - `level` argument to `err` and `ret` to override the level of the generated return value event (#2335) - Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - Updated `syn` dependency to 2.0 (#2516) ### Fixed - Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - Documented default level for `err` (#2433) - Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, and @dbidwell94 for contributign to this release! [`syn`]: https://crates.io/crates/syn

@nitnelave

# 0.1.38 (April 25th, 2023) This `tracing` release changes the `Drop` implementation for `Instrumented` `Future`s so that the attached `Span` is entered when dropping the `Future`. This means that events emitted by the `Future`'s `Drop` implementation will now be recorded within its `Span`. It also adds `#[inline]` hints to methods called in the `event!` macro's expansion, for an improvement in both binary size and performance. Additionally, this release updates the `tracing-attributes` dependency to [v0.1.24][attrs-0.1.24], which updates the [`syn`] dependency to v2.x.x. `tracing-attributes` v0.1.24 also includes improvements to the `#[instrument]` macro; see [the `tracing-attributes` 0.1.24 release notes][attrs-0.1.24] for details. ### Added - `Instrumented` futures will now enter the attached `Span` in their `Drop` implementation, allowing events emitted when dropping the future to occur within the span (#2562) - `#[inline]` attributes for methods called by the `event!` macros, making generated code smaller (#2555) - **attributes**: `level` argument to `#[instrument(err)]` and `#[instrument(ret)]` to override the level of the generated return value event (#2335) - **attributes**: Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - `tracing-attributes`: updated to [0.1.24][attrs-0.1.24] - Removed unneeded `cfg-if` dependency (#2553) - **attributes**: Updated [`syn`] dependency to 2.0 (#2516) ### Fixed - **attributes**: Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - **attributes**: Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - **attributes**: Documented default level for `#[instrument(err)]` (#2433) - **attributes**: Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, @dbidwell94, @ldm0, @NobodyXu, @ilsv, and @daxpedda for contributing to this release! [`syn`]: https://crates.io/crates/syn [attrs-0.1.24]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.24

@nitnelave

# 0.1.38 (April 25th, 2023) This `tracing` release changes the `Drop` implementation for `Instrumented` `Future`s so that the attached `Span` is entered when dropping the `Future`. This means that events emitted by the `Future`'s `Drop` implementation will now be recorded within its `Span`. It also adds `#[inline]` hints to methods called in the `event!` macro's expansion, for an improvement in both binary size and performance. Additionally, this release updates the `tracing-attributes` dependency to [v0.1.24][attrs-0.1.24], which updates the [`syn`] dependency to v2.x.x. `tracing-attributes` v0.1.24 also includes improvements to the `#[instrument]` macro; see [the `tracing-attributes` 0.1.24 release notes][attrs-0.1.24] for details. ### Added - `Instrumented` futures will now enter the attached `Span` in their `Drop` implementation, allowing events emitted when dropping the future to occur within the span (#2562) - `#[inline]` attributes for methods called by the `event!` macros, making generated code smaller (#2555) - **attributes**: `level` argument to `#[instrument(err)]` and `#[instrument(ret)]` to override the level of the generated return value event (#2335) - **attributes**: Improved compiler error message when `#[instrument]` is added to a `const fn` (#2418) ### Changed - `tracing-attributes`: updated to [0.1.24][attrs-0.1.24] - Removed unneeded `cfg-if` dependency (#2553) - **attributes**: Updated [`syn`] dependency to 2.0 (#2516) ### Fixed - **attributes**: Fix `clippy::unreachable` warnings in `#[instrument]`-generated code (#2356) - **attributes**: Removed unused "visit" feature flag from `syn` dependency (#2530) ### Documented - **attributes**: Documented default level for `#[instrument(err)]` (#2433) - **attributes**: Improved documentation for levels in `#[instrument]` (#2350) Thanks to @nitnelave, @jsgf, @Abhicodes-crypto, @LukeMathWalker, @andrewpollack, @quad, @klensy, @davidpdrsn, @dbidwell94, @ldm0, @NobodyXu, @ilsv, and @daxpedda for contributing to this release! [`syn`]: https://crates.io/crates/syn [attrs-0.1.24]: https://github.com/tokio-rs/tracing/releases/tag/tracing-attributes-0.1.24

hawkw reviewed Dec 19, 2022

View reviewed changes

Add compile error on const fn instrumentation

0e3399a

andrewpollack force-pushed the add-compile-error branch from 2790681 to 0e3399a Compare December 20, 2022 01:03

andrewpollack marked this pull request as ready for review December 20, 2022 01:07

andrewpollack requested review from davidbarsky and a team as code owners December 20, 2022 01:07

andrewpollack requested review from hawkw and removed request for davidbarsky December 20, 2022 04:59

hawkw approved these changes Dec 29, 2022

View reviewed changes

tracing-attributes/src/lib.rs Outdated Show resolved Hide resolved

andrewpollack and others added 4 commits December 29, 2022 15:59

Update tracing-attributes/src/lib.rs

006ed05

Co-authored-by: Eliza Weisman <eliza@buoyant.io>

Merge branch 'master' into add-compile-error

4e6a539

Update const_instrument.stderr

f377ae0

Update const_instrument.stderr

8225b90

andrewpollack requested a review from hawkw December 31, 2022 01:25

hawkw enabled auto-merge (squash) December 31, 2022 19:53

hawkw approved these changes Dec 31, 2022

View reviewed changes

hawkw merged commit 58accc6 into tokio-rs:master Dec 31, 2022

hawkw mentioned this pull request Apr 21, 2023

backport a gigantic pile of stuff #2570

Merged

hawkw mentioned this pull request Apr 24, 2023

attributes: prepare to release v0.1.24 #2574

Merged

hawkw mentioned this pull request Apr 25, 2023

tracing: prepare to release v0.1.38 #2575

Merged

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add compile error on const fn instrumentation #2418

Add compile error on const fn instrumentation #2418

andrewpollack commented Dec 19, 2022 •

edited

hawkw left a comment •

edited

andrewpollack commented Dec 20, 2022

hawkw left a comment

hawkw left a comment

	return quote_spanned! {skip.span()=>
	compile_error!("attempting to skip non-existent parameter")
	};

Add compile error on const fn instrumentation #2418

Add compile error on const fn instrumentation #2418

Conversation

andrewpollack commented Dec 19, 2022 • edited

hawkw left a comment • edited

Choose a reason for hiding this comment

andrewpollack commented Dec 20, 2022

hawkw left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

andrewpollack commented Dec 19, 2022 •

edited

hawkw left a comment •

edited