Merge pull request #42 from ubolonton/symbol-global-refs

Improve ergonomics of using symbols and defining/signaling errors

Author: ubolonton

CHANGELOG.md

@@ -4,7 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
-- Added `Env::define_error` and `Env::signal` to simplify the process of signaling Lisp errors.
+- Added `OnceGlobalRef`, which eases the initialization of static references to long-lived Lisp values.
+  + Added `use_symbols!`, which enables module code to use Lisp symbols without repeatedly interning them.
+  + Added `define_errors!` and `Env::signal` to simplify the process of defining and signaling custom Lisp errors.
+- Raised the minimum supported Rust version to 1.45.
+- Added `ResultExt::or_signal` to make it more convenient to convert a Rust error into a Lisp error.
+- Remove `ResultExt::unwrap_or_propagate`.
 
 ## [0.16.2] - 2021-03-04
 - Fixed compilation on `aarch64-apple-darwin` (Apple Silicon).

build.rs

@@ -5,8 +5,8 @@ use rustc_version::{version, Version};
 
 fn main() {
     let version = version().unwrap();
-    if version < Version::parse("1.38.0").unwrap() {
-        eprintln!("emacs-module-rs requires rustc 1.38.0 or newer, got {}", version);
+    if version < Version::parse("1.45.0").unwrap() {
+        eprintln!("emacs-module-rs requires rustc 1.45.0 or newer, got {}", version);
         exit(1);
     }
 }

emacs-macros/src/lib.rs

@@ -9,6 +9,7 @@ extern crate proc_macro2;
 use proc_macro::TokenStream;
 
 use syn::{self, AttributeArgs, ItemFn, LitInt, parse_macro_input};
+use quote::quote;
 
 mod util;
 mod module;
@@ -129,3 +130,13 @@ pub fn impl_lisp_args_for_arrays(length: TokenStream) -> TokenStream {
     let length: LitInt = parse_macro_input!(length);
     lisp_args::impl_for_arrays(length.base10_parse::<usize>().unwrap()).into()
 }
+
+/// Converts an identifier into a Lisp name, as a string literal.
+///
+/// This replaces underscores with hyphens.
+#[doc(hidden)]
+#[proc_macro]
+pub fn lisp_name(ident: TokenStream) -> TokenStream {
+    let name = util::lisp_name(&parse_macro_input!(ident));
+    quote!(#name).into()
+}

guide/src/errors.md

@@ -40,24 +40,19 @@ This is similar to handling Lisp errors. The only difference is `ErrorKind::Thro
 
 ## Signaling Lisp Errors from Rust
 
-The function `env.signal` allows signaling a Lisp error from Rust code. The error symbol must have been defined, e.g. by calling `env.define_error`:
+The function `env.signal` allows signaling a Lisp error from Rust code. The error symbol must have been defined, e.g. by the macro `define_errors!`:
 
 ```rust
-pub static my_custom_error: OnceCell<GlobalRef> = OnceCell::new();
-
-#[emacs::module]
-fn init(env: &Env) -> Result<Value<'_>> {
-    env.define_error(
-        my_custom_error.get_or_try_init(|| env.intern("my-custom-error").map(GlobalRef::new))?,
-        "This number should not be negative",
-        [env.intern("error")?]
-    )
+// The parentheses denote parent error signals.
+// If unspecified, the parent error signal is `error`.
+emacs::define_errors! {
+    my_custom_error "This number should not be negative" (arith_error range_error)
 }
 
 #[defun]
 fn signal_if_negative(env: &Env, x: i16) -> Result<()> {
     if (x < 0) {
-        return env.signal(my_custom_error.get().unwrap(), ("associated", "DATA", 7))
+        return env.signal(my_custom_error., ("associated", "DATA", 7))
     }
     Ok(())
 }

src/error.rs

@@ -1,16 +1,17 @@
 #[doc(no_inline)]
+use std::{any::Any, fmt::Display, mem::MaybeUninit, result, thread};
+
 pub use anyhow::{self, Error};
-use std::mem::MaybeUninit;
-use std::result;
-use std::thread;
-use std::any::Any;
 use thiserror::Error;
 
-use super::IntoLisp;
-use super::{Env, Value};
 use emacs_module::*;
-use crate::{symbol::{self, IntoLispSymbol}, GlobalRef};
-use crate::call::IntoLispArgs;
+
+use crate::{
+    Env, Value, IntoLisp,
+    GlobalRef,
+    symbol::{self, IntoLispSymbol},
+    call::IntoLispArgs,
+};
 
 // We use const instead of enum, in case Emacs add more exit statuses in the future.
 // See https://github.com/rust-lang/rust/issues/36927
@@ -23,6 +24,38 @@ pub struct TempValue {
     raw: emacs_value,
 }
 
+/// Defines new error signals.
+///
+/// TODO: Document this properly.
+///
+/// This macro can be used only once per Rust `mod`.
+#[macro_export]
+macro_rules! define_errors {
+    ($( $name:ident $message:literal $( ( $( $parent:ident )+ ) )? )*) => {
+        $crate::global_refs! {__emrs_init_global_refs_to_error_symbols__(init_to_symbol) =>
+            $( $name )*
+        }
+
+        #[$crate::deps::ctor::ctor]
+        fn __emrs_define_errors__() {
+            $crate::init::__CUSTOM_ERRORS__.try_lock()
+                .expect("Failed to acquire a write lock on the list of initializers for custom error signals")
+                .push(::std::boxed::Box::new(|env| {
+                    $(
+                        env.define_error($name, $message, [
+                            $(
+                                $(
+                                    env.intern($crate::deps::emacs_macros::lisp_name!($parent))?
+                                ),+
+                            )?
+                        ])?;
+                    )*
+                    Ok(())
+                }));
+        }
+    }
+}
+
 /// Error types generic to all Rust dynamic modules.
 ///
 /// This list is intended to grow over time and it is not recommended to exhaustively match against
@@ -100,6 +133,7 @@ impl TempValue {
 // XXX: Technically these are unsound, but they are necessary to use the `Fail` trait. We ensure
 // safety by marking TempValue methods as unsafe.
 unsafe impl Send for TempValue {}
+
 unsafe impl Sync for TempValue {}
 
 impl Env {
@@ -117,16 +151,14 @@ impl Env {
                 Err(ErrorKind::Signal {
                     symbol: unsafe { TempValue::new(symbol.assume_init()) },
                     data: unsafe { TempValue::new(data.assume_init()) },
-                }
-                .into())
+                }.into())
             }
             (THROW, tag, value) => {
                 self.non_local_exit_clear();
                 Err(ErrorKind::Throw {
                     tag: unsafe { TempValue::new(tag.assume_init()) },
                     value: unsafe { TempValue::new(value.assume_init()) },
-                }
-                .into())
+                }.into())
             }
             _ => panic!("Unexpected non local exit status {}", status),
         }
@@ -163,7 +195,7 @@ impl Env {
                 if let Err(error) = m {
                     m = match error.downcast::<ErrorKind>() {
                         // TODO: Explain safety.
-                        Ok(err) => unsafe { return self.handle_known(&*err) },
+                        Ok(err) => unsafe { return self.handle_known(&*err); },
                         Err(error) => Err(error),
                     }
                 }
@@ -175,15 +207,15 @@ impl Env {
         }
     }
 
-    pub(crate) fn define_errors(&self) -> Result<()> {
+    pub(crate) fn define_core_errors(&self) -> Result<()> {
         // FIX: Make panics louder than errors, by somehow make sure that 'rust-panic is
         // not a sub-type of 'error.
-        self.define_error(symbol::rust_panic, "Rust panic", (symbol::error,))?;
-        self.define_error(symbol::rust_error, "Rust error", (symbol::error,))?;
+        self.define_error(symbol::rust_panic, "Rust panic", (symbol::error, ))?;
+        self.define_error(symbol::rust_error, "Rust error", (symbol::error, ))?;
         self.define_error(
             symbol::rust_wrong_type_user_ptr,
             "Wrong type user-ptr",
-            (symbol::rust_error, self.intern("wrong-type-argument")?)
+            (symbol::rust_error, self.intern("wrong-type-argument")?),
         )?;
         Ok(())
     }
@@ -255,46 +287,21 @@ impl Env {
     }
 }
 
-/// Emacs-specific extension methods for [`Result`].
+/// Emacs-specific extension methods for the standard library's [`Result`].
 ///
-/// [`Result`]: type.Result.html
+/// [`Result`]: result::Result
 pub trait ResultExt<T, E> {
-    /// Unwraps a result, yielding the content of an [`Ok`].
-    ///
-    /// # Panics
-    ///
-    /// Panics if the value is an [`Err`], using a sensible panic value.
-    ///
-    /// If the underlying error is an [`ErrorKind`], it will be used as the value of the panic,
-    /// which makes the `#[defun]` behave as if the corresponding non-local exit was propagated.
-    /// Otherwise, tries to use [`Display`] to get a descriptive error message.
+    /// Converts the error into a Lisp signal if this result is an [`Err`]. The first element of the
+    /// associated signal data will be a string formatted with [`Display::fmt`].
     ///
-    /// This is useful when errors cannot be propagated using [`Result`], e.g. callbacks whose types
-    /// are dictated by 3rd-party libraries.
-    ///
-    /// # Safety
-    ///
-    /// The panic must not propagate across an FFI boundary, e.g. this must not be used in callbacks
-    /// that will be called by C code. See Rust's [`issue #52652`].
-    ///
-    /// [`Ok`]: https://doc.rust-lang.org/std/result/enum.Result.html#variant.Ok
-    /// [`Err`]: https://doc.rust-lang.org/std/result/enum.Result.html#variant.Err
-    /// [`ErrorKind`]: enum.ErrorKind.html
-    /// [`Display`]: https://doc.rust-lang.org/std/fmt/trait.Display.html
-    /// [`Result`]: type.Result.html
-    /// [`issue #52652`]: https://github.com/rust-lang/rust/issues/52652
-    #[deprecated(since = "0.12.0", note = "Use Result or a variable to track error instead")]
-    unsafe fn unwrap_or_propagate(self) -> T;
+    /// If the result is an [`Ok`], it is returned unchanged.
+    fn or_signal<'e, S>(self, env: &'e Env, symbol: S) -> Result<T> where S: IntoLispSymbol<'e>;
 }
 
-impl<T> ResultExt<T, Error> for Result<T> {
-    #[inline]
-    unsafe fn unwrap_or_propagate(self) -> T {
-        self.unwrap_or_else(|error| {
-            match error.downcast::<ErrorKind>() {
-                Ok(err) => panic!(err),
-                Err(error) => panic!("{}", error),
-            };
-        })
+impl<T, E: Display> ResultExt<T, E> for result::Result<T, E> {
+    fn or_signal<'e, S>(self, env: &'e Env, symbol: S) -> Result<T> where S: IntoLispSymbol<'e> {
+        self.or_else(|err| env.signal(symbol, (
+            format!("{}", err),
+        )))
     }
 }

src/global.rs

@@ -95,12 +95,6 @@ impl<'e> IntoLisp<'e> for &'e GlobalRef {
     }
 }
 
-impl<'e> IntoLisp<'e> for &'e OnceGlobalRef {
-    #[inline(always)]
-    fn into_lisp(self, env: &'e Env) -> Result<Value<'e>> {
-        Ok(self.bind(env))
-    }
-}
 
 impl<'e> Value<'e> {
     /// Creates a new [`GlobalRef`] for this value.
@@ -112,45 +106,99 @@ impl<'e> Value<'e> {
     }
 }
 
+/// Declares global references. These will be initialized when the module is loaded.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! global_refs {
+    ($($name:ident)*) => {
+        $(
+            #[allow(non_upper_case_globals)]
+            pub static $name: &'static $crate::OnceGlobalRef = {
+                static X: $crate::OnceGlobalRef = $crate::OnceGlobalRef::new();
+                &X
+            };
+        )*
+    };
+    ($registrator_name:ident ($init_method:ident) =>
+        $(
+            $name:ident $( => $lisp_name:expr )?
+        )*
+    ) => {
+        $crate::global_refs! {
+            $($name)*
+        }
+
+        #[$crate::deps::ctor::ctor]
+        fn $registrator_name() {
+            $crate::init::__GLOBAL_REFS__.try_lock()
+                .expect("Failed to acquire a write lock on the list of initializers for global refs")
+                .push(::std::boxed::Box::new(|env| {
+                    $(
+                        #[allow(unused_variables)]
+                        let name = $crate::deps::emacs_macros::lisp_name!($name);
+                        $( let name = $lisp_name; )?
+                        $crate::OnceGlobalRef::$init_method(&$name, env, name)?;
+                    )*
+                    Ok(())
+                }));
+        }
+    };
+}
+
 /// A [`GlobalRef`] that can be initialized once. This is useful for long-lived values that should
-/// be initialized when the module is loaded, such as frequently-used symbols.
+/// be initialized when the dynamic module is loaded. A typical use case is specifying
+/// frequently-used symbols, which can be done with the help of the macro [`use_symbols!`].
 ///
-/// [`GlobalRef`]: struct.GlobalRef.html
+/// [`use_symbols`]: crate::use_symbols
 #[derive(Debug)]
 #[repr(transparent)]
 pub struct OnceGlobalRef {
     inner: OnceCell<GlobalRef>
 }
 
 impl OnceGlobalRef {
-    pub(crate) const fn new() -> Self {
+    pub const fn new() -> Self {
         Self { inner: OnceCell::new() }
     }
 
     /// Initializes this global reference with the given function.
     #[doc(hidden)]
-    pub fn init<F: FnOnce(&Env) -> Result<Value>>(&self, env: &Env, f: F) -> Result<()> {
+    pub fn init<F: FnOnce(&Env) -> Result<Value>>(&self, env: &Env, f: F) -> Result<&GlobalRef> {
         let g = f(env)?.make_global_ref();
         self.inner.set(g).expect("Cannot initialize a global reference more than once");
-        Ok(())
+        Ok(self.inner.get().expect("Failed to get an initialized OnceGlobalRef"))
     }
 
     /// Points this global reference to an interned Lisp symbol with the given name.
+    ///
+    /// This should be called once, during module initialization.
     #[doc(hidden)]
-    pub fn init_to_symbol(&self, env: &Env, name: &str) -> Result<()> {
+    pub fn init_to_symbol(&self, env: &Env, name: &str) -> Result<&GlobalRef> {
         self.init(env, |env| env.intern(name))
     }
 
     /// Points this global reference to the function bound to the Lisp symbol with the given name.
+    ///
+    /// This should be called once, during module initialization.
+    ///
+    /// If the symbol is later bound to another function, this global reference will still point to
+    /// the old function. Therefore, this is best used for built-in and primitive functions.
     #[doc(hidden)]
-    pub fn init_to_function(&self, env: &Env, name: &str) -> Result<()> {
+    pub fn init_to_function(&self, env: &Env, name: &str) -> Result<&GlobalRef> {
         self.init(env, |env| {
             let symbol = env.intern(name)?;
             env.call("indirect-function", [symbol])
         })
     }
 }
 
+impl<'e> IntoLisp<'e> for &'e OnceGlobalRef {
+    #[inline(always)]
+    fn into_lisp(self, env: &'e Env) -> Result<Value<'e>> {
+        Ok(self.bind(env))
+    }
+}
+
 impl Deref for OnceGlobalRef {
     type Target = GlobalRef;