attributes: fix #[instrument(err)] with mutable parameters

[okready]

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.

[hawkw]

this looks good to me, thanks for fixing this!

[hawkw]

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 cargo expand output, and yet it still triggers the lint. maybe @Manishearth can figure this out.

[okready]

Thanks @hawkw for looking into the issue. Sorry for not catching it originally!

I've managed to track down the clippy::unit_arg cause to the Ok match arms on the function's result value. It seems that if the result has a unit Ok value, Clippy will complain in the match statement because (in most cases) you don't need to specify a named variable for the unit value (e.g. Ok(x)). I have a fix that should be ready to push shortly.

[hawkw]

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.

[hawkw]

Thanks @hawkw for looking into the issue. Sorry for not catching it originally!

I've managed to track down the clippy::unit_arg cause to the Ok match arms on the function's result value. It seems that if the result has a unit Ok value, Clippy will complain in the match statement because (in most cases) you don't need to specify a named variable for the unit value (e.g. Ok(x)). I have a fix that should be ready to push shortly.

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.

[okready]

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 SergioBenitez/Rocket#1495). let's 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 Result<(), E>, which Clippy can see is always a unit value in the Ok arms, so the attribute may still be necessary even after the bug is fixed (outside of generating different Ok match arm code for unit types).

[hawkw]

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 SergioBenitez/Rocket#1495). let's 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 Result<(), E>, which Clippy can see is always a unit value in the Ok arms, so the attribute may still be necessary even after the bug is fixed (outside of generating different Ok match arm code for unit types).

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.