Split Arena API into its own crate

Author: HoneyPony

Cargo.lock

@@ -1,6 +1,6 @@
 [workspace]
 resolver = "3"
-members = [ "poniescript","poniescript-core", "poniescript-lsp", "poniescript-gc" , "poni-build", "poni-build-cli", "poni-doc", "poni-doc-cli"]
+members = [ "poniescript","poniescript-core", "poniescript-lsp", "poniescript-gc" , "poni-build", "poni-build-cli", "poni-doc", "poni-doc-cli", "poni-arena"]
 
 [workspace.dependencies]
 clap = { version = "4.5.8", features = ["derive"] }

Cargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "poni-arena"
+version = "0.1.0"
+edition = "2024"
+
+[dependencies]

poni-arena/Cargo.toml

@@ -2,6 +2,12 @@ use std::{cell::UnsafeCell, marker::PhantomData, num::{NonZeroU32, NonZeroUsize}
 
 const DEFAULT_CAPACITY: usize = 256;
 
+/// Trait for keys for Arenas.
+/// 
+/// These are the core methods necessary to use a particular opaque type as
+/// a key for the various Arena types. However, for the most part you do not need
+/// to implement ArenaKey manually. Instead, use [`define_arena_key!`] for defining
+/// your opaque key types.
 pub trait ArenaKey: Copy {
     fn to_nonzero_usize(self) -> NonZeroUsize;
     unsafe fn from_nonzero_u32(id: NonZeroU32) -> Self;
@@ -13,30 +19,44 @@ pub trait ArenaKey: Copy {
 
     #[inline(always)]
     unsafe fn from_index(index: usize) -> Self {
-        #[cfg(debug_assertions)]
-        {
-            Self::from_nonzero_u32(NonZeroU32::new(index as u32 + 1).unwrap())
-        }
+        unsafe {
+            #[cfg(debug_assertions)]
+            {
+                Self::from_nonzero_u32(NonZeroU32::new(index as u32 + 1).unwrap())
+            }
 
-        #[cfg(not(debug_assertions))]
-        {
-            Self::from_nonzero_u32(NonZeroU32::new_unchecked(index as u32 + 1))
+            #[cfg(not(debug_assertions))]
+            {
+                Self::from_nonzero_u32(NonZeroU32::new_unchecked(index as u32 + 1))
+            }
         }
     }
 
     #[inline(always)]
     unsafe fn invalid() -> Self {
-        Self::from_index(0)
+        unsafe { Self::from_index(0) }
     }
 }
 
+/// Defines a new key type for an Arena.
+/// 
+/// The idea here is to associate a new opaque type with each kind of Arena.
+/// For example, if you have an Arena of Exprs and an Arena of Stmts, you may
+/// want an ExprKey/ExprId and a StmtKey/StmtId.
+/// 
+/// In this case, you could `define_arena_key!(ExprId);` and then later construct
+/// a new arena such as `let arena: Arena<Expr, ExprId> = ...;`
+/// 
+/// Note that Arena keys are currently not associated in any way with the Arena
+/// instance they belong to. This may change in the future.
+#[macro_export]
 macro_rules! define_arena_key {
     ($key_name:ident) => {
         #[derive(Clone, Copy, PartialEq, Eq, Hash)]
         pub struct $key_name(std::num::NonZeroU32);
 
         // TODO: Is this the best fully qualified name for this trait?
-        impl crate::arena::ArenaKey for $key_name {
+        impl $crate::ArenaKey for $key_name {
             #[inline(always)]
             fn to_nonzero_usize(self) -> std::num::NonZeroUsize {
                 // Safety: Our self.0 is always nonzero, so this should always also
@@ -52,6 +72,11 @@ macro_rules! define_arena_key {
     }
 }
 
+/// The simplest Arena type.
+/// 
+/// This Arena is essentially a `Vec<Ty>`, indexed by the `Key` type. The main
+/// value it provides is to enable the Key types to be opaque, rather than 
+/// normal `usize`s, and to enable conversion into an ArenaCell.
 pub struct Arena<Ty, Key: ArenaKey> {
     objects: Vec<Ty>,
     phantom: PhantomData<Key>
@@ -137,6 +162,21 @@ impl<Ty, Key: ArenaKey> Arena<Ty, Key> {
     }
 }
 
+/// This type is essentially a Vec<RefCell<T>>, except that the borrow checking
+/// for the inner T happens entirely in a side table, rather than per-item.
+/// 
+/// There is also an additional caveat that, as of this writing, ArenaCell
+/// is *intentionally* unsound. In particular, the runtime borrow-checking rules
+/// (a la RefCell) are only enabled when `debug_assertions` are enabled. In
+/// release mode, the borrow checking rules are entirely elided.
+/// 
+/// The motivation for this is that the target use-case of ArenaCell, namely
+/// compiler data structures, does not really enable 'accidentally' forgetting
+/// to drop a borrow. If you have a half-decent test suite, and you have an
+/// accidental double-borrow, you WILL see the panic in debug mode, and you
+/// will be able to trivially fix it.
+/// 
+/// In the future, there may be some 
 pub struct ArenaCell<Ty, Key: ArenaKey> {
     objects: UnsafeCell<Vec<Ty>>,
 

poniescript-core/src/arena.rs poni-arena/src/lib.rsponiescript-core/src/arena.rs renamed to poni-arena/src/lib.rs

@@ -10,5 +10,6 @@ anstyle = "1.0.7"
 clap = { workspace = true }
 crossbeam = { version = "0.8.4", features = ["crossbeam-channel", "crossbeam-queue"] }
 log = "0.4.28"
+poni-arena = { version = "0.1.0", path = "../poni-arena" }
 rustc-hash = "2.0.0"
 ufmt = { version = "0.2.0", features = ["std"] }

poniescript-core/Cargo.toml

@@ -48,7 +48,7 @@ fn generate_impl(file: &mut File, pairs: &Vec<(&str, &str)>) {
 	let mut init = String::new();
 	let mut struct_ = String::new();
 	let mut db_impl = String::new();
-	writeln!(file, "use crate::arena::Arena;").unwrap();
+	writeln!(file, "use poni_arena::Arena;").unwrap();
 	writeln!(file, "pub type IdType = {TY};").unwrap();
 	writeln!(init, "impl DbArenas {{").unwrap();
 	writeln!(init, "\tpub fn new() -> Self {{").unwrap();

poniescript-core/build/build_db.rs

@@ -7,7 +7,7 @@ use crate::module::Module;
 use crate::source::SourceLocation;
 use crate::typ::Type;
 
-use crate::arena::IndexCell;
+use poni_arena::IndexCell;
 
 struct NameChecker {
 	buffer: String,

poniescript-core/src/binder.rs

@@ -4,7 +4,7 @@ use crossbeam::channel;
 use std::fs::File;
 use std::io::Write;
 
-use crate::arena::ArenaKey;
+use poni_arena::ArenaKey;
 use crate::{db::*, Args};
 use crate::typ::Type;
 

poniescript-core/src/codegen.rs

@@ -1,7 +1,7 @@
 use rustc_hash::FxHashSet;
 use ufmt::uwrite;
 
-use crate::arena::ArenaKey;
+use poni_arena::ArenaKey;
 use crate::codegen::*;
 use crate::db::*;
 use crate::lexer::Tok;

poniescript-core/src/codegen/single_codegen.rs

@@ -19,15 +19,16 @@ use crate::expr::Var;
 use crate::typ::RangeEnd;
 use crate::typ::Type;
 use crate::source::{PathBufFileSource, Source, SourceLocation, SyntheticSource};
-use crate::{arena::*, inf_writeln, Args};
+use crate::{inf_writeln, Args};
 
 use crate::glue::lexer::GlueTok;
 
 use crate::lexer::{Tok, Token};
 
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use crate::arena::IndexCell;
+use poni_arena::*;
+use poni_arena::define_arena_key;
 
 // Include arenas
 include!(concat!(env!("OUT_DIR"), "/db.arenas.rs"));