attributes: fix #[instrument(err)] with mutable parameters #1167
Conversation
## Motivation The closure generated by `#[instrument(err)]` for non-async functions is not mutable, preventing any captured variables from being mutated. If a function has any mutable parameters that are modified within the function, adding the `#[instrument(err)]` attribute will result in a compiler error. ## Solution This change makes it so the closure is executed directly as a temporary variable instead of storing it in a named variable prior to its use, avoiding the need to explicitly declare it as mutable altogether or to add an `#[allow(unused_mut)]` annotation on the closure declaration to silence lint warnings for closures that do not need to be mutable.
The clippy::unit_arg warning is triggered for the Ok branch in the match statements generated for the #[instrument(err)] attribute against the function result if that function returns a unit Ok value. This commit disables the unit_arg lint check for the Ok match arms and cleans up other attempts to isolate the lint error.
|
okay i've spent like an hour trying to figure out the Clippy lint and i just do not get it --- as far as i can tell, the pattern the lint fires on no longer appears anywhere in the |
|
Thanks @hawkw for looking into the issue. Sorry for not catching it originally! I've managed to track down the |
|
ah, okay, looks like it's just a clippy bug: rust-lang/rust-clippy#6447 (which looks like it trips on proc-macros a lot, see also rwf2/Rocket#1495). let's just add an allow attribute. |
great, i'm glad you figured it out --- if you have a fix, that's fantastic. otherwise, i think it would be fine to just add an allow attribute. |
I think in this case the lint may technically be correct, as the generated code for the tests is explicitly matching against a |
yeah, that makes sense. it looks like you've pushed the fix for this, thanks so much for figuring it out! i'm going to go ahead and merge this now. |
Motivation ---------- The closure generated by `#[instrument(err)]` for non-async functions is not mutable, preventing any captured variables from being mutated. If a function has any mutable parameters that are modified within the function, adding the `#[instrument(err)]` attribute will result in a compiler error. Solution -------- This change makes it so the closure is executed directly as a temporary variable instead of storing it in a named variable prior to its use, avoiding the need to explicitly declare it as mutable altogether or to add an `#[allow(unused_mut)]` annotation on the closure declaration to silence lint warnings for closures that do not need to be mutable.
Motivation ---------- The closure generated by `#[instrument(err)]` for non-async functions is not mutable, preventing any captured variables from being mutated. If a function has any mutable parameters that are modified within the function, adding the `#[instrument(err)]` attribute will result in a compiler error. Solution -------- This change makes it so the closure is executed directly as a temporary variable instead of storing it in a named variable prior to its use, avoiding the need to explicitly declare it as mutable altogether or to add an `#[allow(unused_mut)]` annotation on the closure declaration to silence lint warnings for closures that do not need to be mutable.
Fixed - Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([#1167]) - Missing function visibility modifier when using `#[instrument]` with `async-trait` ([#977]) - Multiple documentation fixes and improvements ([#965], [#981], [#1215]) Changed - `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([#808]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, and @okready for contributing to this release! [#1167]: #1167 [#977]: #977 [#965]: #965 [#981]: #981 [#1215]: #1215 [#808]: #808
Fixed - Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([#1167]) - Missing function visibility modifier when using `#[instrument]` with `async-trait` ([#977]) - Multiple documentation fixes and improvements ([#965], [#981], [#1215]) Changed - `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([#808]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, and @okready for contributing to this release! [#1167]: #1167 [#977]: #977 [#965]: #965 [#981]: #981 [#1215]: #1215 [#808]: #808
### Fixed - Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([#1167]) - Missing function visibility modifier when using `#[instrument]` with `async-trait` ([#977]) - Multiple documentation fixes and improvements ([#965], [#981], [#1215]) ### Changed - `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([#808]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, and @okready for contributing to this release! [#1167]: #1167 [#977]: #977 [#965]: #965 [#981]: #981 [#1215]: #1215 [#808]: #808
Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([#1167]) - **attributes**: Missing function visibility modifier when using `#[instrument]` with `async-trait` ([#977]) - **attributes** Removed unused `syn` features ([#928]) - **log**: Fixed an issue where the `tracing` macros would generate code for events whose levels are disabled statically by the `log` crate's `static_max_level_XXX` features ([#1175]) - Fixed deprecations and clippy lints ([#1195]) - Several documentation fixes and improvements ([#941], [#965], [#981], [#1146], [#1215]) Changed - **attributes**: `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([#808]) - **attributes**: Updated `tracing-attributes` minimum dependency to v0.1.12 ([#1222]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, @okready, and @krojew for contributing to this release!
### Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([#1167]) - **attributes**: Missing function visibility modifier when using `#[instrument]` with `async-trait` ([#977]) - **attributes** Removed unused `syn` features ([#928]) - **log**: Fixed an issue where the `tracing` macros would generate code for events whose levels are disabled statically by the `log` crate's `static_max_level_XXX` features ([#1175]) - Fixed deprecations and clippy lints ([#1195]) - Several documentation fixes and improvements ([#941], [#965], [#981], [#1146], [#1215]) ### Changed - **attributes**: `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([#808]) - **attributes**: Updated `tracing-attributes` minimum dependency to v0.1.12 ([#1222]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, @okready, and @krojew for contributing to this release!
Motivation
The closure generated by
#[instrument(err)]for non-async functions isnot mutable, preventing any captured variables from being mutated. If a
function has any mutable parameters that are modified within the
function, adding the
#[instrument(err)]attribute will result in acompiler error.
Solution
This change makes it so the closure is executed directly as a temporary
variable instead of storing it in a named variable prior to its use,
avoiding the need to explicitly declare it as mutable altogether or to
add an
#[allow(unused_mut)]annotation on the closure declaration tosilence lint warnings for closures that do not need to be mutable.