Allow real borrow-checking of #[pg_extern] fn lifetimes

pgrx-pg-sys/src/submodules/panic.rs

@@ -377,11 +377,13 @@ enum GuardAction<R> {
 ///
 /// [trivially-deallocated stack frames](https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md#plain-old-frames)
 #[doc(hidden)]
-pub unsafe fn pgrx_extern_c_guard<Func, R: Copy>(f: Func) -> R
+// FIXME: previously, R was bounded on Copy, but this prevents using move-only POD types
+// what we really want is a bound of R: !Drop, but negative bounds don't exist yet
+pub unsafe fn pgrx_extern_c_guard<Func, R>(f: Func) -> R
 where
     Func: FnOnce() -> R + UnwindSafe + RefUnwindSafe,
 {
-    match run_guarded(f) {
+    match unsafe { run_guarded(f) } {
         GuardAction::Return(r) => r,
         GuardAction::ReThrow => {
             extern "C" /* "C-unwind" */ {
@@ -399,8 +401,9 @@ where
     }
 }
 
+// SAFETY: similar constraints as pgrx_extern_c_guard
 #[inline(never)]
-fn run_guarded<F, R: Copy>(f: F) -> GuardAction<R>
+unsafe fn run_guarded<F, R>(f: F) -> GuardAction<R>
 where
     F: FnOnce() -> R + UnwindSafe + RefUnwindSafe,
 {

pgrx-sql-entity-graph/src/finfo.rs

@@ -40,7 +40,6 @@ pub fn finfo_v1_extern_c(
         #[no_mangle]
         #[doc(hidden)]
         #unused_lifetimes
-        #[::pgrx::pgrx_macros::pg_guard]
         pub unsafe extern "C" fn #wrapper_symbol #lifetimes(#fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
             #contents
         }

pgrx-sql-entity-graph/src/metadata/sql_translatable.rs

@@ -46,7 +46,7 @@ pub enum SqlMapping {
     Composite {
         array_brackets: bool,
     },
-    /// Placeholder for some types with no simple translation
+    /// A type which does not actually appear in SQL
     Skip,
 }
 

pgrx-sql-entity-graph/src/pg_extern/mod.rs

@@ -28,6 +28,7 @@ pub(crate) use attribute::Attribute;
 pub use cast::PgCast;
 pub use operator::PgOperator;
 pub use returning::NameMacro;
+use syn::token::Comma;
 
 use self::returning::Returning;
 use super::UsedType;
@@ -38,7 +39,7 @@ use crate::ToSqlConfig;
 use operator::{PgrxOperatorAttributeWithIdent, PgrxOperatorOpName};
 use search_path::SearchPathList;
 
-use proc_macro2::{Ident, TokenStream as TokenStream2};
+use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
 use quote::{format_ident, quote, quote_spanned};
 use syn::parse::{Parse, ParseStream, Parser};
 use syn::punctuated::Punctuated;
@@ -377,6 +378,10 @@ impl PgExtern {
         let is_raw = self.extern_attrs().contains(&Attribute::Raw);
         // We use a `_` prefix to make functions with no args more satisfied during linting.
         let fcinfo_ident = syn::Ident::new("_fcinfo", self.func.sig.ident.span());
+        let lifetimes =
+            self.func.sig.generics.lifetimes().collect::<syn::punctuated::Punctuated<_, Comma>>();
+        let fc_lt = syn::Lifetime::new("'fcx", Span::mixed_site());
+        let fc_ltparam = syn::LifetimeParam::new(fc_lt.clone());
 
         let args = &self.inputs;
         let arg_pats = args.iter().map(|v| format_ident!("{}_", &v.pat)).collect::<Vec<_>>();
@@ -416,21 +421,26 @@ impl PgExtern {
                     syn::ReturnType::Type(_, ret_ty) => ret_ty.clone(),
                 };
                 let wrapper_code = quote_spanned! { self.func.block.span() =>
+                    fn _internal_wrapper<#fc_ltparam, #lifetimes>(fcinfo: ::pgrx::callconv::Fcinfo<#fc_lt>) -> ::pgrx::datum::Datum<#fc_lt> {
                     #[allow(unused_unsafe)]
                     unsafe {
-                        let fcinfo = #fcinfo_ident;
-                        let result = match <#ret_ty as ::pgrx::callconv::RetAbi>::check_fcinfo_and_prepare(fcinfo) {
+                        let #fcinfo_ident = fcinfo.0;
+                        let result = match <#ret_ty as ::pgrx::callconv::RetAbi>::check_fcinfo_and_prepare(#fcinfo_ident) {
                             ::pgrx::callconv::CallCx::WrappedFn(mcx) => {
                                 let mut mcx = ::pgrx::PgMemoryContexts::For(mcx);
                                 ::pgrx::callconv::RetAbi::to_ret(mcx.switch_to(|_| {
                                     #(#arg_fetches)*
                                     #func_name( #(#arg_pats),* )
                                 }))
                             }
-                            ::pgrx::callconv::CallCx::RestoreCx => <#ret_ty as ::pgrx::callconv::RetAbi>::ret_from_fcinfo_fcx(fcinfo),
+                            ::pgrx::callconv::CallCx::RestoreCx => <#ret_ty as ::pgrx::callconv::RetAbi>::ret_from_fcinfo_fcx(#fcinfo_ident),
                         };
-                        unsafe { <#ret_ty as ::pgrx::callconv::RetAbi>::box_ret_in_fcinfo(fcinfo, result) }
-                    }
+                        ::core::mem::transmute(unsafe { <#ret_ty as ::pgrx::callconv::RetAbi>::box_ret_in_fcinfo(#fcinfo_ident, result) })
+                    }}
+                    let fcinfo = ::pgrx::callconv::Fcinfo(#fcinfo_ident, ::core::marker::PhantomData);
+                    // We preserve the invariants
+                    let datum = unsafe { ::pgrx::pg_sys::submodules::panic::pgrx_extern_c_guard(move || _internal_wrapper(fcinfo)) };
+                    datum.sans_lifetime()
                 };
                 finfo_v1_extern_c(&self.func, fcinfo_ident, wrapper_code)
             }

pgrx-sql-entity-graph/src/pg_trigger/mod.rs

@@ -71,29 +71,33 @@ impl PgTrigger {
     pub fn wrapper_tokens(&self) -> Result<ItemFn, syn::Error> {
         let function_ident = self.func.sig.ident.clone();
         let fcinfo_ident = Ident::new("_fcinfo", function_ident.span());
+
         let tokens = quote! {
-            let fcinfo_ref = unsafe {
-                // SAFETY:  The caller should be Postgres in this case and it will give us a valid "fcinfo" pointer
-                #fcinfo_ident.as_ref().expect("fcinfo was NULL from Postgres")
-            };
-            let maybe_pg_trigger = unsafe { ::pgrx::trigger_support::PgTrigger::from_fcinfo(fcinfo_ref) };
-            let pg_trigger = maybe_pg_trigger.expect("PgTrigger::from_fcinfo failed");
-            let trigger_fn_result: Result<
-                Option<::pgrx::heap_tuple::PgHeapTuple<'_, _>>,
-                _,
-            > = #function_ident(&pg_trigger);
-
-
-            // The trigger "protocol" allows a function to return the null pointer, but NOT to
-            // set the isnull result flag.  This is why we return `Datum::from(0)` in the None cases
-            let trigger_retval = trigger_fn_result.expect("Trigger function panic");
-            match trigger_retval {
-                None => unsafe { ::pgrx::pg_sys::Datum::from(0) },
-                Some(trigger_retval) => match trigger_retval.into_trigger_datum() {
+            fn _internal(fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
+                let fcinfo_ref = unsafe {
+                    // SAFETY:  The caller should be Postgres in this case and it will give us a valid "fcinfo" pointer
+                    fcinfo.as_ref().expect("fcinfo was NULL from Postgres")
+                };
+                let maybe_pg_trigger = unsafe { ::pgrx::trigger_support::PgTrigger::from_fcinfo(fcinfo_ref) };
+                let pg_trigger = maybe_pg_trigger.expect("PgTrigger::from_fcinfo failed");
+                let trigger_fn_result: Result<
+                    Option<::pgrx::heap_tuple::PgHeapTuple<'_, _>>,
+                    _,
+                > = #function_ident(&pg_trigger);
+
+
+                // The trigger "protocol" allows a function to return the null pointer, but NOT to
+                // set the isnull result flag.  This is why we return `Datum::from(0)` in the None cases
+                let trigger_retval = trigger_fn_result.expect("Trigger function panic");
+                match trigger_retval {
                     None => unsafe { ::pgrx::pg_sys::Datum::from(0) },
-                    Some(datum) => datum,
+                    Some(trigger_retval) => match trigger_retval.into_trigger_datum() {
+                        None => unsafe { ::pgrx::pg_sys::Datum::from(0) },
+                        Some(datum) => datum,
+                    }
                 }
             }
+            ::pgrx::pg_sys::submodules::panic::pgrx_extern_c_guard(move || _internal(#fcinfo_ident))
         };
 
         finfo_v1_extern_c(&self.func, fcinfo_ident, tokens)

pgrx-tests/tests/todo/roundtrip-tests.stderr

@@ -18,6 +18,18 @@ error[E0261]: use of undeclared lifetime name `'a`
 69 |         Vec<Option<&'a str>>,
    |                     ^^ undeclared lifetime
 
+error[E0261]: use of undeclared lifetime name `'a`
+  --> tests/todo/roundtrip-tests.rs:69:21
+   |
+22 |               fn $fname(i: $rtype) -> $rtype {
+   |  ____________________________________________-
+23 | |                 i
+24 | |             }
+   | |_____________- lifetime `'a` is missing in item created through this procedural macro
+...
+69 |           Vec<Option<&'a str>>,
+   |                       ^^ undeclared lifetime
+
 error[E0261]: use of undeclared lifetime name `'a`
   --> tests/todo/roundtrip-tests.rs:69:21
    |

pgrx/src/callconv.rs

@@ -10,13 +10,22 @@
 #![doc(hidden)]
 #![deny(unsafe_op_in_unsafe_fn)]
 //! Helper implementations for returning sets and tables from `#[pg_extern]`-style functions
+
 use crate::heap_tuple::PgHeapTuple;
 use crate::{
     pg_return_null, pg_sys, AnyNumeric, Date, Inet, Internal, Interval, IntoDatum, Json, PgBox,
     PgVarlena, Time, TimeWithTimeZone, Timestamp, TimestampWithTimeZone, Uuid,
 };
+use core::marker::PhantomData;
+use core::panic::{RefUnwindSafe, UnwindSafe};
 use std::ffi::{CStr, CString};
 
+type FcinfoData = pg_sys::FunctionCallInfoBaseData;
+// FIXME: replace with a real implementation
+pub struct Fcinfo<'a>(pub pg_sys::FunctionCallInfo, pub PhantomData<&'a mut FcinfoData>);
+impl<'fcx> UnwindSafe for Fcinfo<'fcx> {}
+impl<'fcx> RefUnwindSafe for Fcinfo<'fcx> {}
+
 /// How to return a value from Rust to Postgres
 ///
 /// This bound is necessary to distinguish things which can be returned from a `#[pg_extern] fn`.