From 0c158f0e9d708e7705249bca8e38757ebf2e5c8c Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Mon, 10 Oct 2022 16:00:46 -0700 Subject: [PATCH 1/6] Add a test case for #[track_caller] on async fn --- .../ui/async-await/panic-no-track-caller.rs | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 src/test/ui/async-await/panic-no-track-caller.rs diff --git a/src/test/ui/async-await/panic-no-track-caller.rs b/src/test/ui/async-await/panic-no-track-caller.rs new file mode 100644 index 0000000000000..934764912d35a --- /dev/null +++ b/src/test/ui/async-await/panic-no-track-caller.rs @@ -0,0 +1,74 @@ +// run-pass +// edition:2021 + +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] +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())), 39); + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 52); +} From 3db41d13f08db377c9bc516d8f285f61ed668edd Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 11 Oct 2022 16:37:54 -0700 Subject: [PATCH 2/6] wip: trying to enable #[track_caller] on async fn --- compiler/rustc_ast_lowering/src/expr.rs | 20 ++++++++++++++++++- compiler/rustc_ast_lowering/src/item.rs | 2 +- ...-track-caller.rs => panic-track-caller.rs} | 1 + 3 files changed, 21 insertions(+), 2 deletions(-) rename src/test/ui/async-await/{panic-no-track-caller.rs => panic-track-caller.rs} (98%) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 46886c518afd5..61d91874e6173 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -617,8 +617,26 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Closure(c) }; + let generator_hir_id = self.lower_node_id(closure_node_id); + // FIXME: only add track caller if the parent is track_caller + self.lower_attrs( + generator_hir_id, + &[Attribute { + kind: AttrKind::Normal(ptr::P(NormalAttr { + item: AttrItem { + path: Path::from_ident(Ident::new(sym::track_caller, span)), + args: MacArgs::Empty, + tokens: None, + }, + tokens: None, + })), + id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), + style: AttrStyle::Outer, + span, + }], + ); let generator = hir::Expr { - hir_id: self.lower_node_id(closure_node_id), + hir_id: generator_hir_id, kind: generator_kind, span: self.lower_span(span), }; diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 9a46444d82398..e9cc53b7138c9 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -86,7 +86,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> { impl_trait_defs: Vec::new(), impl_trait_bounds: Vec::new(), allow_try_trait: Some([sym::try_trait_v2, sym::yeet_desugar_details][..].into()), - allow_gen_future: Some([sym::gen_future][..].into()), + allow_gen_future: Some([sym::gen_future, sym::closure_track_caller][..].into()), allow_into_future: Some([sym::into_future][..].into()), generics_def_id_map: Default::default(), }; diff --git a/src/test/ui/async-await/panic-no-track-caller.rs b/src/test/ui/async-await/panic-track-caller.rs similarity index 98% rename from src/test/ui/async-await/panic-no-track-caller.rs rename to src/test/ui/async-await/panic-track-caller.rs index 934764912d35a..76776d41c57c7 100644 --- a/src/test/ui/async-await/panic-no-track-caller.rs +++ b/src/test/ui/async-await/panic-track-caller.rs @@ -1,5 +1,6 @@ // run-pass // edition:2021 +#![feature(closure_track_caller)] use std::future::Future; use std::panic; From fa99cb82690d99c0df2018d37aedaed29e40e131 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Tue, 8 Nov 2022 23:45:55 +0000 Subject: [PATCH 3/6] Allow and add `track_caller` to generators This patch allows the usage of the `track_caller` annotation on generators, as well as sets them conditionally if the parent also has `track_caller` set. Also add this annotation on the `GenFuture`'s `poll()` function. --- compiler/rustc_ast_lowering/src/expr.rs | 60 ++++++++++++------- library/core/src/future/mod.rs | 1 + .../{ => track-caller}/panic-track-caller.rs | 4 +- 3 files changed, 40 insertions(+), 25 deletions(-) rename src/test/ui/async-await/{ => track-caller}/panic-track-caller.rs (93%) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 61d91874e6173..6f68f679cc00a 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -617,33 +617,47 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Closure(c) }; - let generator_hir_id = self.lower_node_id(closure_node_id); - // FIXME: only add track caller if the parent is track_caller - self.lower_attrs( - generator_hir_id, - &[Attribute { - kind: AttrKind::Normal(ptr::P(NormalAttr { - item: AttrItem { - path: Path::from_ident(Ident::new(sym::track_caller, span)), - args: MacArgs::Empty, + let mut parent_has_track_caller = false; + for attrs in self.attrs.values() { + for attr in attrs.into_iter() { + if attr.has_name(sym::track_caller) { + parent_has_track_caller = true; + break; + } + } + if parent_has_track_caller { + break; + } + } + let unstable_span = + self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone()); + + let hir_id = if parent_has_track_caller { + let generator_hir_id = self.lower_node_id(closure_node_id); + self.lower_attrs( + generator_hir_id, + &[Attribute { + kind: AttrKind::Normal(ptr::P(NormalAttr { + item: AttrItem { + path: Path::from_ident(Ident::new(sym::track_caller, span)), + args: MacArgs::Empty, + tokens: None, + }, tokens: None, - }, - tokens: None, - })), - id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), - style: AttrStyle::Outer, - span, - }], - ); - let generator = hir::Expr { - hir_id: generator_hir_id, - kind: generator_kind, - span: self.lower_span(span), + })), + id: self.tcx.sess.parse_sess.attr_id_generator.mk_attr_id(), + style: AttrStyle::Outer, + span: unstable_span, + }], + ); + generator_hir_id + } else { + self.lower_node_id(closure_node_id) }; + let generator = hir::Expr { hir_id, kind: generator_kind, span: self.lower_span(span) }; + // `future::from_generator`: - let unstable_span = - self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone()); let gen_future = self.expr_lang_item_path( unstable_span, hir::LangItem::FromGenerator, diff --git a/library/core/src/future/mod.rs b/library/core/src/future/mod.rs index 6487aa0885955..107cf92c1c0f7 100644 --- a/library/core/src/future/mod.rs +++ b/library/core/src/future/mod.rs @@ -82,6 +82,7 @@ where impl> Future for GenFuture { type Output = T::Return; + #[track_caller] fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { // SAFETY: Safe because we're !Unpin + !Drop, and this is just a field projection. let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; diff --git a/src/test/ui/async-await/panic-track-caller.rs b/src/test/ui/async-await/track-caller/panic-track-caller.rs similarity index 93% rename from src/test/ui/async-await/panic-track-caller.rs rename to src/test/ui/async-await/track-caller/panic-track-caller.rs index 76776d41c57c7..4e659da9ee069 100644 --- a/src/test/ui/async-await/panic-track-caller.rs +++ b/src/test/ui/async-await/track-caller/panic-track-caller.rs @@ -70,6 +70,6 @@ fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { } fn main() { - assert_eq!(panicked_at(|| block_on(foo())), 39); - assert_eq!(panicked_at(|| block_on(foo_track_caller())), 52); + assert_eq!(panicked_at(|| block_on(foo())), 40); + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 53); } From 509b9478f58582d8ddca1e2a31436e487b74fae5 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 10 Nov 2022 00:13:48 +0000 Subject: [PATCH 4/6] Refactor nested for-loops into find() calls --- compiler/rustc_ast_lowering/src/expr.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 6f68f679cc00a..12930fd136362 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -617,18 +617,11 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Closure(c) }; - let mut parent_has_track_caller = false; - for attrs in self.attrs.values() { - for attr in attrs.into_iter() { - if attr.has_name(sym::track_caller) { - parent_has_track_caller = true; - break; - } - } - if parent_has_track_caller { - break; - } - } + let parent_has_track_caller = self + .attrs + .values() + .find(|attrs| attrs.into_iter().find(|attr| attr.has_name(sym::track_caller)).is_some()) + .is_some(); let unstable_span = self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone()); From b678e9221a57dcdb7cf959af4e83c0579622349d Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Fri, 11 Nov 2022 17:35:13 +0000 Subject: [PATCH 5/6] Ignore wasm and emscripten targets for test --- src/test/ui/async-await/track-caller/panic-track-caller.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 4e659da9ee069..5884a1b4a8cfe 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,5 +1,8 @@ // run-pass // edition:2021 + +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support #![feature(closure_track_caller)] use std::future::Future; @@ -70,6 +73,6 @@ fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { } fn main() { - assert_eq!(panicked_at(|| block_on(foo())), 40); - assert_eq!(panicked_at(|| block_on(foo_track_caller())), 53); + assert_eq!(panicked_at(|| block_on(foo())), 43); + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 56); } From 79c06fc595261a118cea2e5440ed98fbf5659a99 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Tue, 15 Nov 2022 01:07:18 +0000 Subject: [PATCH 6/6] Use `needs-unwind` instead of ignoring WASM/emscripten --- .../ui/async-await/track-caller/panic-track-caller.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 5884a1b4a8cfe..b113c56412ff6 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,8 +1,6 @@ // run-pass // edition:2021 - -// ignore-wasm no panic or subprocess support -// ignore-emscripten no panic or subprocess support +// needs-unwind #![feature(closure_track_caller)] use std::future::Future; @@ -73,6 +71,6 @@ fn panicked_at(f: impl FnOnce() + panic::UnwindSafe) -> u32 { } fn main() { - assert_eq!(panicked_at(|| block_on(foo())), 43); - assert_eq!(panicked_at(|| block_on(foo_track_caller())), 56); + assert_eq!(panicked_at(|| block_on(foo())), 41); + assert_eq!(panicked_at(|| block_on(foo_track_caller())), 54); }