attributes: fix #[instrument(err)] with impl Trait return types
#1236
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…1233) ## Motivation Currently, using `#[instrument(err)]` on a function returning a `Result` with an `impl Trait` in it results in a compiler error. This is because we generate a type annotation on the closure in the function body that contains the user function's actual body, and `impl Trait` isn't allowed on types in local bindings, only on function parameters and return types. ## Solution This branch fixes the issue by simply removing the return type annotation from the closure. I've also added tests that break on master for functions returning `impl Trait`, both with and without the `err` argument. Fixes #1227 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Feb 17, 2021
hawkw
added a commit
that referenced
this pull request
Feb 17, 2021
Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions which return `impl Trait` ([#1236]) - Fixed broken match arms in event macros ([#1239]) - Documentation improvements ([#1232]) Thanks to @bkchr and @lfranke for contributing to this release! [#1236]: #1236 [#1239]: #1239 [#1232]: #1232 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Feb 17, 2021
### Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions which return `impl Trait` ([#1236]) - Fixed broken match arms in event macros ([#1239]) - Documentation improvements ([#1232]) Thanks to @bkchr and @lfranke for contributing to this release! [#1236]: #1236 [#1239]: #1239 [#1232]: #1232 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell
pushed a commit
to kaffarell/tracing
that referenced
this pull request
May 22, 2024
### Fixed - Compiler error when using `#[instrument(err)]` on functions which return `impl Trait` ([tokio-rs#1236]) [tokio-rs#1236]: tokio-rs#1236 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell
pushed a commit
to kaffarell/tracing
that referenced
this pull request
May 22, 2024
### Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions which return `impl Trait` ([tokio-rs#1236]) - Fixed broken match arms in event macros ([tokio-rs#1239]) - Documentation improvements ([tokio-rs#1232]) Thanks to @bkchr and @lfranke for contributing to this release! [tokio-rs#1236]: tokio-rs#1236 [tokio-rs#1239]: tokio-rs#1239 [tokio-rs#1232]: tokio-rs#1232 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This backports PR #1233 to v0.1.x. This isn't just a simple cherry-pick
because the new tests in that branch use the v0.2.x module names, so
that had to be fixed. Otherwise, though, it's the same change, and I'll go
ahead and merge it when CI passes, since it was approved on
master.Motivation
Currently, using
#[instrument(err)]on a function returning aResultwith an
impl Traitin it results in a compiler error. This is becausewe generate a type annotation on the closure in the function body that
contains the user function's actual body, and
impl Traitisn't allowedon types in local bindings, only on function parameters and return
types.
Solution
This branch fixes the issue by simply removing the return type
annotation from the closure. I've also added tests that break on master
for functions returning
impl Trait, both with and without theerrargument.
Fixes #1227
Signed-off-by: Eliza Weisman eliza@buoyant.io