From 04926e0534be77ed1b33602057d60d0bb14c1d66 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Tue, 22 Nov 2022 21:15:27 +0000 Subject: [PATCH 1/7] 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 #104588 --- .../locales/en-US/lint.ftl | 2 + compiler/rustc_lint/src/builtin.rs | 45 ++++++++++- .../issue-104588-no-op-panic-track-caller.rs | 78 +++++++++++++++++++ ...sue-104588-no-op-panic-track-caller.stderr | 12 +++ .../issue-104588-no-op-track-caller.rs | 9 +++ .../issue-104588-no-op-track-caller.stderr | 12 +++ 6 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs create mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr create mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs create mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 7e28f22c0ba..3980f9a2a7a 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -350,6 +350,8 @@ lint_builtin_mutable_transmutes = lint_builtin_unstable_features = unstable feature +lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled + lint_builtin_unreachable_pub = unreachable `pub` {$what} .suggestion = consider restricting its visibility .help = or consider exporting it for use by other crates diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index cd19e65b6fc..d7d91a12e65 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -25,6 +25,7 @@ types::{transparent_newtype_field, CItemKind}, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext, }; +use hir::IsAsync; use rustc_ast::attr; use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{FnCtxt, FnKind}; @@ -40,7 +41,10 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID}; -use rustc_hir::{ForeignItemKind, GenericParamKind, HirId, Node, PatKind, PredicateOrigin}; +use rustc_hir::intravisit::FnKind as HirFnKind; +use rustc_hir::{ + Body, FnDecl, ForeignItemKind, GenericParamKind, HirId, Node, PatKind, PredicateOrigin, +}; use rustc_index::vec::Idx; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::layout::{LayoutError, LayoutOf}; @@ -1370,6 +1374,45 @@ fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &ast::Attribute) { } } +declare_lint! { + /// `#[track_caller]` is a no-op without corresponding feature flag + UNGATED_ASYNC_FN_TRACK_CALLER, + Warn, + "enabling track_caller on an async fn is a no-op unless the closure_track_caller feature is enabled" +} + +declare_lint_pass!( + /// Explains corresponding feature flag must be enabled for the `#[track_caller] attribute to + /// do anything + UngatedAsyncFnTrackCaller => [UNGATED_ASYNC_FN_TRACK_CALLER] +); + +impl<'tcx> LateLintPass<'tcx> for UngatedAsyncFnTrackCaller { + fn check_fn( + &mut self, + cx: &LateContext<'_>, + fn_kind: HirFnKind<'_>, + _: &'tcx FnDecl<'_>, + _: &'tcx Body<'_>, + span: Span, + hir_id: HirId, + ) { + if let HirFnKind::ItemFn(_, _, _) = fn_kind && fn_kind.asyncness() == IsAsync::Async && !cx.tcx.features().closure_track_caller { + // Now, check if the function has the `#[track_caller]` attribute + let attrs = cx.tcx.hir().attrs(hir_id); + let maybe_track_caller = attrs.iter().find(|attr| attr.has_name(sym::track_caller)); + if let Some(attr) = maybe_track_caller { + cx.struct_span_lint( + UNGATED_ASYNC_FN_TRACK_CALLER, + span.with_hi(attr.span.hi()), + fluent::lint_ungated_async_fn_track_caller, + |lint| lint, + ); + } + } + } +} + declare_lint! { /// The `unreachable_pub` lint triggers for `pub` items not reachable from /// the crate root. diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs new file mode 100644 index 00000000000..5ef40408e26 --- /dev/null +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs @@ -0,0 +1,78 @@ +// run-pass +// edition:2021 +// needs-unwind + + +use std::future::Future; +use std::panic; +use std::sync::{Arc, Mutex}; +use std::task::{Context, Poll, Wake}; +use std::thread::{self, Thread}; + +/// A waker that wakes up the current thread when called. +struct ThreadWaker(Thread); + +impl Wake for ThreadWaker { + fn wake(self: Arc) { + self.0.unpark(); + } +} + +/// Run a future to completion on the current thread. +fn block_on(fut: impl Future) -> T { + // Pin the future so it can be polled. + let mut fut = Box::pin(fut); + + // Create a new context to be passed to the future. + let t = thread::current(); + let waker = Arc::new(ThreadWaker(t)).into(); + let mut cx = Context::from_waker(&waker); + + // Run the future to completion. + loop { + match fut.as_mut().poll(&mut cx) { + Poll::Ready(res) => return res, + Poll::Pending => thread::park(), + } + } +} + +async fn bar() { + panic!() +} + +async fn foo() { + bar().await +} + +#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled [ungated_async_fn_track_caller] +async fn bar_track_caller() { + panic!() +} + +async fn foo_track_caller() { + bar_track_caller().await +} + +fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { + let loc = Arc::new(Mutex::new(None)); + + let hook = panic::take_hook(); + { + let loc = loc.clone(); + panic::set_hook(Box::new(move |info| { + *loc.lock().unwrap() = info.location().map(|loc| loc.line()) + })); + } + panic::catch_unwind(f).unwrap_err(); + panic::set_hook(hook); + let x = loc.lock().unwrap().unwrap(); + x +} + +fn main() { + assert_eq!(panicked_at(|| block_on(foo())), 41); + // Since the `closure_track_caller` feature is not enabled, the + // `track_caller annotation does nothing. + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 50); +} diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr new file mode 100644 index 00000000000..5bfd9ed8490 --- /dev/null +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr @@ -0,0 +1,12 @@ +warning: `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled + --> $DIR/issue-104588-no-op-panic-track-caller.rs:48:16 + | +LL | #[track_caller] + | ________________^ +LL | | async fn bar_track_caller() { + | |_ + | + = note: `#[warn(ungated_async_fn_track_caller)]` on by default + +warning: 1 warning emitted + diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs new file mode 100644 index 00000000000..146d3c9ec53 --- /dev/null +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs @@ -0,0 +1,9 @@ +// check-pass +// edition:2021 + +#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled +async fn foo() {} + +fn main() { + foo(); +} diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr new file mode 100644 index 00000000000..bf66cc9ea90 --- /dev/null +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr @@ -0,0 +1,12 @@ +warning: `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled + --> $DIR/issue-104588-no-op-track-caller.rs:4:16 + | +LL | #[track_caller] + | ________________^ +LL | | async fn foo() {} + | |_ + | + = note: `#[warn(ungated_async_fn_track_caller)]` on by default + +warning: 1 warning emitted + From dc2c4ce578bd3bee824e1b30789cfa6cb777e059 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 24 Nov 2022 03:39:47 +0000 Subject: [PATCH 2/7] Update code based on PR comments This patch does the following: - Refactor some repeated lines into a single one - Split the `ungated_async_fn_caller` lint into multiple lines, and make one of those lines only print out on nightly - Use test revisions instead of copying an existing test --- .../locales/en-US/lint.ftl | 3 +- compiler/rustc_lint/src/builtin.rs | 12 ++- .../issue-104588-no-op-panic-track-caller.rs | 78 ------------------- ...sue-104588-no-op-panic-track-caller.stderr | 12 --- .../issue-104588-no-op-track-caller.rs | 2 +- .../issue-104588-no-op-track-caller.stderr | 12 +-- .../panic-track-caller.nofeat.stderr | 14 ++++ .../track-caller/panic-track-caller.rs | 24 ++++-- 8 files changed, 51 insertions(+), 106 deletions(-) delete mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs delete mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr create mode 100644 src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 3980f9a2a7a..a36a511bd4a 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -350,7 +350,8 @@ lint_builtin_mutable_transmutes = lint_builtin_unstable_features = unstable feature -lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled +lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op + .suggestion = enable this feature lint_builtin_unreachable_pub = unreachable `pub` {$what} .suggestion = consider restricting its visibility diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index d7d91a12e65..e90572cb238 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1404,10 +1404,16 @@ fn check_fn( if let Some(attr) = maybe_track_caller { cx.struct_span_lint( UNGATED_ASYNC_FN_TRACK_CALLER, - span.with_hi(attr.span.hi()), + attr.span, fluent::lint_ungated_async_fn_track_caller, - |lint| lint, - ); + |lint| { + lint.span_label(span, "this function will not propagate the caller location"); + if cx.tcx.sess.is_nightly_build() { + lint.span_suggestion(attr.span, fluent::suggestion, "#[closure_track_caller]", Applicability::MachineApplicable); + } + lint + } + ); } } } diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs deleted file mode 100644 index 5ef40408e26..00000000000 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.rs +++ /dev/null @@ -1,78 +0,0 @@ -// run-pass -// edition:2021 -// needs-unwind - - -use std::future::Future; -use std::panic; -use std::sync::{Arc, Mutex}; -use std::task::{Context, Poll, Wake}; -use std::thread::{self, Thread}; - -/// A waker that wakes up the current thread when called. -struct ThreadWaker(Thread); - -impl Wake for ThreadWaker { - fn wake(self: Arc) { - self.0.unpark(); - } -} - -/// Run a future to completion on the current thread. -fn block_on(fut: impl Future) -> T { - // Pin the future so it can be polled. - let mut fut = Box::pin(fut); - - // Create a new context to be passed to the future. - let t = thread::current(); - let waker = Arc::new(ThreadWaker(t)).into(); - let mut cx = Context::from_waker(&waker); - - // Run the future to completion. - loop { - match fut.as_mut().poll(&mut cx) { - Poll::Ready(res) => return res, - Poll::Pending => thread::park(), - } - } -} - -async fn bar() { - panic!() -} - -async fn foo() { - bar().await -} - -#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled [ungated_async_fn_track_caller] -async fn bar_track_caller() { - panic!() -} - -async fn foo_track_caller() { - bar_track_caller().await -} - -fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { - let loc = Arc::new(Mutex::new(None)); - - let hook = panic::take_hook(); - { - let loc = loc.clone(); - panic::set_hook(Box::new(move |info| { - *loc.lock().unwrap() = info.location().map(|loc| loc.line()) - })); - } - panic::catch_unwind(f).unwrap_err(); - panic::set_hook(hook); - let x = loc.lock().unwrap().unwrap(); - x -} - -fn main() { - assert_eq!(panicked_at(|| block_on(foo())), 41); - // Since the `closure_track_caller` feature is not enabled, the - // `track_caller annotation does nothing. - assert_eq!(panicked_at(|| block_on(foo_track_caller())), 50); -} diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr deleted file mode 100644 index 5bfd9ed8490..00000000000 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-panic-track-caller.stderr +++ /dev/null @@ -1,12 +0,0 @@ -warning: `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled - --> $DIR/issue-104588-no-op-panic-track-caller.rs:48:16 - | -LL | #[track_caller] - | ________________^ -LL | | async fn bar_track_caller() { - | |_ - | - = note: `#[warn(ungated_async_fn_track_caller)]` on by default - -warning: 1 warning emitted - diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs index 146d3c9ec53..6443e14296d 100644 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs @@ -1,7 +1,7 @@ // check-pass // edition:2021 -#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled +#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op async fn foo() {} fn main() { diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr index bf66cc9ea90..bd39c9d092c 100644 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr @@ -1,10 +1,10 @@ -warning: `#[track_caller]` on async functions is a no-op, unless the `closure_track_caller` feature is enabled - --> $DIR/issue-104588-no-op-track-caller.rs:4:16 +warning: `#[track_caller]` on async functions is a no-op + --> $DIR/issue-104588-no-op-track-caller.rs:4:1 | -LL | #[track_caller] - | ________________^ -LL | | async fn foo() {} - | |_ +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ help: enable this feature: `#[closure_track_caller]` +LL | async fn foo() {} + | ----------------- this function will not propagate the caller location | = note: `#[warn(ungated_async_fn_track_caller)]` on by default diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr new file mode 100644 index 00000000000..e0201f2238d --- /dev/null +++ b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr @@ -0,0 +1,14 @@ +warning: `#[track_caller]` on async functions is a no-op + --> $DIR/panic-track-caller.rs:50:1 + | +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ help: enable this feature: `#[closure_track_caller]` +LL | / async fn bar_track_caller() { +LL | | panic!() +LL | | } + | |_- this function will not propagate the caller location + | + = note: `#[warn(ungated_async_fn_track_caller)]` on by default + +warning: 1 warning emitted + diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.rs b/src/test/ui/async-await/track-caller/panic-track-caller.rs index 066cf97628f..ee37e64be4f 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.rs +++ b/src/test/ui/async-await/track-caller/panic-track-caller.rs @@ -1,7 +1,9 @@ // run-pass // edition:2021 +// revisions: feat nofeat // needs-unwind -#![feature(closure_track_caller, async_closure, stmt_expr_attributes)] +#![feature(async_closure, stmt_expr_attributes)] +#![cfg_attr(feat, feature(closure_track_caller))] use std::future::Future; use std::panic; @@ -45,7 +47,7 @@ async fn foo() { bar().await } -#[track_caller] +#[track_caller] //[nofeat]~ WARN `#[track_caller]` on async functions is a no-op async fn bar_track_caller() { panic!() } @@ -91,8 +93,20 @@ fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { } fn main() { - assert_eq!(panicked_at(|| block_on(foo())), 41); - assert_eq!(panicked_at(|| block_on(foo_track_caller())), 54); - assert_eq!(panicked_at(|| block_on(foo_assoc())), 67); + assert_eq!(panicked_at(|| block_on(foo())), 43); + + #[cfg(feat)] + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 56); + #[cfg(nofeat)] + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 52); + + #[cfg(feat)] + assert_eq!(panicked_at(|| block_on(foo_assoc())), 69); + #[cfg(nofeat)] + assert_eq!(panicked_at(|| block_on(foo_assoc())), 64); + + #[cfg(feat)] + assert_eq!(panicked_at(|| block_on(foo_closure())), 76); + #[cfg(feat)] assert_eq!(panicked_at(|| block_on(foo_closure())), 74); } From e28a07a0a157b63dc11e9f590484d5332866623a Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 24 Nov 2022 03:48:27 +0000 Subject: [PATCH 3/7] update wording of lint --- compiler/rustc_error_messages/locales/en-US/lint.ftl | 2 +- compiler/rustc_lint/src/builtin.rs | 2 +- .../track-caller/issue-104588-no-op-track-caller.stderr | 2 +- .../async-await/track-caller/panic-track-caller.nofeat.stderr | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index a36a511bd4a..0d5507835a9 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -351,7 +351,7 @@ lint_builtin_mutable_transmutes = lint_builtin_unstable_features = unstable feature lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op - .suggestion = enable this feature + .suggestion = enable this unstable feature lint_builtin_unreachable_pub = unreachable `pub` {$what} .suggestion = consider restricting its visibility diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index e90572cb238..c353f742516 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1409,7 +1409,7 @@ fn check_fn( |lint| { lint.span_label(span, "this function will not propagate the caller location"); if cx.tcx.sess.is_nightly_build() { - lint.span_suggestion(attr.span, fluent::suggestion, "#[closure_track_caller]", Applicability::MachineApplicable); + lint.span_suggestion(attr.span, fluent::suggestion, "closure_track_caller", Applicability::MachineApplicable); } lint } diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr index bd39c9d092c..b41077a0b92 100644 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr +++ b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr @@ -2,7 +2,7 @@ warning: `#[track_caller]` on async functions is a no-op --> $DIR/issue-104588-no-op-track-caller.rs:4:1 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ help: enable this feature: `#[closure_track_caller]` + | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` LL | async fn foo() {} | ----------------- this function will not propagate the caller location | diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr index e0201f2238d..71d4b00cc6e 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr +++ b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr @@ -2,7 +2,7 @@ warning: `#[track_caller]` on async functions is a no-op --> $DIR/panic-track-caller.rs:50:1 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ help: enable this feature: `#[closure_track_caller]` + | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` LL | / async fn bar_track_caller() { LL | | panic!() LL | | } From 2d060034f0fd5f5780c7fd41046901b8e0cdf7e8 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 7 Dec 2022 20:13:22 +0000 Subject: [PATCH 4/7] Update track_caller logic/lint after rebase --- compiler/rustc_ast_lowering/src/expr.rs | 53 ++++++++++--------- compiler/rustc_lint/src/builtin.rs | 16 ++++-- compiler/rustc_lint/src/lib.rs | 1 + .../panic-track-caller.nofeat.stderr | 12 ++++- .../track-caller/panic-track-caller.rs | 7 +-- 5 files changed, 52 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index a3f5c18f2e7..a58c74dbc33 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -656,34 +656,35 @@ pub(super) fn make_async_expr( hir::ExprKind::Closure(c) }; - let track_caller = self - .attrs - .get(&outer_hir_id.local_id) - .map_or(false, |attrs| attrs.into_iter().any(|attr| attr.has_name(sym::track_caller))); - let hir_id = self.lower_node_id(closure_node_id); - if track_caller { - let unstable_span = self.mark_span_with_reason( - DesugaringKind::Async, - span, - self.allow_gen_future.clone(), - ); - self.lower_attrs( - hir_id, - &[Attribute { - kind: AttrKind::Normal(ptr::P(NormalAttr { - item: AttrItem { - path: Path::from_ident(Ident::new(sym::track_caller, span)), - args: AttrArgs::Empty, + let unstable_span = self.mark_span_with_reason( + DesugaringKind::Async, + span, + self.allow_gen_future.clone(), + ); + if self.tcx.features().closure_track_caller { + let track_caller = self + .attrs + .get(&outer_hir_id.local_id) + .map_or(false, |attrs| attrs.into_iter().any(|attr| attr.has_name(sym::track_caller))); + if track_caller { + self.lower_attrs( + hir_id, + &[Attribute { + kind: AttrKind::Normal(ptr::P(NormalAttr { + item: AttrItem { + path: Path::from_ident(Ident::new(sym::track_caller, span)), + args: AttrArgs::Empty, + tokens: None, + }, tokens: None, - }, - tokens: None, - })), - id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), - style: AttrStyle::Outer, - span: unstable_span, - }], - ); + })), + id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), + style: AttrStyle::Outer, + span: unstable_span, + }], + ); + } } let generator = hir::Expr { hir_id, kind: generator_kind, span: self.lower_span(span) }; diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index c353f742516..6d3e33f2b62 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1397,7 +1397,7 @@ fn check_fn( span: Span, hir_id: HirId, ) { - if let HirFnKind::ItemFn(_, _, _) = fn_kind && fn_kind.asyncness() == IsAsync::Async && !cx.tcx.features().closure_track_caller { + if fn_kind.asyncness() == IsAsync::Async && !cx.tcx.features().closure_track_caller { // Now, check if the function has the `#[track_caller]` attribute let attrs = cx.tcx.hir().attrs(hir_id); let maybe_track_caller = attrs.iter().find(|attr| attr.has_name(sym::track_caller)); @@ -1407,12 +1407,20 @@ fn check_fn( attr.span, fluent::lint_ungated_async_fn_track_caller, |lint| { - lint.span_label(span, "this function will not propagate the caller location"); + lint.span_label( + span, + "this function will not propagate the caller location", + ); if cx.tcx.sess.is_nightly_build() { - lint.span_suggestion(attr.span, fluent::suggestion, "closure_track_caller", Applicability::MachineApplicable); + lint.span_suggestion( + attr.span, + fluent::suggestion, + "closure_track_caller", + Applicability::MachineApplicable, + ); } lint - } + }, ); } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 11022eb80ea..1275d6f223c 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -219,6 +219,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { // May Depend on constants elsewhere UnusedBrokenConst: UnusedBrokenConst, UnstableFeatures: UnstableFeatures, + UngatedAsyncFnTrackCaller: UngatedAsyncFnTrackCaller, ArrayIntoIter: ArrayIntoIter::default(), DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr index 71d4b00cc6e..f313658c446 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr +++ b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr @@ -10,5 +10,15 @@ LL | | } | = note: `#[warn(ungated_async_fn_track_caller)]` on by default -warning: 1 warning emitted +warning: `#[track_caller]` on async functions is a no-op + --> $DIR/panic-track-caller.rs:62:5 + | +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` +LL | / async fn bar_assoc() { +LL | | panic!(); +LL | | } + | |_____- this function will not propagate the caller location + +warning: 2 warnings emitted diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.rs b/src/test/ui/async-await/track-caller/panic-track-caller.rs index ee37e64be4f..02077db7c62 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.rs +++ b/src/test/ui/async-await/track-caller/panic-track-caller.rs @@ -59,7 +59,7 @@ async fn foo_track_caller() { struct Foo; impl Foo { - #[track_caller] + #[track_caller] //[nofeat]~ WARN `#[track_caller]` on async functions is a no-op async fn bar_assoc() { panic!(); } @@ -104,9 +104,4 @@ fn main() { assert_eq!(panicked_at(|| block_on(foo_assoc())), 69); #[cfg(nofeat)] assert_eq!(panicked_at(|| block_on(foo_assoc())), 64); - - #[cfg(feat)] - assert_eq!(panicked_at(|| block_on(foo_closure())), 76); - #[cfg(feat)] - assert_eq!(panicked_at(|| block_on(foo_closure())), 74); } From f702e89f9de9ffb651ffb3b6b8dde5d9ffd1e4d5 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 7 Dec 2022 20:26:56 +0000 Subject: [PATCH 5/7] Add lint doc comment --- compiler/rustc_lint/src/builtin.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 6d3e33f2b62..7c16bc1eade 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1375,7 +1375,26 @@ fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &ast::Attribute) { } declare_lint! { - /// `#[track_caller]` is a no-op without corresponding feature flag + /// The `ungated_async_fn_track_caller` lint warns when the + /// `#[track_caller]` attribute is used on an async function, method, or + /// closure, without enabling the corresponding unstable feature flag. + /// + /// ### Example + /// + /// ```rust + /// #[track_caller] + /// async fn foo() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// The attribute must be used in conjunction with the + /// [`closure_track_caller` feature flag]. Otherwise, the `#[track_caller]` + /// annotation will function as as no-op. + /// + /// [`closure_track_caller` feature flag]: https://doc.rust-lang.org/beta/unstable-book/language-features/closure-track-caller.html UNGATED_ASYNC_FN_TRACK_CALLER, Warn, "enabling track_caller on an async fn is a no-op unless the closure_track_caller feature is enabled" From 9650a4168f2d5f4edb3338a484ab5c8d1a4b0e52 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 21 Dec 2022 03:13:28 +0000 Subject: [PATCH 6/7] Improve code based on feedback. This patch improves the readability of some of the code by using if-let-chains. Also, make use of the `add_feature_diagnostics` function. --- compiler/rustc_ast_lowering/src/expr.rs | 42 +++++++++---------- .../locales/en-US/lint.ftl | 2 +- compiler/rustc_lint/src/builtin.rs | 26 +++++------- .../issue-104588-no-op-track-caller.rs | 9 ---- .../issue-104588-no-op-track-caller.stderr | 12 ------ .../panic-track-caller.nofeat.stderr | 9 +++- 6 files changed, 38 insertions(+), 62 deletions(-) delete mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs delete mode 100644 src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index a58c74dbc33..805050e681b 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -662,29 +662,27 @@ pub(super) fn make_async_expr( span, self.allow_gen_future.clone(), ); - if self.tcx.features().closure_track_caller { - let track_caller = self - .attrs - .get(&outer_hir_id.local_id) - .map_or(false, |attrs| attrs.into_iter().any(|attr| attr.has_name(sym::track_caller))); - if track_caller { - self.lower_attrs( - hir_id, - &[Attribute { - kind: AttrKind::Normal(ptr::P(NormalAttr { - item: AttrItem { - path: Path::from_ident(Ident::new(sym::track_caller, span)), - args: AttrArgs::Empty, - tokens: None, - }, + + if self.tcx.features().closure_track_caller + && let Some(attrs) = self.attrs.get(&outer_hir_id.local_id) + && attrs.into_iter().any(|attr| attr.has_name(sym::track_caller)) + { + self.lower_attrs( + hir_id, + &[Attribute { + kind: AttrKind::Normal(ptr::P(NormalAttr { + item: AttrItem { + path: Path::from_ident(Ident::new(sym::track_caller, span)), + args: AttrArgs::Empty, tokens: None, - })), - id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), - style: AttrStyle::Outer, - span: unstable_span, - }], - ); - } + }, + tokens: None, + })), + id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), + style: AttrStyle::Outer, + span: unstable_span, + }], + ); } let generator = hir::Expr { hir_id, kind: generator_kind, span: self.lower_span(span) }; diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 0d5507835a9..2eb409a5ddd 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -351,7 +351,7 @@ lint_builtin_mutable_transmutes = lint_builtin_unstable_features = unstable feature lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op - .suggestion = enable this unstable feature + .label = this function will not propagate the caller location lint_builtin_unreachable_pub = unreachable `pub` {$what} .suggestion = consider restricting its visibility diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 7c16bc1eade..d6de6e70ead 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1416,33 +1416,27 @@ fn check_fn( span: Span, hir_id: HirId, ) { - if fn_kind.asyncness() == IsAsync::Async && !cx.tcx.features().closure_track_caller { + if fn_kind.asyncness() == IsAsync::Async + && !cx.tcx.features().closure_track_caller + && let attrs = cx.tcx.hir().attrs(hir_id) // Now, check if the function has the `#[track_caller]` attribute - let attrs = cx.tcx.hir().attrs(hir_id); - let maybe_track_caller = attrs.iter().find(|attr| attr.has_name(sym::track_caller)); - if let Some(attr) = maybe_track_caller { + && let Some(attr) = attrs.iter().find(|attr| attr.has_name(sym::track_caller)) + { cx.struct_span_lint( UNGATED_ASYNC_FN_TRACK_CALLER, attr.span, fluent::lint_ungated_async_fn_track_caller, |lint| { - lint.span_label( - span, - "this function will not propagate the caller location", + lint.span_label(span, fluent::label); + rustc_session::parse::add_feature_diagnostics( + lint, + &cx.tcx.sess.parse_sess, + sym::closure_track_caller, ); - if cx.tcx.sess.is_nightly_build() { - lint.span_suggestion( - attr.span, - fluent::suggestion, - "closure_track_caller", - Applicability::MachineApplicable, - ); - } lint }, ); } - } } } diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs deleted file mode 100644 index 6443e14296d..00000000000 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.rs +++ /dev/null @@ -1,9 +0,0 @@ -// check-pass -// edition:2021 - -#[track_caller] //~ WARN `#[track_caller]` on async functions is a no-op -async fn foo() {} - -fn main() { - foo(); -} diff --git a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr b/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr deleted file mode 100644 index b41077a0b92..00000000000 --- a/src/test/ui/async-await/track-caller/issue-104588-no-op-track-caller.stderr +++ /dev/null @@ -1,12 +0,0 @@ -warning: `#[track_caller]` on async functions is a no-op - --> $DIR/issue-104588-no-op-track-caller.rs:4:1 - | -LL | #[track_caller] - | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` -LL | async fn foo() {} - | ----------------- this function will not propagate the caller location - | - = note: `#[warn(ungated_async_fn_track_caller)]` on by default - -warning: 1 warning emitted - diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr index f313658c446..51ea225f4cb 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr +++ b/src/test/ui/async-await/track-caller/panic-track-caller.nofeat.stderr @@ -2,23 +2,28 @@ warning: `#[track_caller]` on async functions is a no-op --> $DIR/panic-track-caller.rs:50:1 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` + | ^^^^^^^^^^^^^^^ LL | / async fn bar_track_caller() { LL | | panic!() LL | | } | |_- this function will not propagate the caller location | + = note: see issue #87417 for more information + = help: add `#![feature(closure_track_caller)]` to the crate attributes to enable = note: `#[warn(ungated_async_fn_track_caller)]` on by default warning: `#[track_caller]` on async functions is a no-op --> $DIR/panic-track-caller.rs:62:5 | LL | #[track_caller] - | ^^^^^^^^^^^^^^^ help: enable this unstable feature: `closure_track_caller` + | ^^^^^^^^^^^^^^^ LL | / async fn bar_assoc() { LL | | panic!(); LL | | } | |_____- this function will not propagate the caller location + | + = note: see issue #87417 for more information + = help: add `#![feature(closure_track_caller)]` to the crate attributes to enable warning: 2 warnings emitted From ccbba0a60e3b094aeb48991cac9b6e342eb3e229 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 21 Dec 2022 23:22:56 +0000 Subject: [PATCH 7/7] Update track_caller tests; run fmt --- compiler/rustc_ast_lowering/src/expr.rs | 7 ++----- .../track-caller/async-closure-gate.rs | 1 - .../track-caller/async-closure-gate.stderr | 15 +-------------- .../track-caller/panic-track-caller.rs | 6 ++++++ 4 files changed, 9 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 805050e681b..3634e6e47ce 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -657,11 +657,8 @@ pub(super) fn make_async_expr( }; let hir_id = self.lower_node_id(closure_node_id); - let unstable_span = self.mark_span_with_reason( - DesugaringKind::Async, - span, - self.allow_gen_future.clone(), - ); + let unstable_span = + self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone()); if self.tcx.features().closure_track_caller && let Some(attrs) = self.attrs.get(&outer_hir_id.local_id) diff --git a/src/test/ui/async-await/track-caller/async-closure-gate.rs b/src/test/ui/async-await/track-caller/async-closure-gate.rs index 9593fdb1908..d9d55685599 100644 --- a/src/test/ui/async-await/track-caller/async-closure-gate.rs +++ b/src/test/ui/async-await/track-caller/async-closure-gate.rs @@ -5,6 +5,5 @@ fn main() { let _ = #[track_caller] async || { //~^ ERROR `#[track_caller]` on closures is currently unstable [E0658] - //~| ERROR `#[track_caller]` on closures is currently unstable [E0658] }; } diff --git a/src/test/ui/async-await/track-caller/async-closure-gate.stderr b/src/test/ui/async-await/track-caller/async-closure-gate.stderr index be3d110eccd..498f1b43b9b 100644 --- a/src/test/ui/async-await/track-caller/async-closure-gate.stderr +++ b/src/test/ui/async-await/track-caller/async-closure-gate.stderr @@ -7,19 +7,6 @@ LL | let _ = #[track_caller] async || { = note: see issue #87417 for more information = help: add `#![feature(closure_track_caller)]` to the crate attributes to enable -error[E0658]: `#[track_caller]` on closures is currently unstable - --> $DIR/async-closure-gate.rs:6:38 - | -LL | let _ = #[track_caller] async || { - | ______________________________________^ -LL | | -LL | | -LL | | }; - | |_____^ - | - = note: see issue #87417 for more information - = help: add `#![feature(closure_track_caller)]` to the crate attributes to enable - -error: aborting due to 2 previous errors +error: aborting due to previous error For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/async-await/track-caller/panic-track-caller.rs b/src/test/ui/async-await/track-caller/panic-track-caller.rs index 02077db7c62..f45243b0ea6 100644 --- a/src/test/ui/async-await/track-caller/panic-track-caller.rs +++ b/src/test/ui/async-await/track-caller/panic-track-caller.rs @@ -69,6 +69,9 @@ async fn foo_assoc() { Foo::bar_assoc().await } +// Since compilation is expected to fail for this fn when using +// `nofeat`, we test that separately in `async-closure-gate.rs` +#[cfg(feat)] async fn foo_closure() { let c = #[track_caller] async || { panic!(); @@ -104,4 +107,7 @@ fn main() { assert_eq!(panicked_at(|| block_on(foo_assoc())), 69); #[cfg(nofeat)] assert_eq!(panicked_at(|| block_on(foo_assoc())), 64); + + #[cfg(feat)] + assert_eq!(panicked_at(|| block_on(foo_closure())), 79); }