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

attributes: remove unnecessary quote_spanned! #1617

Merged
merged 3 commits into from
Oct 5, 2021
Merged

attributes: remove unnecessary quote_spanned! #1617

merged 3 commits into from
Oct 5, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 5, 2021

Motivation

Apparently, using quote_spanned! can trigger a Clippy bug where the
text else, even inside a comment, may cause the
suspicious_else_formatting lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the #[instrument] attribute is
used on async fns. See issue #1613 for details.

Solution

It turns out that some of the uses of quote_spanned! in the
tracing-attributes code generation are not needed. We really only need
quote_spanned! when actually interpolating the user provided code into
a block, not in the tracing-attributes code that inserts the generated
code for producing the span etc. Replacing some of these
quote_spanned! uses with the normal quote! macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.

I've added a few test cases that should reproduce the bug.

Fixes #1613

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation

Apparently, using `quote_spanned!` can trigger a Clippy bug where the
text `else`, even inside a comment, _may_ cause the
`suspicious_else_formatting` lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the `#[instrument]` attribute is
used on `async fn`s. See issue #1613 for details.

## Solution

It turns out that some of the uses of `quote_spanned!` in the
`tracing-attributes` code generation are not needed. We really only need
`quote_spanned!` when actually interpolating the user provided code into
a block, not in the `tracing-attributes` code that inserts the generated
code for producing the span etc. Replacing some of these
`quote_spanned!` uses with the normal `quote!` macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.

I've added a few test cases that should reproduce the bug.

Fixes #1613
@hawkw hawkw self-assigned this Oct 5, 2021
@hawkw hawkw requested review from davidbarsky and a team as code owners October 5, 2021 18:01
@hawkw hawkw merged commit 2f6fcb0 into master Oct 5, 2021
@hawkw hawkw deleted the eliza/fix-1613 branch October 5, 2021 19:59
hawkw added a commit that referenced this pull request Oct 5, 2021
## Motivation

Apparently, using `quote_spanned!` can trigger a Clippy bug where the
text `else`, even inside a comment, _may_ cause the
`suspicious_else_formatting` lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the `#[instrument]` attribute is
used on `async fn`s. See issue #1613 for details.

## Solution

It turns out that some of the uses of `quote_spanned!` in the
`tracing-attributes` code generation are not needed. We really only need
`quote_spanned!` when actually interpolating the user provided code into
a block, not in the `tracing-attributes` code that inserts the generated
code for producing the span etc. Replacing some of these
`quote_spanned!` uses with the normal `quote!` macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.

I've added a few test cases that should reproduce the bug.

Fixes #1613

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 5, 2021
## Motivation

Apparently, using `quote_spanned!` can trigger a Clippy bug where the
text `else`, even inside a comment, _may_ cause the
`suspicious_else_formatting` lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the `#[instrument]` attribute is
used on `async fn`s. See issue #1613 for details.

## Solution

It turns out that some of the uses of `quote_spanned!` in the
`tracing-attributes` code generation are not needed. We really only need
`quote_spanned!` when actually interpolating the user provided code into
a block, not in the `tracing-attributes` code that inserts the generated
code for producing the span etc. Replacing some of these
`quote_spanned!` uses with the normal `quote!` macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.

I've added a few test cases that should reproduce the bug.

Fixes #1613

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.17 (October 1, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.17 (October 1, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.18 (October 5, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([#1614])

[#1617]: #1617
[#1616]: #1616
[#1614]: #1614
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Oct 6, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1605]: #1605
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.1.18 (October 5, 2021)

This release fixes issues introduced in v0.1.17.

### Fixed

- fixed mismatched types compiler error that may occur when using
  `#[instrument]` on an `async fn` that returns an `impl Trait` value
  that includes a closure ([tokio-rs#1616])
- fixed false positives for `clippy::suspicious_else_formatting`
  warnings due to rust-lang/rust-clippy#7760 and
  rust-lang/rust-clippy#6249 ([tokio-rs#1617])
- fixed `clippy::let_unit_value` lints when using `#[instrument]`
  ([tokio-rs#1614])

[tokio-rs#1617]: tokio-rs#1617
[tokio-rs#1616]: tokio-rs#1616
[tokio-rs#1614]: tokio-rs#1614
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([tokio-rs#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([tokio-rs#1600], [tokio-rs#1605],
  [tokio-rs#1614], [tokio-rs#1616], [tokio-rs#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([tokio-rs#1602])
- Documentation fixes and improvements ([tokio-rs#1595], [tokio-rs#1601], [tokio-rs#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[tokio-rs#1585]: tokio-rs#1585
[tokio-rs#1595]: tokio-rs#1596
[tokio-rs#1597]: tokio-rs#1597
[tokio-rs#1600]: tokio-rs#1600
[tokio-rs#1601]: tokio-rs#1601
[tokio-rs#1602]: tokio-rs#1602
[tokio-rs#1605]: tokio-rs#1605
[tokio-rs#1614]: tokio-rs#1614
[tokio-rs#1616]: tokio-rs#1616
[tokio-rs#1617]: tokio-rs#1617
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.

tracing::instrument triggers clippy warning
1 participant