Skip to content

Commit 9fb7b2e

Browse files
committed
Add ark::register macro to pass &Console and catch panics
1 parent a726ae6 commit 9fb7b2e

8 files changed

Lines changed: 225 additions & 16 deletions

File tree

Cargo.lock

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ resolver = "2"
1212
members = [
1313
"crates/amalthea",
1414
"crates/ark",
15+
"crates/ark/ark_macros",
1516
"crates/echo",
1617
"crates/harp",
1718
"crates/libr",
@@ -38,6 +39,7 @@ aether_syntax = { git = "https://github.com/posit-dev/air", package = "air_r_syn
3839
amalthea = { path = "crates/amalthea" }
3940
anyhow = "1.0.100"
4041
ark = { path = "crates/ark" }
42+
ark_macros = { path = "crates/ark/ark_macros" }
4143
ark_test = { path = "crates/ark_test" }
4244
assert_matches = "1.5.0"
4345
async-trait = "0.1.66"

crates/ark/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ workspace = true
1919
[dependencies]
2020
actix-web.workspace = true
2121
aether_factory.workspace = true
22+
ark_macros.workspace = true
2223
aether_lsp_utils.workspace = true
2324
aether_parser.workspace = true
2425
aether_syntax.workspace = true

crates/ark/ark_macros/Cargo.toml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
[package]
2+
name = "ark_macros"
3+
version = "0.1.0"
4+
authors.workspace = true
5+
edition.workspace = true
6+
rust-version.workspace = true
7+
license.workspace = true
8+
9+
[lib]
10+
proc-macro = true
11+
12+
[lints]
13+
workspace = true
14+
15+
[dependencies]
16+
quote.workspace = true
17+
syn.workspace = true

crates/ark/ark_macros/src/lib.rs

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
//
2+
// lib.rs
3+
//
4+
// Copyright (C) 2025 Posit Software, PBC. All rights reserved.
5+
//
6+
//
7+
8+
//! Proc macros for the Ark kernel.
9+
//!
10+
//! ## `#[ark::register]`
11+
//!
12+
//! Registers a function as an R `.Call` entry point with automatic `Console`
13+
//! access and panic safety. Composes with `#[harp::register]`.
14+
//!
15+
//! ```ignore
16+
//! #[ark::register]
17+
//! fn ps_my_function(console: &Console, x: SEXP) -> anyhow::Result<SEXP> {
18+
//! let dc = console.device_context();
19+
//! Ok(harp::r_null())
20+
//! }
21+
//! ```
22+
//!
23+
//! The macro transforms this into:
24+
//!
25+
//! ```ignore
26+
//! #[harp::register]
27+
//! unsafe extern "C-unwind" fn ps_my_function(x: SEXP) -> anyhow::Result<SEXP> {
28+
//! crate::console::Console::with(|console| {
29+
//! let dc = console.device_context();
30+
//! Ok(harp::r_null())
31+
//! })
32+
//! }
33+
//! ```
34+
//!
35+
//! `harp::register` then adds `r_unwrap()` (Rust error to R error),
36+
//! `r_sandbox()` (catches R longjumps), and ctor-based routine registration.
37+
//!
38+
//! `Console::with()` catches Rust panics (e.g. from `RefCell` borrow
39+
//! violations) and converts them to `anyhow::Error`, which `r_unwrap()`
40+
//! surfaces as a clean R error instead of crashing the session.
41+
//!
42+
//! The first parameter may be `&Console` (any name, type is matched by
43+
//! the last path segment). It is stripped from the generated C signature
44+
//! and injected at runtime. All remaining parameters must be `SEXP`.
45+
//!
46+
//! The return type must be `anyhow::Result<SEXP>`.
47+
48+
use proc_macro::TokenStream;
49+
use quote::quote;
50+
use syn::parse_macro_input;
51+
52+
extern crate proc_macro;
53+
54+
#[proc_macro_attribute]
55+
pub fn register(_attr: TokenStream, item: TokenStream) -> TokenStream {
56+
let function = parse_macro_input!(item as syn::ItemFn);
57+
match register_impl(function) {
58+
Ok(tokens) => tokens.into(),
59+
Err(err) => err.to_compile_error().into(),
60+
}
61+
}
62+
63+
fn register_impl(function: syn::ItemFn) -> syn::Result<proc_macro::TokenStream> {
64+
let span = function.sig.ident.span();
65+
66+
// Partition parameters: optional leading `&Console` + remaining SEXP args.
67+
let mut console_ident: Option<syn::Ident> = None;
68+
let mut sexp_params: Vec<syn::FnArg> = Vec::new();
69+
70+
for (i, param) in function.sig.inputs.iter().enumerate() {
71+
let typed = match param {
72+
syn::FnArg::Typed(t) => t,
73+
syn::FnArg::Receiver(r) => {
74+
return Err(syn::Error::new_spanned(
75+
r,
76+
"ark::register functions cannot have a `self` parameter",
77+
));
78+
},
79+
};
80+
81+
if i == 0 && is_ref_console(&typed.ty) {
82+
if let syn::Pat::Ident(pat) = &*typed.pat {
83+
console_ident = Some(pat.ident.clone());
84+
} else {
85+
console_ident = Some(syn::Ident::new("console", span));
86+
}
87+
continue;
88+
}
89+
90+
if !is_sexp_type(&typed.ty) {
91+
return Err(syn::Error::new_spanned(
92+
&typed.ty,
93+
"ark::register parameters (other than the leading `&Console`) must be `SEXP`",
94+
));
95+
}
96+
97+
sexp_params.push(param.clone());
98+
}
99+
100+
let ident = &function.sig.ident;
101+
let vis = &function.vis;
102+
let attrs = &function.attrs;
103+
let function_block = &function.block;
104+
105+
// Build the body: wrap in `Console::with()` if `&Console` was requested,
106+
// otherwise just invoke the block directly.
107+
let body = if let Some(console_name) = console_ident {
108+
quote! {
109+
crate::console::Console::with(|#console_name| #function_block)
110+
}
111+
} else {
112+
quote! {
113+
(|| #function_block)()
114+
}
115+
};
116+
117+
Ok(quote! {
118+
#(#attrs)*
119+
#[harp::register]
120+
#vis unsafe extern "C-unwind" fn #ident(#(#sexp_params),*) -> anyhow::Result<libr::SEXP> {
121+
#body
122+
}
123+
}
124+
.into())
125+
}
126+
127+
/// Check if a type is `&Console` (matches `&Console` or `&path::to::Console`).
128+
fn is_ref_console(ty: &syn::Type) -> bool {
129+
let syn::Type::Reference(ref_ty) = ty else {
130+
return false;
131+
};
132+
if ref_ty.mutability.is_some() {
133+
return false;
134+
}
135+
match &*ref_ty.elem {
136+
syn::Type::Path(path) => path
137+
.path
138+
.segments
139+
.last()
140+
.is_some_and(|seg| seg.ident == "Console"),
141+
_ => false,
142+
}
143+
}
144+
145+
/// Check if a type is `SEXP`.
146+
fn is_sexp_type(ty: &syn::Type) -> bool {
147+
let syn::Type::Path(path) = ty else {
148+
return false;
149+
};
150+
path.path
151+
.segments
152+
.last()
153+
.is_some_and(|seg| seg.ident == "SEXP")
154+
}

crates/ark/src/console/console_repl.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,28 @@ impl Console {
677677
Console::get_mut()
678678
}
679679

680+
/// Run a closure with `&Console`, catching panics at the boundary.
681+
///
682+
/// Intended for use in `#[ark::register]` entry points and C callbacks
683+
/// where a panic would unwind through R's C call stack. Panics are
684+
/// caught and converted to `anyhow::Error`, which `harp::register`'s
685+
/// `r_unwrap()` then surfaces as a clean R error.
686+
pub fn with<T>(f: impl FnOnce(&Console) -> anyhow::Result<T>) -> anyhow::Result<T> {
687+
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(Console::get()))) {
688+
Ok(result) => result,
689+
Err(panic) => {
690+
let msg = match panic.downcast_ref::<&str>() {
691+
Some(s) => s.to_string(),
692+
None => match panic.downcast_ref::<String>() {
693+
Some(s) => s.clone(),
694+
None => String::from("(unknown payload)"),
695+
},
696+
};
697+
Err(anyhow!("Panic in Console callback: {msg}"))
698+
},
699+
}
700+
}
701+
680702
/// Install a minimal stopgap `Console` in the thread-local for unit tests
681703
/// that need a `&Console` (e.g. to pass to `CommHandler` methods) but
682704
/// don't go through the full `Console::start()` path.

crates/ark/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
//
66
//
77

8+
extern crate self as ark;
9+
10+
pub use ark_macros::register;
11+
812
pub mod analysis;
913
pub mod ark_comm;
1014
pub mod browser;

crates/ark/src/plots/graphics_device.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,8 @@ pub(crate) fn compute_plot_overrides(
10851085
)
10861086
}
10871087

1088-
/// Activation callback
1088+
/// Run a closure with `&Console` and `&DeviceContext`, catching any panic at
1089+
/// the FFI boundary. Graphics device callbacks are invoked from R
10891090
///
10901091
/// Only used for logging
10911092
///
@@ -1250,13 +1251,12 @@ unsafe extern "C-unwind" fn ps_graphics_device() -> anyhow::Result<SEXP> {
12501251
/// that intermediate plot since we are still on the same plot page with the same plot
12511252
/// `id`.
12521253
#[tracing::instrument(level = "trace", skip_all)]
1253-
#[harp::register]
1254-
unsafe extern "C-unwind" fn ps_graphics_before_plot_new(_name: SEXP) -> anyhow::Result<SEXP> {
1254+
#[ark::register]
1255+
fn ps_graphics_before_plot_new(console: &Console, _name: SEXP) -> anyhow::Result<SEXP> {
12551256
log::trace!("Entering ps_graphics_before_plot_new");
12561257

12571258
// Process changes related to the last plot before opening a new page.
12581259
// Particularly important if we make multiple plots in a single chunk.
1259-
let console = Console::get();
12601260
console.device_context().process_changes(console);
12611261

12621262
Ok(harp::r_null())
@@ -1267,8 +1267,8 @@ unsafe extern "C-unwind" fn ps_graphics_before_plot_new(_name: SEXP) -> anyhow::
12671267
/// Returns a named list with fields: name, kind, execution_id, code, origin_uri.
12681268
/// Returns NULL if no metadata is found for the given ID.
12691269
#[tracing::instrument(level = "trace", skip_all)]
1270-
#[harp::register]
1271-
unsafe extern "C-unwind" fn ps_graphics_get_metadata(id: SEXP) -> anyhow::Result<SEXP> {
1270+
#[ark::register]
1271+
fn ps_graphics_get_metadata(console: &Console, id: SEXP) -> anyhow::Result<SEXP> {
12721272
let id_str: String = RObject::view(id).try_into()?;
12731273
let plot_id = PlotId(id_str);
12741274

@@ -1277,7 +1277,7 @@ unsafe extern "C-unwind" fn ps_graphics_get_metadata(id: SEXP) -> anyhow::Result
12771277
// error handlers that re-enter `plot_contexts.borrow_mut()`, which would
12781278
// panic if the shared borrow were still held.
12791279
let metadata = {
1280-
let contexts = Console::get().device_context().plot_contexts.borrow();
1280+
let contexts = console.device_context().plot_contexts.borrow();
12811281
contexts.get(&plot_id).map(|ctx| ctx.metadata.clone())
12821282
};
12831283

@@ -1311,26 +1311,26 @@ unsafe extern "C-unwind" fn ps_graphics_get_metadata(id: SEXP) -> anyhow::Result
13111311

13121312
/// Return the current plot ID. Used by tests to verify that layout panels
13131313
/// share the same page (same ID) and that overflow creates a new page.
1314-
#[harp::register]
1315-
unsafe extern "C-unwind" fn ps_graphics_current_plot_id() -> anyhow::Result<SEXP> {
1316-
let id = Console::get().device_context().id();
1314+
#[ark::register]
1315+
fn ps_graphics_current_plot_id(console: &Console) -> anyhow::Result<SEXP> {
1316+
let id = console.device_context().id();
13171317
Ok(RObject::from(&id).sexp)
13181318
}
13191319

13201320
/// Push a source file URI onto the source context stack.
13211321
/// Called from the `source()` hook when entering a sourced file.
1322-
#[harp::register]
1323-
unsafe extern "C-unwind" fn ps_graphics_push_source_context(uri: SEXP) -> anyhow::Result<SEXP> {
1322+
#[ark::register]
1323+
fn ps_graphics_push_source_context(console: &Console, uri: SEXP) -> anyhow::Result<SEXP> {
13241324
let uri_str: String = RObject::view(uri).try_into()?;
1325-
Console::get().device_context().push_source_context(uri_str);
1325+
console.device_context().push_source_context(uri_str);
13261326
Ok(harp::r_null())
13271327
}
13281328

13291329
/// Pop a source file URI from the source context stack.
13301330
/// Called from the `source()` hook when leaving a sourced file.
1331-
#[harp::register]
1332-
unsafe extern "C-unwind" fn ps_graphics_pop_source_context() -> anyhow::Result<SEXP> {
1333-
Console::get().device_context().pop_source_context();
1331+
#[ark::register]
1332+
fn ps_graphics_pop_source_context(console: &Console) -> anyhow::Result<SEXP> {
1333+
console.device_context().pop_source_context();
13341334
Ok(harp::r_null())
13351335
}
13361336

0 commit comments

Comments
 (0)