Merge pull request #3311 from Skgland/cleanup2

some cleanup and fixes

src/arena.rs

@@ -283,7 +283,10 @@ pub trait ArenaAllocated {
         Self::Payload: Sized,
     {
         TypedArenaPtr(NonNull::new_unchecked(
-            ptr.payload_offset().cast_mut().cast::<Self::Payload>(),
+            ptr.get_ptr()
+                .byte_add(Self::header_offset_from_payload())
+                .cast_mut()
+                .cast::<Self::Payload>(),
         ))
     }
 
@@ -633,16 +636,22 @@ mod tests {
     #[test]
     fn heap_cell_value_const_cast() {
         let mut wam = MockWAM::new();
+
         #[cfg(target_pointer_width = "32")]
-        let const_value = HeapCellValue::from(ConsPtr::build_with(
+        assert_eq!(ConsPtr::NICHE_SHIFT, 0);
-            std::ptr::without_provenance(0x0000_0431),
+
-            ConsPtrMaskTag::Cons,
+        #[cfg(not(target_pointer_width = "32"))]
-        ));
+        assert_eq!(ConsPtr::NICHE_SHIFT, 3);
+
+        #[cfg(target_pointer_width = "32")]
+        let dummy_ptr: *const ArenaHeader = std::ptr::without_provenance(0x0000_0438);
+
         #[cfg(target_pointer_width = "64")]
-        let const_value = HeapCellValue::from(ConsPtr::build_with(
+        let dummy_ptr: *const ArenaHeader = std::ptr::without_provenance(0x0000_5555_ff00_0438);
-            std::ptr::without_provenance(0x0000_5555_ff00_0431),
+
-            ConsPtrMaskTag::Cons,
+        assert!(dummy_ptr.is_aligned());
-        ));
+
+        let const_value = HeapCellValue::from_arena_header_ptr(dummy_ptr);
 
         match const_value.to_untyped_arena_ptr() {
             Some(arena_ptr) => {
@@ -657,8 +666,7 @@ mod tests {
         }
 
         let stream = Stream::from_static_string("test", &mut wam.machine_st.arena);
-        let stream_cell =
+        let stream_cell = HeapCellValue::from_arena_header_ptr(stream.as_ptr());
-            HeapCellValue::from(ConsPtr::build_with(stream.as_ptr(), ConsPtrMaskTag::Cons));
 
         match stream_cell.to_untyped_arena_ptr() {
             Some(arena_ptr) => {
@@ -727,7 +735,7 @@ mod tests {
             Some(untyped_arena_ptr) => {
                 assert_eq!(
                     Some(big_rat_ptr.header_ptr()),
-                    Some(untyped_arena_ptr.into()),
+                    Some(untyped_arena_ptr.get_ptr()),
                 );
             }
             None => {

src/debray_allocator.rs

@@ -15,6 +15,7 @@ use indexmap::IndexMap;
 use std::cell::Cell;
 use std::collections::VecDeque;
 use std::ops::{Deref, DerefMut};
+use std::sync::Arc;
 
 pub type BranchHits = IndexMap<usize, BitVec, FxBuildHasher>; // key: var_num, value: branch arm occurrences.
 
@@ -25,12 +26,12 @@ pub struct BranchOccurrences {
     pub deep_safety: BitSet<usize>,
     pub num_branches: usize,
     pub current_branch_idx: usize,
-    pub current_branch_num: BranchNumber,
+    pub current_branch_num: Arc<BranchNumber>,
     pub subsumed_hits: SubsumedBranchHits,
 }
 
 impl BranchOccurrences {
-    fn new(current_branch_num: BranchNumber, num_branches: usize) -> Self {
+    fn new(current_branch_num: Arc<BranchNumber>, num_branches: usize) -> Self {
         Self {
             hits: BranchHits::with_hasher(FxBuildHasher::default()),
             shallow_safety: BitSet::default(),
@@ -98,7 +99,7 @@ impl BranchStack {
         }
     }
 
-    pub(crate) fn add_branch_stack(&mut self, branch_num: BranchNumber, num_branches: usize) {
+    pub(crate) fn add_branch_stack(&mut self, branch_num: Arc<BranchNumber>, num_branches: usize) {
         self.push(BranchOccurrences::new(branch_num, num_branches));
     }
 
@@ -112,7 +113,7 @@ impl BranchStack {
     }
 
     #[inline]
-    pub(crate) fn incr_current_branch(&mut self, branch_num: BranchNumber) {
+    pub(crate) fn incr_current_branch(&mut self, branch_num: Arc<BranchNumber>) {
         let branch_occurrences = self.last_mut().unwrap();
         branch_occurrences.current_branch_idx += 1;
         branch_occurrences.current_branch_num = branch_num;
@@ -201,7 +202,7 @@ impl DebrayAllocator {
             },
         );
 
-        let branch_designator = self.branch_stack.current_branch_designator();
+        let branch_designator = Arc::new(self.branch_stack.current_branch_designator());
 
         let (deep_safety, shallow_safety) = match self.branch_stack.last_mut() {
             Some(latest_branch) => {
@@ -506,7 +507,7 @@ impl DebrayAllocator {
     }
 
     pub(crate) fn mark_safe_var_unconditionally(&mut self, var_num: usize) {
-        let branch_designator = self.branch_stack.current_branch_designator();
+        let branch_designator = Arc::new(self.branch_stack.current_branch_designator());
 
         match &mut self.var_data.records[var_num].allocation {
             VarAlloc::Perm(
@@ -530,7 +531,7 @@ impl DebrayAllocator {
     }
 
     fn mark_safe_var(&mut self, var_num: usize, lvl: Level, term_loc: GenContext) {
-        let branch_designator = self.branch_stack.current_branch_designator();
+        let branch_designator = Arc::new(self.branch_stack.current_branch_designator());
 
         match &mut self.var_data.records[var_num].allocation {
             VarAlloc::Perm(
@@ -574,7 +575,7 @@ impl DebrayAllocator {
         r: RegType,
         arg_c: usize,
     ) -> Instruction {
-        let branch_designator = self.branch_stack.current_branch_designator();
+        let branch_designator = Arc::new(self.branch_stack.current_branch_designator());
 
         match &mut self.var_data.records[var_num].allocation {
             VarAlloc::Perm(
@@ -610,7 +611,7 @@ impl DebrayAllocator {
         var_num: usize,
         r: RegType,
     ) -> Instruction {
-        let branch_designator = self.branch_stack.current_branch_designator();
+        let branch_designator = Arc::new(self.branch_stack.current_branch_designator());
 
         match &mut self.var_data.records[var_num].allocation {
             VarAlloc::Perm(

src/forms.rs

@@ -25,6 +25,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::ops::{AddAssign, Deref, DerefMut};
 use std::path::PathBuf;
+use std::sync::Arc;
 
 pub type PredicateKey = (Atom, usize); // name, arity.
 
@@ -194,7 +195,7 @@ impl BranchNumber {
 #[derive(Debug)]
 pub enum ChunkedTerms {
     Branch {
-        branch_nums: Vec<BranchNumber>,
+        branch_nums: Vec<Arc<BranchNumber>>,
         arms: Vec<VecDeque<ChunkedTerms>>,
     },
     Chunk {

src/iterators.rs

@@ -7,6 +7,7 @@ use std::cell::Cell;
 use std::collections::VecDeque;
 use std::iter::*;
 use std::rc::Rc;
+use std::sync::Arc;
 use std::vec::Vec;
 
 #[allow(clippy::borrowed_box)]
@@ -320,7 +321,7 @@ pub(crate) fn breadth_first_iter(
 enum ClauseIteratorState<'a> {
     RemainingChunks(&'a VecDeque<ChunkedTerms>, usize),
     RemainingBranches(
-        &'a Vec<BranchNumber>,
+        &'a Vec<Arc<BranchNumber>>,
         &'a Vec<VecDeque<ChunkedTerms>>,
         usize,
     ),
@@ -329,11 +330,11 @@ enum ClauseIteratorState<'a> {
 #[derive(Debug, Clone)]
 pub(crate) enum ClauseItem<'a> {
     FirstBranch {
-        branch_num: &'a BranchNumber,
+        branch_num: &'a Arc<BranchNumber>,
         num_branches: usize,
     },
     NextBranch {
-        branch_num: &'a BranchNumber,
+        branch_num: &'a Arc<BranchNumber>,
     },
     BranchEnd {
         depth: usize,

src/machine/disjuncts.rs

@@ -15,6 +15,7 @@ use std::cell::Cell;
 use std::collections::VecDeque;
 use std::hash::Hash;
 use std::ops::{Deref, DerefMut};
+use std::sync::Arc;
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct VarInfo {
@@ -34,12 +35,12 @@ pub struct ChunkInfo {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct BranchInfo {
-    branch_num: BranchNumber,
+    branch_num: Arc<BranchNumber>,
     chunks: Vec<ChunkInfo>,
 }
 
 impl BranchInfo {
-    fn new(branch_num: BranchNumber) -> Self {
+    fn new(branch_num: Arc<BranchNumber>) -> Self {
         Self {
             branch_num,
             chunks: vec![],
@@ -68,7 +69,7 @@ impl DerefMut for BranchMap {
     }
 }
 
-type RootSet = IndexSet<BranchNumber>;
+type RootSet = IndexSet<Arc<BranchNumber>>;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct ClassifyInfo {
@@ -92,14 +93,14 @@ enum TraversalState {
     Term(Term),
     OverrideGlobalCutVar(usize),
     ResetGlobalCutVarOverride(Option<usize>),
-    AddBranchNum(BranchNumber), // set current_branch_num, add it to the root set
+    AddBranchNum(Arc<BranchNumber>), // set current_branch_num, add it to the root set
-    RepBranchNum(BranchNumber), // replace current_branch_num and the latest in the root set
+    RepBranchNum(Arc<BranchNumber>), // replace current_branch_num and the latest in the root set
 }
 
 #[derive(Debug)]
 pub struct VariableClassifier {
     call_policy: CallPolicy,
-    current_branch_num: BranchNumber,
+    current_branch_num: Arc<BranchNumber>,
     current_chunk_num: usize,
     current_chunk_type: ChunkType,
     branch_map: BranchMap,
@@ -156,22 +157,26 @@ pub type ClassifyFactResult = (Term, VarData);
 pub type ClassifyRuleResult = (Term, ChunkedTermVec, VarData);
 
 fn merge_branch_seq(branches: impl Iterator<Item = BranchInfo>) -> BranchInfo {
-    let mut branch_info = BranchInfo::new(BranchNumber::default());
+    let mut branch_info = BranchInfo::new(Arc::new(BranchNumber::default()));
 
     for mut branch in branches {
         branch_info.branch_num = branch.branch_num;
         branch_info.chunks.append(&mut branch.chunks);
     }
 
-    branch_info.branch_num.delta = branch_info.branch_num.delta * Integer::from(2);
+    let new_delta = branch_info.branch_num.delta.clone() * Integer::from(2);
-    branch_info.branch_num.branch_num -= &branch_info.branch_num.delta;
+
+    branch_info.branch_num = Arc::new(BranchNumber {
+        branch_num: branch_info.branch_num.branch_num.clone() - &new_delta,
+        delta: new_delta,
+    });
 
     branch_info
 }
 
 fn flatten_into_disjunct(
     build_stack: &mut ChunkedTermVec,
-    branch_num: BranchNumber,
+    branch_num: Arc<BranchNumber>,
     preceding_len: usize,
 ) {
     let branch_vec = build_stack.drain(preceding_len + 1..).collect();
@@ -188,7 +193,7 @@ impl VariableClassifier {
     pub fn new(call_policy: CallPolicy) -> Self {
         Self {
             call_policy,
-            current_branch_num: BranchNumber::default(),
+            current_branch_num: Arc::new(BranchNumber::default()),
             current_chunk_num: 0,
             current_chunk_type: ChunkType::Head,
             branch_map: BranchMap(BranchMapInt::new()),
@@ -542,7 +547,7 @@ impl VariableClassifier {
                             let tail = terms.pop().unwrap();
                             let head = terms.pop().unwrap();
 
-                            let first_branch_num = self.current_branch_num.split();
+                            let first_branch_num = Arc::new(self.current_branch_num.split());
                             let branches: Vec<_> = std::iter::once(head)
                                 .chain(unfold_by_str(tail, atom!(";")).into_iter())
                                 .collect();
@@ -553,18 +558,18 @@ impl VariableClassifier {
                                 let succ_branch_number = branch_numbers[idx - 1].incr_by_delta();
 
                                 branch_numbers.push(if idx + 1 < branches.len() {
-                                    succ_branch_number.split()
+                                    Arc::new(succ_branch_number.split())
                                 } else {
-                                    succ_branch_number
+                                    Arc::new(succ_branch_number)
                                 });
                             }
 
                             let build_stack_len = build_stack.len();
                             build_stack.reserve_branch(branches.len());
 
-                            state_stack.push(TraversalState::RepBranchNum(
+                            state_stack.push(TraversalState::RepBranchNum(Arc::new(
                                 self.current_branch_num.halve_delta(),
-                            ));
+                            )));
 
                             let iter = branches.into_iter().zip(branch_numbers.into_iter());
                             let final_disjunct_loc = state_stack.len();
@@ -619,14 +624,14 @@ impl VariableClassifier {
                             let not_term = terms.pop().unwrap();
                             let build_stack_len = build_stack.len();
 
-                            let first_branch_num = self.current_branch_num.split();
+                            let first_branch_num = Arc::new(self.current_branch_num.split());
-                            let second_branch_num = first_branch_num.incr_by_delta();
+                            let second_branch_num = Arc::new(first_branch_num.incr_by_delta());
 
                             build_stack.reserve_branch(2);
 
-                            state_stack.push(TraversalState::RepBranchNum(
+                            state_stack.push(TraversalState::RepBranchNum(Arc::new(
                                 self.current_branch_num.halve_delta(),
-                            ));
+                            )));
                             state_stack.push(TraversalState::BuildFinalDisjunct(build_stack_len));
                             state_stack.push(TraversalState::Term(Term::Clause(
                                 Cell::default(),

src/machine/gc.rs

@@ -690,8 +690,7 @@ mod tests {
 
         // term is: [a, <stream ptr>]
         let stream = Stream::from_static_string("test", &mut wam.machine_st.arena);
-        let stream_cell =
+        let stream_cell = HeapCellValue::from_arena_header_ptr(stream.as_ptr());
-            HeapCellValue::from(ConsPtr::build_with(stream.as_ptr(), ConsPtrMaskTag::Cons));
 
         let mut writer = wam.machine_st.heap.reserve(16).unwrap();
 

src/machine/streams.rs

@@ -23,7 +23,7 @@ use std::fmt::Debug;
 use std::fs::{File, OpenOptions};
 use std::hash::Hash;
 use std::io;
-use std::io::{Cursor, ErrorKind, IsTerminal, Read, Seek, SeekFrom, Write};
+use std::io::{Cursor, ErrorKind, Read, Seek, SeekFrom, Write};
 use std::mem::ManuallyDrop;
 use std::net::{Shutdown, TcpStream};
 use std::ops::{Deref, DerefMut};
@@ -657,6 +657,8 @@ impl Stream {
 
     #[inline]
     pub fn stdin(arena: &mut Arena, add_history: bool) -> Stream {
+        #[cfg(unix)]
+        use std::io::IsTerminal;
         #[cfg(unix)]
         if !std::io::stdin().is_terminal() {
             use std::os::unix::io::{FromRawFd, RawFd};

src/macros.rs

@@ -139,12 +139,7 @@ macro_rules! typed_arena_ptr_as_cell {
 
 macro_rules! raw_ptr_as_cell {
     ($ptr:expr) => {{
-        // Cell is 64-bit, but raw ptr is 32-bit in 32-bit systems
+        HeapCellValue::from_arena_header_ptr($ptr)
-        let ptr: *const _ = $ptr;
-        // This needs to expose provenance because it needs to be turned back into a pointer
-        // in contexts where there is no available provenance locally. For example, in
-        // `ConsPtr::as_ptr`.
-        HeapCellValue::from_ptr_addr(ptr.expose_provenance())
     }};
 }
 

src/types.rs

@@ -16,6 +16,7 @@ use std::ops::{Add, Sub, SubAssign};
 
 use dashu::{Integer, Rational};
 
+// Variant tag MUST be odd for all but Cons
 #[derive(Specifier, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 #[repr(u8)]
 #[bits = 6]
@@ -43,33 +44,6 @@ pub enum HeapCellValueTag {
     TrailedBlackboardOffset = 0b110001,
 }
 
-#[derive(Specifier, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-#[repr(u8)]
-#[bits = 6]
-pub enum HeapCellValueView {
-    Str = 0b000001,
-    Lis = 0b000101,
-    Var = 0b001011,
-    StackVar = 0b001101,
-    AttrVar = 0b010001,
-    PStrLoc = 0b010011,
-    // constants.
-    Cons = 0b0,
-    F64Offset = 0b010101,
-    Fixnum = 0b011001,
-    CodeIndexOffset = 0b011011,
-    Atom = 0b011111,
-    CutPoint = 0b011101,
-    // trail elements.
-    TrailedHeapVar = 0b100001,
-    TrailedStackVar = 0b100011,
-    TrailedAttrVar = 0b100101,
-    TrailedAttrVarListLink = 0b101001,
-    TrailedAttachedValue = 0b101011,
-    TrailedBlackboardEntry = 0b101101,
-    TrailedBlackboardOffset = 0b110001,
-}
-
 #[derive(Specifier, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 #[bits = 1]
 pub enum ConsPtrMaskTag {
@@ -88,19 +62,46 @@ pub struct ConsPtr {
 }
 
 impl ConsPtr {
+    // ConstPtr.ptr has 61 bits if usize on the current arch is larger than that
+    // use the niche provided by the alignment of ArenaHeader.
+    // The niece is that the log2(alignment) least significant bits are always 0, so we can shift the
+    // address down by that many places without losing information.
+    // That way with an alignment of 8 a 64-bit address fits into 61-bits after shifting it down by 3 places
+    // and as a result be can store it without loss in ConsPtr.ptr
+    pub(crate) const NICHE_SHIFT: u32 = if usize::BITS > 61 {
+        std::mem::align_of::<ArenaHeader>().ilog2()
+    } else {
+        0
+    };
+
     #[inline(always)]
-    pub fn build_with(ptr: *const ArenaHeader, tag: ConsPtrMaskTag) -> Self {
+    pub fn from_ptr(ptr: *const ArenaHeader) -> Self {
+        Self::build_with(ptr, ConsPtrMaskTag::Cons)
+    }
+
+    #[inline(always)]
+    fn build_with(ptr: *const ArenaHeader, tag: ConsPtrMaskTag) -> Self {
+        let mut addr = u64::try_from(ptr.expose_provenance())
+            .expect("pointer address {ptr:p} should fit into u64");
+
+        debug_assert_eq!(addr % std::mem::align_of::<ArenaHeader>() as u64, 0);
+
+        addr >>= Self::NICHE_SHIFT;
+
         ConsPtr::new()
-            .with_ptr(ptr.expose_provenance() as u64)
+            .with_ptr(addr)
             .with_f(false)
             .with_m(false)
             .with_tag(tag)
     }
 
     #[inline(always)]
-    pub fn as_ptr(self) -> *mut u8 {
+    pub fn as_ptr(self) -> *const ArenaHeader {
-        let addr: u64 = self.ptr();
+        let mut addr: u64 = self.ptr();
-        std::ptr::with_exposed_provenance_mut(addr as usize)
+
+        addr <<= Self::NICHE_SHIFT;
+
+        std::ptr::with_exposed_provenance(addr as usize)
     }
 
     #[inline(always)]
@@ -363,7 +364,7 @@ where
 {
     #[inline]
     fn from(arena_ptr: TypedArenaPtr<T>) -> HeapCellValue {
-        HeapCellValue::from(arena_ptr.header_ptr().expose_provenance() as u64)
+        HeapCellValue::from_arena_header_ptr(arena_ptr.header_ptr())
     }
 }
 
@@ -384,18 +385,6 @@ impl From<CodeIndexOffset> for HeapCellValue {
     }
 }
 
-impl From<ConsPtr> for HeapCellValue {
-    #[inline(always)]
-    fn from(cons_ptr: ConsPtr) -> HeapCellValue {
-        HeapCellValue::from_bytes(
-            ConsPtr::from(cons_ptr.as_ptr().expose_provenance() as u64)
-                .with_tag(ConsPtrMaskTag::Cons)
-                .with_m(false)
-                .into_bytes(),
-        )
-    }
-}
-
 impl From<(Number, &mut Arena)> for HeapCellValue {
     #[inline(always)]
     fn from((n, arena): (Number, &mut Arena)) -> HeapCellValue {
@@ -585,12 +574,12 @@ impl HeapCellValue {
     }
 
     #[inline]
-    pub fn from_ptr_addr(ptr_bytes: usize) -> Self {
+    pub fn from_arena_header_ptr(ptr: *const ArenaHeader) -> Self {
-        HeapCellValue::from_bytes((ptr_bytes as u64).to_ne_bytes())
+        HeapCellValue::from_bytes(ConsPtr::from_ptr(ptr).into_bytes())
     }
 
-    pub fn to_ptr_addr(self) -> usize {
+    pub fn to_arena_header_ptr(self) -> *const ArenaHeader {
-        u64::from_ne_bytes(self.into_bytes()) as usize
+        ConsPtr::from_bytes(self.into_bytes()).as_ptr()
     }
 
     #[inline]
@@ -734,21 +723,14 @@ const_assert!(mem::size_of::<UntypedArenaPtr>() == 8);
 impl From<*const ArenaHeader> for UntypedArenaPtr {
     #[inline]
     fn from(ptr: *const ArenaHeader) -> UntypedArenaPtr {
-        UntypedArenaPtr::build_with(ptr.expose_provenance())
+        UntypedArenaPtr::from_bytes(ConsPtr::from_ptr(ptr).into_bytes())
-    }
-}
-
-impl From<*const IndexPtr> for UntypedArenaPtr {
-    #[inline]
-    fn from(ptr: *const IndexPtr) -> UntypedArenaPtr {
-        UntypedArenaPtr::build_with(ptr.expose_provenance())
     }
 }
 
 impl From<UntypedArenaPtr> for *const ArenaHeader {
     #[inline]
     fn from(ptr: UntypedArenaPtr) -> *const ArenaHeader {
-        ptr.get_ptr().cast::<ArenaHeader>()
+        ptr.get_ptr()
     }
 }
 
@@ -759,9 +741,8 @@ impl UntypedArenaPtr {
     }
 
     #[inline]
-    pub fn get_ptr(self) -> *const u8 {
+    pub fn get_ptr(self) -> *const ArenaHeader {
-        let addr: u64 = self.ptr();
+        ConsPtr::from_bytes(self.into_bytes()).as_ptr()
-        std::ptr::with_exposed_provenance(addr as usize)
     }
 
     #[inline]
@@ -773,11 +754,6 @@ impl UntypedArenaPtr {
         }
     }
 
-    #[inline]
-    pub fn payload_offset(self) -> *const u8 {
-        unsafe { self.get_ptr().add(size_of::<ArenaHeader>()) }
-    }
-
     /// # Safety
     /// - this UntypedArenaPtr actual pointee type is T
     /// - the pointer must be non-null

src/variable_records.rs

@@ -7,6 +7,7 @@ use indexmap::{IndexMap, IndexSet};
 use num_order::NumOrd;
 
 use std::ops::{Deref, DerefMut};
+use std::sync::Arc;
 
 #[derive(Debug, Clone)]
 pub struct TempVarData {
@@ -17,7 +18,7 @@ pub struct TempVarData {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct BranchDesignator {
-    pub branch_num: BranchNumber,
+    pub branch_num: Arc<BranchNumber>,
 }
 
 impl BranchDesignator {