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

bug: #[tracing::instrument] reorders async and unsafe keywords #2576

Closed
ParkMyCar opened this issue Apr 25, 2023 · 3 comments
Closed

bug: #[tracing::instrument] reorders async and unsafe keywords #2576

ParkMyCar opened this issue Apr 25, 2023 · 3 comments

Comments

@ParkMyCar
Copy link

Bug Report

Version

tracing v0.1.37

Platform

Darwin Parkers-MacBook-Pro.local 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar  6 20:59:58 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6020 arm64

Crates

tracing

Description

Currently you cannot use the #[tracing::instrument] macro to instrument an unsafe async function. The problem is the macro re-orders the async and unsafe keywords in the wrong order.

Minimum Repro:

use tracing;

#[tracing::instrument]
async unsafe fn foo() -> usize {
    42
}

Playground

This fails to compile with the error:

error: expected one of `extern` or `fn`, found keyword `async`
 --> src/lib.rs:4:1
  |
4 | async unsafe fn foo() -> usize {
  | ^^^^^-------
  | |
  | expected one of `extern` or `fn`
  | help: `async` must come before `unsafe`: `async unsafe`
  |
  = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

error: could not compile `playground` due to previous error

Using rust-analyzer you can see what this expands to:

unsafe async fn bar< >() -> usize {
  {}
  let __tracing_attr_span = tracing::span!(target:module_path!(),tracing::Level::INFO,"bar",);
  let __tracing_instrument_future = async move {
    #[allow(unreachable_code,clippy::diverging_sub_expression,clippy::let_unit_value,clippy::unreachable)]
    if false {
      let __tracing_attr_fake_return:usize = unreachable!("this is just for type inference, and is unreachable code");
      return __tracing_attr_fake_return;
    }{
      42
    }
  };
  if!__tracing_attr_span.is_disabled(){
    tracing::Instrument::instrument(__tracing_instrument_future,__tracing_attr_span).await
  }else {
    __tracing_instrument_future.await
  }
}

And indeed if you look at the first line of the expansion you can see unsafe async when it should be async unsafe.

kaffarell added a commit to kaffarell/tracing that referenced this issue Jan 26, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: tokio-rs#2576

Signed-off-by: Gabriel Goller <[email protected]>
kaffarell added a commit to kaffarell/tracing that referenced this issue Jan 26, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: tokio-rs#2576

Signed-off-by: Gabriel Goller <[email protected]>
hawkw pushed a commit that referenced this issue Jan 26, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: #2576

Signed-off-by: Gabriel Goller <[email protected]>
@kaffarell
Copy link
Contributor

kaffarell commented Jan 27, 2024

This issue was fixed with PR #2864. Don't know why it wasn't closed automatically :/ (@hawkw)

@hawkw
Copy link
Member

hawkw commented Jan 27, 2024

This issue was fixed with PR #2864. Don't know why it wasn't closed automatically :/ (@hawkw)

I think it's because the PR was against the v0.1.x branch. Issues only get closed automatically when merging to master, I think this is a GitHub behavior we don't get to control.

kaffarell added a commit to kaffarell/tracing that referenced this issue Feb 14, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: tokio-rs#2576

Signed-off-by: Gabriel Goller <[email protected]>
@kaffarell
Copy link
Contributor

kaffarell commented Feb 15, 2024

I think we can close this issue manually though @hawkw?

@hawkw hawkw closed this as completed Feb 15, 2024
kaffarell added a commit to kaffarell/tracing that referenced this issue May 22, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: tokio-rs#2576

Signed-off-by: Gabriel Goller <[email protected]>
davidbarsky pushed a commit that referenced this issue Oct 2, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: #2576

Signed-off-by: Gabriel Goller <[email protected]>
davidbarsky pushed a commit that referenced this issue Oct 2, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: #2576

Signed-off-by: Gabriel Goller <[email protected]>
davidbarsky pushed a commit that referenced this issue Oct 2, 2024
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.

Fixes: #2576

Signed-off-by: Gabriel Goller <[email protected]>
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

No branches or pull requests

3 participants