Merge pull request #52 from CppCXY/refactor-stack

Refactor stack

Author: CppCXY

crates/luars/src/gc/gc_object.rs

@@ -483,24 +483,30 @@ impl GcObjectPtr {
     const PTR_MASK: u64 = (1u64 << 48) - 1; // low 48 bits
 
     // Tag values — must match GcObjectKind repr(u8)
-    const TAG_STRING: u64 = 0;
-    const TAG_TABLE: u64 = 1;
-    const TAG_FUNCTION: u64 = 2;
-    const TAG_CCLOSURE: u64 = 3;
-    const TAG_RCLOSURE: u64 = 4;
-    const TAG_UPVALUE: u64 = 5;
-    const TAG_THREAD: u64 = 6;
-    const TAG_USERDATA: u64 = 7;
-    const TAG_PROTO: u64 = 8;
-    #[inline(always)]
-    fn new_tagged(ptr: u64, tag: u64) -> Self {
+    pub const TAG_STRING: u64 = 0;
+    pub const TAG_TABLE: u64 = 1;
+    pub const TAG_FUNCTION: u64 = 2;
+    pub const TAG_CCLOSURE: u64 = 3;
+    pub const TAG_RCLOSURE: u64 = 4;
+    pub const TAG_UPVALUE: u64 = 5;
+    pub const TAG_THREAD: u64 = 6;
+    pub const TAG_USERDATA: u64 = 7;
+    pub const TAG_PROTO: u64 = 8;
+    pub const TAG_NONE: u64 = 9;
+    #[inline(always)]
+    pub fn new_tagged(ptr: u64, tag: u64) -> Self {
         debug_assert!(
             ptr & !Self::PTR_MASK == 0,
             "pointer exceeds 48 bits: 0x{ptr:016x}"
         );
         Self(ptr | (tag << Self::TAG_SHIFT))
     }
 
+    #[inline(always)]
+    pub fn null() -> Self {
+        Self(0)
+    }
+
     #[inline(always)]
     fn tag(&self) -> u8 {
         (self.0 >> Self::TAG_SHIFT) as u8

crates/luars/src/gc/mod.rs

@@ -399,7 +399,7 @@ impl GC {
     #[inline]
     fn prepare_object_for_release(obj: &mut GcObjectOwner) {
         if let Some(thread) = obj.as_thread_mut() {
-            thread.release();
+            thread.release_ci();
         }
     }
 

crates/luars/src/lua_value/lua_table/native_table.rs

@@ -265,7 +265,7 @@ impl NativeTable {
     /// Zero-copy short string lookup — writes directly to destination pointer.
     /// Assumes hash is non-empty and key is short string.
     /// Returns true if found and written.
-    pub unsafe fn get_shortstr_into(&self, key: &LuaValue, dest: *mut LuaValue) -> bool {
+    pub(crate) fn get_shortstr_into(&self, key: &LuaValue, dest: *mut LuaValue) -> bool {
         let mut node = self.mainposition_string(key);
         let key_ptr = key.string_ptr_raw();
 
@@ -388,7 +388,7 @@ impl NativeTable {
         self.pset_shortstr_parts(key, value.value, value.tt)
     }
 
-    pub fn pset_shortstr_parts(
+    pub(crate) fn pset_shortstr_parts(
         &mut self,
         key: &LuaValue,
         value: Value,
@@ -538,19 +538,6 @@ impl NativeTable {
         }
     }
 
-    pub fn finish_shortstr_set_parts(
-        &mut self,
-        key: &LuaValue,
-        value: Value,
-        tt: u8,
-        result: ShortStrSetResult,
-    ) -> (bool, isize) {
-        match result {
-            ShortStrSetResult::Done { new_key, mem_delta } => (new_key, mem_delta),
-            other => self.finish_shortstr_set(key, LuaValue::from_raw(value, tt), other),
-        }
-    }
-
     fn insert_new_shortstr(&mut self, key: &LuaValue, value: LuaValue) -> (bool, isize) {
         debug_assert!(key.is_short_string());
         debug_assert!(!value.is_nil());
@@ -673,7 +660,7 @@ impl NativeTable {
 
     /// Write value to array at Lua index (1-based)
     #[inline(always)]
-    pub unsafe fn write_array(&mut self, lua_index: i64, luaval: LuaValue) {
+    pub(crate) fn write_array(&mut self, lua_index: i64, luaval: LuaValue) {
         if lua_index < 1 || lua_index > self.asize as i64 {
             return;
         }
@@ -1160,7 +1147,7 @@ impl NativeTable {
     /// Returns true if a non-nil/non-empty value was found and written.
     /// CRITICAL: #[inline(always)] — this is the hot path for t[i] reads.
     #[inline(always)]
-    pub unsafe fn fast_geti_into(&self, key: i64, dest: *mut LuaValue) -> bool {
+    pub(crate) fn fast_geti_into(&self, key: i64, dest: *mut LuaValue) -> bool {
         unsafe {
             let u = (key as u64).wrapping_sub(1);
             if u < self.asize as u64 {
@@ -1181,7 +1168,7 @@ impl NativeTable {
     /// Used as fallback when fast_geti_into misses (key not in array range).
     /// Mirrors C Lua's getintfromhash() + finishnodeget().
     #[inline(always)]
-    pub unsafe fn get_int_from_hash_into(&self, key: i64, dest: *mut LuaValue) -> bool {
+    pub(crate) fn get_int_from_hash_into(&self, key: i64, dest: *mut LuaValue) -> bool {
         if self.node.is_null() {
             return false;
         }
@@ -1431,9 +1418,7 @@ impl NativeTable {
         }
         // Move each key: write to array, remove from hash
         for (k, v) in to_migrate {
-            unsafe {
-                self.write_array(k, v);
-            }
+            self.write_array(k, v);
             let key_val = LuaValue::integer(k);
             self.set_node(key_val, LuaValue::nil()); // mark dead in hash
         }
@@ -1678,9 +1663,7 @@ impl NativeTable {
             if i >= 1 && i <= self.asize as i64 {
                 // Key is in array range - set in array part ONLY
                 let was_nil = unsafe { self.read_array(i).is_none() };
-                unsafe {
-                    self.write_array(i, value);
-                }
+                self.write_array(i, value);
                 // DEFENSIVE: If setting to nil, also clear any stale hash entry
                 // for this integer key. This can happen when GC clearing corrupted
                 // lenhint and a key was placed in both array and hash parts.
@@ -1700,9 +1683,7 @@ impl NativeTable {
                 if i == current_len + 1 {
                     let new_size = ((i as u32).next_power_of_two()).max(4);
                     let delta = self.resize_array(new_size);
-                    unsafe {
-                        self.write_array(i, value);
-                    }
+                    self.write_array(i, value);
                     self.migrate_hash_int_keys_to_array();
                     return (true, delta);
                 }

crates/luars/src/lua_value/lua_value.rs

@@ -1032,6 +1032,22 @@ impl LuaValue {
         }
     }
 
+    #[inline(always)]
+    pub(crate) fn as_gc_ptr_unchecked(&self) -> GcObjectPtr {
+        // Unsafe version for internal use when caller already knows it's a GC object
+        let tag = match self.tt {
+            LUA_VTABLE => GcObjectPtr::TAG_TABLE,
+            LUA_VFUNCTION => GcObjectPtr::TAG_FUNCTION,
+            LUA_CCLOSURE => GcObjectPtr::TAG_CCLOSURE,
+            LUA_VRCLOSURE => GcObjectPtr::TAG_RCLOSURE,
+            LUA_VSHRSTR | LUA_VLNGSTR => GcObjectPtr::TAG_STRING,
+            LUA_VTHREAD => GcObjectPtr::TAG_THREAD,
+            LUA_VUSERDATA => GcObjectPtr::TAG_USERDATA,
+            _ => GcObjectPtr::TAG_NONE,
+        };
+        GcObjectPtr::new_tagged(self.raw_ptr() as u64, tag)
+    }
+
     // ============ Truthiness (Lua semantics) ============
 
     /// l_isfalse - Lua truthiness: only nil and false are falsy

crates/luars/src/lua_value/mod.rs

@@ -30,12 +30,7 @@ pub(crate) type LuaInnerValue = Value; // For internal use within LuaValue imple
 
 use crate::Instruction;
 use crate::gc::{ProtoPtr, UpvaluePtr};
-use crate::lua_vm::CFunction;
-
-#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
-pub struct LuaValuePtr {
-    pub ptr: *mut LuaValue,
-}
+use crate::lua_vm::{CFunction, StkId};
 
 /// Runtime upvalue — pointer-based design matching C Lua's UpVal.
 ///
@@ -48,7 +43,7 @@ pub struct LuaValuePtr {
 pub struct LuaUpvalue {
     /// Always-valid pointer to the upvalue's current value.
     /// Open → stack slot, Closed → &self.closed_value
-    v: *mut LuaValue,
+    v: StkId,
     /// Storage for the closed value. When closed, `v` points here.
     closed_value: LuaValue,
     /// Stack index (only meaningful when open)
@@ -59,9 +54,9 @@ impl LuaUpvalue {
     /// Create an open upvalue pointing to a stack location (absolute index).
     /// `stack_ptr` must remain valid until the upvalue is closed or the pointer is updated.
     #[inline(always)]
-    pub fn new_open(stack_index: usize, stack_ptr: LuaValuePtr) -> Self {
+    pub fn new_open(stack_index: usize, stk_id: StkId) -> Self {
         LuaUpvalue {
-            v: stack_ptr.ptr,
+            v: stk_id,
             closed_value: LuaValue::nil(),
             stack_index,
         }
@@ -73,7 +68,7 @@ impl LuaUpvalue {
     #[inline(always)]
     pub fn new_closed(value: LuaValue) -> Self {
         LuaUpvalue {
-            v: std::ptr::null_mut(),
+            v: StkId::null(),
             closed_value: value,
             stack_index: 0,
         }
@@ -84,16 +79,16 @@ impl LuaUpvalue {
     /// No-op for open upvalues (where v is already a valid stack pointer).
     #[inline(always)]
     pub fn fix_closed_ptr(&mut self) {
-        if self.v.is_null() {
-            self.v = &mut self.closed_value as *mut LuaValue;
+        if !self.v.is_valid() {
+            self.v = StkId::from_mut_ptr(&mut self.closed_value as *mut LuaValue);
         }
     }
 
     /// Check if this upvalue is open (like C Lua's `upisopen` macro).
     /// Open ⟺ `v` does NOT point to our own `closed_value` field.
     #[inline(always)]
     pub fn is_open(&self) -> bool {
-        !std::ptr::eq(self.v, &self.closed_value)
+        !std::ptr::eq(self.v.as_ptr(), &self.closed_value)
     }
 
     /// Get the stack index (only meaningful when open).
@@ -107,101 +102,43 @@ impl LuaUpvalue {
     #[inline(always)]
     pub fn close(&mut self, stack_value: LuaValue) {
         self.closed_value = stack_value;
-        self.v = &mut self.closed_value as *mut LuaValue;
+        self.v = StkId::from_mut_ptr(&mut self.closed_value as *mut LuaValue);
     }
 
     /// Update the cached stack pointer (called after stack reallocation).
     #[inline(always)]
-    pub fn update_stack_ptr(&mut self, ptr: *mut LuaValue) {
-        self.v = ptr;
+    pub fn update_stack_ptr(&mut self, stk_id: StkId) {
+        self.v = stk_id;
     }
 
     /// Get the raw v pointer (for caching in the execute loop).
     #[inline(always)]
-    pub fn get_v_ptr(&self) -> *mut LuaValue {
+    pub fn get_v_stk_id(&self) -> StkId {
         self.v
     }
 
     /// Get the value with **zero branching** — single pointer dereference.
     #[inline(always)]
     pub fn get_value(&self) -> LuaValue {
-        debug_assert!(!self.v.is_null(), "upvalue get_value: null pointer");
-        debug_assert!(
-            (self.v as usize) > 0x10000,
-            "upvalue get_value: suspiciously low pointer {:p} (stack_index={})",
-            self.v,
-            self.stack_index
-        );
-        let val = unsafe { *self.v };
-        debug_assert!(
-            Self::is_valid_tt(val.tt()),
-            "upvalue get_value: INVALID type tag 0x{:02X} read from {:p} (stack_index={}, is_open={}). Likely dangling pointer!",
-            val.tt(),
-            self.v,
-            self.stack_index,
-            self.is_open()
-        );
-        val
+        self.v.get()
     }
 
     /// Get reference to the value with **zero branching**.
     #[inline(always)]
     pub fn get_value_ref(&self) -> &LuaValue {
-        debug_assert!(!self.v.is_null(), "upvalue get_value_ref: null pointer");
-        unsafe { &*self.v }
+        self.v.get_ref()
     }
 
     /// Set the value with **zero branching** — single pointer write.
     #[inline(always)]
     pub fn set_value(&mut self, val: LuaValue) {
-        debug_assert!(!self.v.is_null(), "upvalue set_value: null pointer");
-        debug_assert!(
-            (self.v as usize) > 0x10000,
-            "upvalue set_value: suspiciously low pointer {:p} (stack_index={})",
-            self.v,
-            self.stack_index
-        );
-        unsafe { *self.v = val }
+        self.v.write(&val);
     }
 
     /// Set the value by raw parts to avoid constructing a temporary LuaValue.
     #[inline(always)]
     pub fn set_value_parts(&mut self, value: Value, tt: u8) {
-        debug_assert!(!self.v.is_null(), "upvalue set_value_parts: null pointer");
-        debug_assert!(
-            (self.v as usize) > 0x10000,
-            "upvalue set_value_parts: suspiciously low pointer {:p} (stack_index={})",
-            self.v,
-            self.stack_index
-        );
-        unsafe {
-            (*self.v).value = value;
-            (*self.v).tt = tt;
-        }
-    }
-
-    /// Check if a type tag is valid (used for dangling pointer detection)
-    fn is_valid_tt(tt: u8) -> bool {
-        use crate::lua_value::lua_value::*;
-        matches!(
-            tt,
-            LUA_VNIL
-                | LUA_VEMPTY
-                | LUA_VABSTKEY
-                | LUA_VFALSE
-                | LUA_VTRUE
-                | LUA_VNUMINT
-                | LUA_VNUMFLT
-                | LUA_VSHRSTR
-                | LUA_VLNGSTR
-                | LUA_VTABLE
-                | LUA_VFUNCTION
-                | LUA_CCLOSURE
-                | LUA_VLCF
-                | LUA_VLIGHTUSERDATA
-                | LUA_VUSERDATA
-                | LUA_VTHREAD
-        )
+        self.v.write_parts(tt, value);
     }
 
     pub fn get_closed_value(&self) -> Option<&LuaValue> {