-
Notifications
You must be signed in to change notification settings - Fork 727
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
chore: fix deprecation and clippy warnings #1195
Conversation
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
// if `compare_exchange` returns Result::Ok(_), then `new` has been set and | ||
// `current`—now the prior value—has been returned in the `Ok()` branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO explaining what compare_exchange
does doesn't seem super necessary to me, but it's not a blocker...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd rather over-explain then under-explain here!
@davidbarsky do you have a sec to open a backport branch? |
This branch fixes two known warnings. The first fixed warning was in `tracing-core/src/callsite.rs`, where clippy suggested using `std::ptr::eq` to compare addresses instead of casting `&T` to a `*const T`, which `&T` coerces to _anyways_. Below is the warning: ``` error: use `std::ptr::eq` when comparing raw pointers --> tracing-core/src/callsite.rs:238:9 | 238 | self.0 as *const _ as *const () == other.0 as *const _ as *const () | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(self.0 as *const _, other.0 as *const _)` | = note: `-D clippy::ptr-eq` implied by `-D warnings` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq ``` The second fixed warning is a bit more complex. As of Rust 1.50, [`AtomicUsize::compare_and_swap`][1] has been deprecated in favor of [`AtomicUsize::compare_exchange`][2] and [`AtomicUsize::compare_exchange_weak`][3]. I've moved `tracing_core::dispatch::set_global_default` to use `AtomicUsize::compare_exchange` as `AtomicUsize::compare_exchange_weak` is designed for atomic loads in loops. Here are a few notes on the differences between `AtomicUsize::compare_and_swap` and `AtomicUsize::compare_exchange`: - `AtomicUsize::compare_exchange` returns a result, where the `Result::Ok(_)` branch contains the prior value. This is equivalent to the now-deprecated [`AtomicUsize::compare_and_swap`][1] returning the `current` parameter upon a successful compare-and-swap operation, but success is now established through a `Result`, not an equality operation. - `AtomicUsize::compare_exchange` accepts an `Ordering` for the failure case of a compare-and-swap. The [migration guide suggests][4] using `Ordering::SeqCst` for both the success and failure cases and I saw no reason to depart from its guidance. After this branch is approved, I'll backport these fixes to the `v0.1.0` branch. [1]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_and_swap [2]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange [3]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange_weak [4]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#migrating-to-compare_exchange-and-compare_exchange_weak
This branch fixes two known warnings. The first fixed warning was in `tracing-core/src/callsite.rs`, where clippy suggested using `std::ptr::eq` to compare addresses instead of casting `&T` to a `*const T`, which `&T` coerces to _anyways_. Below is the warning: ``` error: use `std::ptr::eq` when comparing raw pointers --> tracing-core/src/callsite.rs:238:9 | 238 | self.0 as *const _ as *const () == other.0 as *const _ as *const () | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(self.0 as *const _, other.0 as *const _)` | = note: `-D clippy::ptr-eq` implied by `-D warnings` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq ``` The second fixed warning is a bit more complex. As of Rust 1.50, [`AtomicUsize::compare_and_swap`][1] has been deprecated in favor of [`AtomicUsize::compare_exchange`][2] and [`AtomicUsize::compare_exchange_weak`][3]. I've moved `tracing_core::dispatch::set_global_default` to use `AtomicUsize::compare_exchange` as `AtomicUsize::compare_exchange_weak` is designed for atomic loads in loops. Here are a few notes on the differences between `AtomicUsize::compare_and_swap` and `AtomicUsize::compare_exchange`: - `AtomicUsize::compare_exchange` returns a result, where the `Result::Ok(_)` branch contains the prior value. This is equivalent to the now-deprecated [`AtomicUsize::compare_and_swap`][1] returning the `current` parameter upon a successful compare-and-swap operation, but success is now established through a `Result`, not an equality operation. - `AtomicUsize::compare_exchange` accepts an `Ordering` for the failure case of a compare-and-swap. The [migration guide suggests][4] using `Ordering::SeqCst` for both the success and failure cases and I saw no reason to depart from its guidance. After this branch is approved, I'll backport these fixes to the `v0.1.0` branch. [1]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_and_swap [2]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange [3]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange_weak [4]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#migrating-to-compare_exchange-and-compare_exchange_weak
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!
…1208) This branch fixes two known warnings. The first fixed warning was in `tracing-core/src/callsite.rs`, where clippy suggested using `std::ptr::eq` to compare addresses instead of casting `&T` to a `*const T`, which `&T` coerces to _anyways_. Below is the warning: ``` error: use `std::ptr::eq` when comparing raw pointers --> tracing-core/src/callsite.rs:238:9 | 238 | self.0 as *const _ as *const () == other.0 as *const _ as *const () | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(self.0 as *const _, other.0 as *const _)` | = note: `-D clippy::ptr-eq` implied by `-D warnings` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq ``` The second fixed warning is a bit more complex. As of Rust 1.50, [`AtomicUsize::compare_and_swap`][1] has been deprecated in favor of [`AtomicUsize::compare_exchange`][2] and [`AtomicUsize::compare_exchange_weak`][3]. I've moved `tracing_core::dispatch::set_global_default` to use `AtomicUsize::compare_exchange` as `AtomicUsize::compare_exchange_weak` is designed for atomic loads in loops. Here are a few notes on the differences between `AtomicUsize::compare_and_swap` and `AtomicUsize::compare_exchange`: - `AtomicUsize::compare_exchange` returns a result, where the `Result::Ok(_)` branch contains the prior value. This is equivalent to the now-deprecated [`AtomicUsize::compare_and_swap`][1] returning the `current` parameter upon a successful compare-and-swap operation, but success is now established through a `Result`, not an equality operation. - `AtomicUsize::compare_exchange` accepts an `Ordering` for the failure case of a compare-and-swap. The [migration guide suggests][4] using `Ordering::SeqCst` for both the success and failure cases and I saw no reason to depart from its guidance. After this branch is approved, I'll backport these fixes to the `v0.1.0` branch. [1]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_and_swap [2]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange [3]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange_weak [4]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#migrating-to-compare_exchange-and-compare_exchange_weak
### Fixed - **attributes**: Compiler error when using `#[instrument(err)]` on functions with mutable parameters ([tokio-rs#1167]) - **attributes**: Missing function visibility modifier when using `#[instrument]` with `async-trait` ([tokio-rs#977]) - **attributes** Removed unused `syn` features ([tokio-rs#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 ([tokio-rs#1175]) - Fixed deprecations and clippy lints ([tokio-rs#1195]) - Several documentation fixes and improvements ([tokio-rs#941], [tokio-rs#965], [tokio-rs#981], [tokio-rs#1146], [tokio-rs#1215]) ### Changed - **attributes**: `tracing-futures` dependency is no longer required when using `#[instrument]` on async functions ([tokio-rs#808]) - **attributes**: Updated `tracing-attributes` minimum dependency to v0.1.12 ([tokio-rs#1222]) Thanks to @nagisa, @Txuritan, @TaKO8Ki, @okready, and @krojew for contributing to this release!
This branch fixes two known warnings.
The first fixed warning was in
tracing-core/src/callsite.rs
, where clippy suggested usingstd::ptr::eq
to compare addresses instead of casting&T
to a*const T
, which&T
coerces to anyways. Below is the warning:The second fixed warning is a bit more complex. As of Rust 1.50,
AtomicUsize::compare_and_swap
has been deprecated in favor ofAtomicUsize::compare_exchange
andAtomicUsize::compare_exchange_weak
. I've movedtracing_core::dispatch::set_global_default
to useAtomicUsize::compare_exchange
asAtomicUsize::compare_exchange_weak
is designed for atomic loads in loops. Here are a few notes on the differences betweenAtomicUsize::compare_and_swap
andAtomicUsize::compare_exchange
:AtomicUsize::compare_exchange
returns a result, where theResult::Ok(_)
branch contains the prior value. This is equivalent to the now-deprecatedAtomicUsize::compare_and_swap
returning thecurrent
parameter upon a successful compare-and-swap operation, but success is now established through aResult
, not an equality operation.AtomicUsize::compare_exchange
accepts anOrdering
for the failure case of a compare-and-swap. The migration guide suggests usingOrdering::SeqCst
for both the success and failure cases and I saw no reason to depart from its guidance.After this branch is approved, I'll backport these fixes to the
v0.1.0
branch.