C enums as module consts + deprecation (#1748)

I tried to do without the deprecation warning code but I found it simply
so annoying that rustc couldn't find the right names (its
Levenshtein-derived algorithm doesn't reason about pathnames quite that
way) that I figured it would be easiest to simply add the relevant
codegen. That made it possible to distinguish between disappearing types
and now-outdated names.

Closes #1370.

Author: workingjubilee

pgrx-examples/bad_ideas/src/lib.rs

@@ -110,8 +110,11 @@ fn random_abort() {
 #[pg_guard]
 pub unsafe extern "C" fn _PG_init() {
     #[pg_guard]
-    extern "C" fn random_abort_callback(event: pg_sys::XactEvent, _arg: *mut std::os::raw::c_void) {
-        if event == pg_sys::XactEvent_XACT_EVENT_PRE_COMMIT && rand::random::<bool>() {
+    extern "C" fn random_abort_callback(
+        event: pg_sys::XactEvent::Type,
+        _arg: *mut std::os::raw::c_void,
+    ) {
+        if event == pg_sys::XactEvent::XACT_EVENT_PRE_COMMIT && rand::random::<bool>() {
             // panic!("aborting transaction");
         }
     }

pgrx-pg-sys/build.rs

@@ -7,18 +7,20 @@
 //LICENSE All rights reserved.
 //LICENSE
 //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
-use bindgen::callbacks::{DeriveTrait, ImplementsTrait, MacroParsingBehavior};
+use bindgen::callbacks::{DeriveTrait, EnumVariantValue, ImplementsTrait, MacroParsingBehavior};
 use bindgen::NonCopyUnionStyle;
 use eyre::{eyre, WrapErr};
 use pgrx_pg_config::{
     is_supported_major_version, prefix_path, PgConfig, PgConfigSelector, Pgrx, SUPPORTED_VERSIONS,
 };
 use quote::{quote, ToTokens};
+use std::cell::RefCell;
 use std::cmp::Ordering;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fs;
 use std::path::{self, Path, PathBuf}; // disambiguate path::Path and syn::Type::Path
 use std::process::{Command, Output};
+use std::rc::Rc;
 use std::sync::OnceLock;
 use syn::{ForeignItem, Item, ItemConst};
 
@@ -30,44 +32,49 @@ mod build {
 }
 
 #[derive(Debug)]
-struct PgrxOverrides(HashSet<String>);
+struct BindingOverride {
+    ignore_macros: HashSet<&'static str>,
+    enum_names: InnerMut<EnumMap>,
+}
+
+type InnerMut<T> = Rc<RefCell<T>>;
+type EnumMap = BTreeMap<String, Vec<(String, EnumVariantValue)>>;
 
-impl PgrxOverrides {
-    fn default() -> Self {
+impl BindingOverride {
+    fn new_from(enum_names: InnerMut<EnumMap>) -> Self {
         // these cause duplicate definition problems on linux
         // see: https://github.com/rust-lang/rust-bindgen/issues/687
-        PgrxOverrides(
-            vec![
-                "FP_INFINITE".into(),
-                "FP_NAN".into(),
-                "FP_NORMAL".into(),
-                "FP_SUBNORMAL".into(),
-                "FP_ZERO".into(),
-                "IPPORT_RESERVED".into(),
+        BindingOverride {
+            ignore_macros: HashSet::from_iter([
+                "FP_INFINITE",
+                "FP_NAN",
+                "FP_NORMAL",
+                "FP_SUBNORMAL",
+                "FP_ZERO",
+                "IPPORT_RESERVED",
                 // These are just annoying due to clippy
-                "M_E".into(),
-                "M_LOG2E".into(),
-                "M_LOG10E".into(),
-                "M_LN2".into(),
-                "M_LN10".into(),
-                "M_PI".into(),
-                "M_PI_2".into(),
-                "M_PI_4".into(),
-                "M_1_PI".into(),
-                "M_2_PI".into(),
-                "M_SQRT2".into(),
-                "M_SQRT1_2".into(),
-                "M_2_SQRTPI".into(),
-            ]
-            .into_iter()
-            .collect(),
-        )
+                "M_E",
+                "M_LOG2E",
+                "M_LOG10E",
+                "M_LN2",
+                "M_LN10",
+                "M_PI",
+                "M_PI_2",
+                "M_PI_4",
+                "M_1_PI",
+                "M_2_PI",
+                "M_SQRT2",
+                "M_SQRT1_2",
+                "M_2_SQRTPI",
+            ]),
+            enum_names,
+        }
     }
 }
 
-impl bindgen::callbacks::ParseCallbacks for PgrxOverrides {
+impl bindgen::callbacks::ParseCallbacks for BindingOverride {
     fn will_parse_macro(&self, name: &str) -> MacroParsingBehavior {
-        if self.0.contains(name) {
+        if self.ignore_macros.contains(name) {
             bindgen::callbacks::MacroParsingBehavior::Ignore
         } else {
             bindgen::callbacks::MacroParsingBehavior::Default
@@ -90,6 +97,45 @@ impl bindgen::callbacks::ParseCallbacks for PgrxOverrides {
         };
         Some(implements_trait)
     }
+
+    // FIXME: alter types on some int macros to the actually-used types so we can stop as-casting them
+    fn int_macro(&self, _name: &str, _value: i64) -> Option<bindgen::callbacks::IntKind> {
+        None
+    }
+
+    // FIXME: implement a... C compiler?
+    fn func_macro(&self, _name: &str, _value: &[&[u8]]) {}
+
+    /// Intentionally doesn't do anything, just updates internal state.
+    fn enum_variant_behavior(
+        &self,
+        enum_name: Option<&str>,
+        variant_name: &str,
+        variant_value: bindgen::callbacks::EnumVariantValue,
+    ) -> Option<bindgen::callbacks::EnumVariantCustomBehavior> {
+        enum_name.inspect(|name| match name.strip_prefix("enum").unwrap_or(name).trim() {
+            // specifically overridden enum
+            "NodeTag" => return,
+            name if name.contains("unnamed at") => return,
+            // to prevent problems with BuiltinOid
+            _ if variant_name.contains("OID") => return,
+            name => self
+                .enum_names
+                .borrow_mut()
+                .entry(name.to_string())
+                .or_insert(Vec::new())
+                .push((variant_name.to_string(), variant_value)),
+        });
+        None
+    }
+
+    // FIXME: hide nodetag fields and default them to appropriate values
+    fn field_visibility(
+        &self,
+        _info: bindgen::callbacks::FieldInfo<'_>,
+    ) -> Option<bindgen::FieldVisibilityKind> {
+        None
+    }
 }
 
 fn main() -> eyre::Result<()> {
@@ -718,12 +764,15 @@ fn run_bindgen(
         let builtin_includes = includes.iter().filter_map(|p| Some(format!("-I{}", p.to_str()?)));
         binder = binder.clang_args(builtin_includes);
     };
+    let enum_names = Rc::new(RefCell::new(BTreeMap::new()));
+    let overrides = BindingOverride::new_from(Rc::clone(&enum_names));
     let bindings = binder
         .header(include_h.display().to_string())
         .clang_args(extra_bindgen_clang_args(pg_config)?)
         .clang_args(pg_target_include_flags(major_version, pg_config)?)
         .detect_include_paths(autodetect)
-        .parse_callbacks(Box::new(PgrxOverrides::default()))
+        .parse_callbacks(Box::new(overrides))
+        .default_enum_style(bindgen::EnumVariation::ModuleConsts)
         // The NodeTag enum is closed: additions break existing values in the set, so it is not extensible
         .rustified_non_exhaustive_enum("NodeTag")
         .size_t_is_usize(true)
@@ -733,8 +782,33 @@ fn run_bindgen(
         .default_non_copy_union_style(NonCopyUnionStyle::ManuallyDrop)
         .generate()
         .wrap_err_with(|| format!("Unable to generate bindings for pg{major_version}"))?;
+    let mut binding_str = bindings.to_string();
+    drop(bindings); // So the Rc::into_inner can unwrap
+
+    // FIXME: do this for the Node graph instead of reparsing?
+    let enum_names: EnumMap = Rc::into_inner(enum_names).unwrap().into_inner();
+    binding_str.extend(enum_names.into_iter().flat_map(|(name, variants)| {
+        const MIN_I32: i64 = i32::MIN as _;
+        const MAX_I32: i64 = i32::MAX as _;
+        const MAX_U32: u64 = u32::MAX as _;
+        variants.into_iter().map(move |(variant, value)| {
+            let (ty, value) = match value {
+                EnumVariantValue::Boolean(b) => ("bool", b.to_string()),
+                EnumVariantValue::Signed(v @ MIN_I32..=MAX_I32) => ("i32", v.to_string()),
+                EnumVariantValue::Signed(v) => ("i64", v.to_string()),
+                EnumVariantValue::Unsigned(v @ 0..=MAX_U32) => ("u32", v.to_string()),
+                EnumVariantValue::Unsigned(v) => ("u64", v.to_string()),
+            };
+            format!(
+                r#"
+#[deprecated(since = "0.12.0", note = "you want pg_sys::{module}::{variant}")]
+pub const {module}_{variant}: {ty} = {value};"#,
+                module = &*name, // imprecise closure capture
+            )
+        })
+    }));
 
-    Ok(bindings.to_string())
+    Ok(binding_str)
 }
 
 fn add_blocklists(bind: bindgen::Builder) -> bindgen::Builder {

pgrx/src/bgworkers.rs

@@ -61,9 +61,9 @@ bitflags! {
 /// The various points in which a BackgroundWorker can be started by Postgres
 #[derive(Copy, Clone)]
 pub enum BgWorkerStartTime {
-    PostmasterStart = pg_sys::BgWorkerStartTime_BgWorkerStart_PostmasterStart as isize,
-    ConsistentState = pg_sys::BgWorkerStartTime_BgWorkerStart_ConsistentState as isize,
-    RecoveryFinished = pg_sys::BgWorkerStartTime_BgWorkerStart_RecoveryFinished as isize,
+    PostmasterStart = pg_sys::BgWorkerStartTime::BgWorkerStart_PostmasterStart as isize,
+    ConsistentState = pg_sys::BgWorkerStartTime::BgWorkerStart_ConsistentState as isize,
+    RecoveryFinished = pg_sys::BgWorkerStartTime::BgWorkerStart_RecoveryFinished as isize,
 }
 
 /// Static interface into a running Background Worker
@@ -254,7 +254,7 @@ impl BackgroundWorker {
 
 unsafe extern "C" fn worker_spi_sighup(_signal_args: i32) {
     GOT_SIGHUP.store(true, Ordering::SeqCst);
-    pg_sys::ProcessConfigFile(pg_sys::GucContext_PGC_SIGHUP);
+    pg_sys::ProcessConfigFile(pg_sys::GucContext::PGC_SIGHUP);
     pg_sys::SetLatch(pg_sys::MyLatch);
 }
 
@@ -298,13 +298,14 @@ pub enum BackgroundWorkerStatus {
     },
 }
 
-impl From<pg_sys::BgwHandleStatus> for BackgroundWorkerStatus {
-    fn from(s: pg_sys::BgwHandleStatus) -> Self {
+impl From<pg_sys::BgwHandleStatus::Type> for BackgroundWorkerStatus {
+    fn from(s: pg_sys::BgwHandleStatus::Type) -> Self {
+        use pg_sys::BgwHandleStatus::*;
         match s {
-            pg_sys::BgwHandleStatus_BGWH_STARTED => BackgroundWorkerStatus::Started,
-            pg_sys::BgwHandleStatus_BGWH_NOT_YET_STARTED => BackgroundWorkerStatus::NotYetStarted,
-            pg_sys::BgwHandleStatus_BGWH_STOPPED => BackgroundWorkerStatus::Stopped,
-            pg_sys::BgwHandleStatus_BGWH_POSTMASTER_DIED => BackgroundWorkerStatus::PostmasterDied,
+            BGWH_STARTED => BackgroundWorkerStatus::Started,
+            BGWH_NOT_YET_STARTED => BackgroundWorkerStatus::NotYetStarted,
+            BGWH_STOPPED => BackgroundWorkerStatus::Stopped,
+            BGWH_POSTMASTER_DIED => BackgroundWorkerStatus::PostmasterDied,
             _ => unreachable!(),
         }
     }

pgrx/src/callbacks.rs

@@ -57,18 +57,17 @@ pub enum PgXactCallbackEvent {
 }
 
 impl PgXactCallbackEvent {
-    fn translate_pg_event(pg_event: pg_sys::XactEvent) -> Self {
+    fn translate_pg_event(pg_event: pg_sys::XactEvent::Type) -> Self {
+        use pg_sys::XactEvent::*;
         match pg_event {
-            pg_sys::XactEvent_XACT_EVENT_ABORT => PgXactCallbackEvent::Abort,
-            pg_sys::XactEvent_XACT_EVENT_COMMIT => PgXactCallbackEvent::Commit,
-            pg_sys::XactEvent_XACT_EVENT_PARALLEL_ABORT => PgXactCallbackEvent::ParallelAbort,
-            pg_sys::XactEvent_XACT_EVENT_PARALLEL_COMMIT => PgXactCallbackEvent::ParallelCommit,
-            pg_sys::XactEvent_XACT_EVENT_PARALLEL_PRE_COMMIT => {
-                PgXactCallbackEvent::ParallelPreCommit
-            }
-            pg_sys::XactEvent_XACT_EVENT_PREPARE => PgXactCallbackEvent::Prepare,
-            pg_sys::XactEvent_XACT_EVENT_PRE_COMMIT => PgXactCallbackEvent::PreCommit,
-            pg_sys::XactEvent_XACT_EVENT_PRE_PREPARE => PgXactCallbackEvent::PrePrepare,
+            XACT_EVENT_ABORT => PgXactCallbackEvent::Abort,
+            XACT_EVENT_COMMIT => PgXactCallbackEvent::Commit,
+            XACT_EVENT_PARALLEL_ABORT => PgXactCallbackEvent::ParallelAbort,
+            XACT_EVENT_PARALLEL_COMMIT => PgXactCallbackEvent::ParallelCommit,
+            XACT_EVENT_PARALLEL_PRE_COMMIT => PgXactCallbackEvent::ParallelPreCommit,
+            XACT_EVENT_PREPARE => PgXactCallbackEvent::Prepare,
+            XACT_EVENT_PRE_COMMIT => PgXactCallbackEvent::PreCommit,
+            XACT_EVENT_PRE_PREPARE => PgXactCallbackEvent::PrePrepare,
             unknown => panic!("Unrecognized XactEvent: {unknown}"),
         }
     }
@@ -164,7 +163,10 @@ where
 
     // internal function that we register as an XactCallback
     #[pg_guard]
-    unsafe extern "C" fn callback(event: pg_sys::XactEvent, _arg: *mut ::std::os::raw::c_void) {
+    unsafe extern "C" fn callback(
+        event: pg_sys::XactEvent::Type,
+        _arg: *mut ::std::os::raw::c_void,
+    ) {
         let which_event = PgXactCallbackEvent::translate_pg_event(event);
 
         let hooks = match which_event {
@@ -252,14 +254,13 @@ pub enum PgSubXactCallbackEvent {
 }
 
 impl PgSubXactCallbackEvent {
-    fn translate_pg_event(event: pg_sys::SubXactEvent) -> Self {
+    fn translate_pg_event(event: pg_sys::SubXactEvent::Type) -> Self {
+        use pg_sys::SubXactEvent::*;
         match event {
-            pg_sys::SubXactEvent_SUBXACT_EVENT_ABORT_SUB => PgSubXactCallbackEvent::AbortSub,
-            pg_sys::SubXactEvent_SUBXACT_EVENT_COMMIT_SUB => PgSubXactCallbackEvent::CommitSub,
-            pg_sys::SubXactEvent_SUBXACT_EVENT_PRE_COMMIT_SUB => {
-                PgSubXactCallbackEvent::PreCommitSub
-            }
-            pg_sys::SubXactEvent_SUBXACT_EVENT_START_SUB => PgSubXactCallbackEvent::StartSub,
+            SUBXACT_EVENT_ABORT_SUB => PgSubXactCallbackEvent::AbortSub,
+            SUBXACT_EVENT_COMMIT_SUB => PgSubXactCallbackEvent::CommitSub,
+            SUBXACT_EVENT_PRE_COMMIT_SUB => PgSubXactCallbackEvent::PreCommitSub,
+            SUBXACT_EVENT_START_SUB => PgSubXactCallbackEvent::StartSub,
             _ => panic!("Unrecognized SubXactEvent: {event}"),
         }
     }
@@ -317,7 +318,7 @@ where
 
     #[pg_guard]
     unsafe extern "C" fn callback(
-        event: pg_sys::SubXactEvent,
+        event: pg_sys::SubXactEvent::Type,
         my_subid: pg_sys::SubTransactionId,
         parent_subid: pg_sys::SubTransactionId,
         _arg: *mut ::std::os::raw::c_void,

pgrx/src/callconv.rs

@@ -723,7 +723,7 @@ impl<'fcx> FcInfo<'fcx> {
     pub unsafe fn srf_return_next(&mut self) {
         unsafe {
             self.deref_fcx().call_cntr += 1;
-            self.get_result_info().set_is_done(pg_sys::ExprDoneCond_ExprMultipleResult);
+            self.get_result_info().set_is_done(pg_sys::ExprDoneCond::ExprMultipleResult);
         }
     }
 
@@ -733,7 +733,7 @@ impl<'fcx> FcInfo<'fcx> {
     pub unsafe fn srf_return_done(&mut self) {
         unsafe {
             pg_sys::end_MultiFuncCall(self.0, self.deref_fcx());
-            self.get_result_info().set_is_done(pg_sys::ExprDoneCond_ExprEndResult);
+            self.get_result_info().set_is_done(pg_sys::ExprDoneCond::ExprEndResult);
         }
     }
 
@@ -872,23 +872,23 @@ impl<'fcx> ReturnSetInfoWrapper<'fcx> {
     */
     // These four fields are, in-practice, owned by the callee.
     /// Status for ValuePerCall mode.
-    pub fn set_is_done(&mut self, value: pg_sys::ExprDoneCond) {
+    pub fn set_is_done(&mut self, value: pg_sys::ExprDoneCond::Type) {
         unsafe {
             (*self.0).isDone = value;
         }
     }
     /// Status for ValuePerCall mode.
-    pub fn get_is_done(&self) -> pg_sys::ExprDoneCond {
+    pub fn get_is_done(&self) -> pg_sys::ExprDoneCond::Type {
         unsafe { (*self.0).isDone }
     }
     /// Actual return mode.
-    pub fn set_return_mode(&mut self, return_mode: pgrx_pg_sys::SetFunctionReturnMode) {
+    pub fn set_return_mode(&mut self, return_mode: pgrx_pg_sys::SetFunctionReturnMode::Type) {
         unsafe {
             (*self.0).returnMode = return_mode;
         }
     }
     /// Actual return mode.
-    pub fn get_return_mode(&self) -> pgrx_pg_sys::SetFunctionReturnMode {
+    pub fn get_return_mode(&self) -> pgrx_pg_sys::SetFunctionReturnMode::Type {
         unsafe { (*self.0).returnMode }
     }
     /// Holds the complete returned tuple set.

pgrx/src/enum_helper.rs

@@ -15,7 +15,7 @@ use crate::{ereport, pg_sys, PgLogLevel, PgSqlErrorCode};
 pub fn lookup_enum_by_oid(enumval: pg_sys::Oid) -> (String, pg_sys::Oid, f32) {
     let tup = unsafe {
         pg_sys::SearchSysCache(
-            pg_sys::SysCacheIdentifier_ENUMOID as i32,
+            pg_sys::SysCacheIdentifier::ENUMOID as i32,
             pg_sys::Datum::from(enumval),
             pg_sys::Datum::from(0),
             pg_sys::Datum::from(0),
@@ -61,7 +61,7 @@ pub fn lookup_enum_by_label(typname: &str, label: &str) -> pg_sys::Datum {
         let label =
             alloc::ffi::CString::new(label).expect("failed to convert enum typname to a CString");
         pg_sys::SearchSysCache(
-            pg_sys::SysCacheIdentifier_ENUMTYPOIDNAME as i32,
+            pg_sys::SysCacheIdentifier::ENUMTYPOIDNAME as i32,
             pg_sys::Datum::from(enumtypoid),
             pg_sys::Datum::from(label.as_ptr()),
             pg_sys::Datum::from(0usize),