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

#[track_caller] on async fn's was previously a no-op but now produces a compiler error #104588

Closed
anp opened this issue Nov 18, 2022 · 4 comments · Fixed by #104741
Closed

#[track_caller] on async fn's was previously a no-op but now produces a compiler error #104588

anp opened this issue Nov 18, 2022 · 4 comments · Fixed by #104741
Assignees
Labels
C-bug Category: This is a bug. P-medium Medium priority regression-untriaged Untriaged performance or correctness regression. WG-async Working group: Async & await

Comments

@anp
Copy link
Member

anp commented Nov 18, 2022

Code

I tried this code:

#[track_caller]
async fn foo() {
    println!("{}", std::panic::Location::caller());
}


#[tokio::main]
async fn main() {
    foo().await;
}

I expected to see this happen: a successful build that prints src/main.rs:3:20 (i.e. the attribute is a no-op)

Instead, this happened:

Compiling playground v0.0.1 (/playground)
error[[E0658]](https://doc.rust-lang.org/nightly/error-index.html#E0658): `#[track_caller]` on closures is currently unstable
 --> src/main.rs:2:16
  |
2 | async fn foo() {}
  |                ^^
  |
  = note: [see issue #87417 <https://github.com/rust-lang/rust/issues/87417>](https://github.com/rust-lang/rust/issues/87417) for more information
  = help: [add `#![feature(closure_track_caller)]`](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#) to the crate attributes to enable

Version it worked on

It most recently worked on: 1.65.0 stable

Version with regression

playground's current nightly: 1.67.0-nightly (2022-11-17 83356b78c4ff3e7d84e9)

I think this compilation failure was introduced by #104219 which adds actual support for track_caller on async fn's, since it was previously a no-op.

TBH, in hindsight I think we probably should have disallowed track_caller on async fn's in the initial implementation so that we could leave room for unstable support in the future. However given that this wasn't considered, it seems like this may now be a stability regression.

@anp anp added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 18, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 18, 2022
@compiler-errors
Copy link
Member

compiler-errors commented Nov 18, 2022

cc: @bryangarza

TBH, in hindsight I think we probably should have disallowed track_caller on async fn's in the initial implementation so that we could leave room for unstable support in the future.

@anp, that's a good point. it's probably possible to make #[track_caller] a no-op + lint against the ungated usage as a warning, and gate this behavior behind something else (either closure_track_caller, or a new feature such as async_track_caller).

@anp
Copy link
Member Author

anp commented Nov 18, 2022

That makes a lot of sense as a way to resolve this. Having the ungated usage produce a warning seems like a great way to avoid confusion about a feature flag silently changing compiled behavior.

@compiler-errors compiler-errors added the WG-async Working group: Async & await label Nov 18, 2022
@bryangarza
Copy link
Contributor

cc: @bryangarza

TBH, in hindsight I think we probably should have disallowed track_caller on async fn's in the initial implementation so that we could leave room for unstable support in the future.

@anp, that's a good point. it's probably possible to make #[track_caller] a no-op + lint against the ungated usage as a warning, and gate this behavior behind something else (either closure_track_caller, or a new feature such as async_track_caller).

Sounds good, I can put up a PR that does this. I didn't realize that the annotation was already being used on async fn's in the wild

bryangarza added a commit to bryangarza/rust that referenced this issue Nov 22, 2022
This patch fixes a regression, in which `#[track_caller]`, which was
previously a no-op, was changed to actually turn on the behavior. This
should instead only happen behind the `closure_track_caller` feature
gate.

Also, add a warning for the user to understand how their code will
compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 23, 2022
bryangarza added a commit to bryangarza/rust that referenced this issue Dec 7, 2022
This patch fixes a regression, in which `#[track_caller]`, which was
previously a no-op, was changed to actually turn on the behavior. This
should instead only happen behind the `closure_track_caller` feature
gate.

Also, add a warning for the user to understand how their code will
compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
kpreid added a commit to kpreid/all-is-cubes that referenced this issue Dec 13, 2022
The attribute doesn't actually do anything on `async fn`s, and this may
become a warning in the future. See
<rust-lang/rust#104588>.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 22, 2022
…caller, r=compiler-errors

Switch `#[track_caller]` back to a no-op unless feature gate is enabled

This patch fixes a regression, in which `#[track_caller]`, which was previously a no-op, was changed to actually turn on the behavior. This should instead only happen behind the `closure_track_caller` feature gate.

Also, add a warning for the user to understand how their code will compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 22, 2022
…caller, r=compiler-errors

Switch `#[track_caller]` back to a no-op unless feature gate is enabled

This patch fixes a regression, in which `#[track_caller]`, which was previously a no-op, was changed to actually turn on the behavior. This should instead only happen behind the `closure_track_caller` feature gate.

Also, add a warning for the user to understand how their code will compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
@bors bors closed this as completed in 04926e0 Dec 22, 2022
MaciejWas pushed a commit to MaciejWas/rust that referenced this issue Dec 27, 2022
This patch fixes a regression, in which `#[track_caller]`, which was
previously a no-op, was changed to actually turn on the behavior. This
should instead only happen behind the `closure_track_caller` feature
gate.

Also, add a warning for the user to understand how their code will
compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
bryangarza added a commit to bryangarza/rust that referenced this issue Dec 27, 2022
This patch fixes a regression, in which `#[track_caller]`, which was
previously a no-op, was changed to actually turn on the behavior. This
should instead only happen behind the `closure_track_caller` feature
gate.

Also, add a warning for the user to understand how their code will
compile depending on the feature gate being turned on or not.

Fixes rust-lang#104588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-medium Medium priority regression-untriaged Untriaged performance or correctness regression. WG-async Working group: Async & await
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants