From: Mark Thom <markjordanthom@gmail.com>
Date: Sat, 30 Nov 2019 21:08:02 +0000 (-0700)
Subject: pop AND frames when safe to do so, suspend resizing of AND frames until a proper... 
X-Git-Tag: v0.8.118~36^2~16
X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=0eb20a5d8e6e7462267f58f731bb05cbd0bf1aae;p=scryer-prolog.git

pop AND frames when safe to do so, suspend resizing of AND frames until a proper GC is implemented (#244)
---

diff --git a/src/prolog/machine/and_stack.rs b/src/prolog/machine/and_stack.rs
index 3a16f5c0..002e6f68 100644
--- a/src/prolog/machine/and_stack.rs
+++ b/src/prolog/machine/and_stack.rs
@@ -55,6 +55,10 @@ impl AndStack {
         self.0.clear()
     }
 
+    /* 
+
+    // See MachineState::allocate for why this is commented out.
+
     pub fn resize(&mut self, fr: usize, n: usize) {
         let len = self[fr].perms.len();
 
@@ -66,7 +70,8 @@ impl AndStack {
             }
         }
     }
-
+    */
+    
     #[inline]
     pub fn truncate(&mut self, len: usize) {
         self.0.truncate(len);
diff --git a/src/prolog/machine/machine_state.rs b/src/prolog/machine/machine_state.rs
index 20162ac1..fd506f3c 100644
--- a/src/prolog/machine/machine_state.rs
+++ b/src/prolog/machine/machine_state.rs
@@ -423,8 +423,6 @@ pub(crate) trait CallPolicy: Any {
         machine_st.e = machine_st.or_stack[b].e;
         machine_st.cp = machine_st.or_stack[b].cp.clone();
 
-        machine_st.pop_stack_frames();
-        
         machine_st.or_stack[b].bp = machine_st.p.clone() + offset;
 
         let old_tr = machine_st.or_stack[b].tr;
@@ -471,8 +469,6 @@ pub(crate) trait CallPolicy: Any {
         machine_st.e = machine_st.or_stack[b].e;
         machine_st.cp = machine_st.or_stack[b].cp.clone();
 
-        machine_st.pop_stack_frames();
-        
         machine_st.or_stack[b].bp = machine_st.p.clone() + 1;
 
         let old_tr = machine_st.or_stack[b].tr;
@@ -515,6 +511,8 @@ pub(crate) trait CallPolicy: Any {
             machine_st.registers[i] = machine_st.or_stack[b][i].clone();
         }
 
+        machine_st.pop_stack_frames();
+
         machine_st.num_of_args = n;
         machine_st.e = machine_st.or_stack[b].e;
         machine_st.cp = machine_st.or_stack[b].cp.clone();
@@ -548,8 +546,6 @@ pub(crate) trait CallPolicy: Any {
         machine_st.b = machine_st.or_stack[b].b;
         machine_st.or_stack.truncate(machine_st.b);
 
-        machine_st.pop_stack_frames();
-
         machine_st.hb = machine_st.heap.h;
         machine_st.p += offset;
 
@@ -564,6 +560,8 @@ pub(crate) trait CallPolicy: Any {
             machine_st.registers[i] = machine_st.or_stack[b][i].clone();
         }
 
+        machine_st.pop_stack_frames();
+
         machine_st.num_of_args = n;
         machine_st.e = machine_st.or_stack[b].e;
         machine_st.cp = machine_st.or_stack[b].cp.clone();
@@ -597,8 +595,6 @@ pub(crate) trait CallPolicy: Any {
         machine_st.b = machine_st.or_stack[b].b;
         machine_st.or_stack.truncate(machine_st.b);
 
-        machine_st.pop_stack_frames();
-
         machine_st.hb = machine_st.heap.h;
         machine_st.p += 1;
 
diff --git a/src/prolog/machine/machine_state_impl.rs b/src/prolog/machine/machine_state_impl.rs
index f28eb077..df22f685 100644
--- a/src/prolog/machine/machine_state_impl.rs
+++ b/src/prolog/machine/machine_state_impl.rs
@@ -123,7 +123,7 @@ impl MachineState {
         self.flags
     }
 
-    fn next_global_index(&self) -> usize {
+    pub(super) fn next_global_index(&self) -> usize {
         max(
             if self.and_stack.len() > 0 {
                 self.and_stack[self.e].global_index
@@ -3127,10 +3127,18 @@ impl MachineState {
 
     pub(super) fn allocate(&mut self, num_cells: usize) {
         let gi = self.next_global_index();
+//        let new_e = self.e + 1;
 
         self.p += 1;
 
-        if self.e + 1 < self.and_stack.len() {
+/*
+        /* See issue #244 for an example of a program broken (at the 
+           top level) by the inclusion of this code. A proper GC must determine if an 
+           existing AND frame is safe to resize; the check here is not 
+           enough.
+        */
+
+        if new_e < self.and_stack.len() {
             let and_gi = self.and_stack[self.e].global_index;
             let or_gi = self
                 .or_stack
@@ -3139,10 +3147,8 @@ impl MachineState {
                 .unwrap_or(0);
 
             if and_gi > or_gi {
-                let new_e = self.e + 1;
-
                 self.and_stack[new_e].e = self.e;
-                self.and_stack[new_e].cp = self.cp.clone();
+                self.and_stack[new_e].cp = self.cp;
                 self.and_stack[new_e].global_index = gi;
 
                 self.and_stack.resize(new_e, num_cells);
@@ -3151,7 +3157,7 @@ impl MachineState {
                 return;
             }
         }
-
+*/
         self.and_stack.push(gi, self.e, self.cp.clone(), num_cells);
         self.e = self.and_stack.len() - 1;
     }
@@ -3353,7 +3359,7 @@ impl MachineState {
                 self.b = self.or_stack.len();
                 let b = self.b - 1;
 
-                for i in 1..n + 1 {
+                for i in 1 .. n + 1 {
                     self.or_stack[b][i] = self.registers[i].clone();
                 }
 
diff --git a/src/prolog/toplevel.pl b/src/prolog/toplevel.pl
index 63433554..4700806a 100644
--- a/src/prolog/toplevel.pl
+++ b/src/prolog/toplevel.pl
@@ -32,7 +32,7 @@
     ;  !,
        catch(throw(error(type_error(atom, Item), repl/0)),
 	     E,
-	     '$print_exception_with_check'(E))       
+	     '$print_exception_with_check'(E))
     ).
 '$instruction_match'(Term, VarList) :-
     '$submit_query_and_print_results'(Term, VarList),
@@ -42,7 +42,9 @@
     (  expand_goals(Term0, Term) -> true
     ;  Term = Term0
     ),
-    (  '$get_b_value'(B), call(Term), '$write_eqs_and_read_input'(B, VarList), !
+    (  '$get_b_value'(B), call(Term),
+       '$write_eqs_and_read_input'(B, VarList),
+       !
     ;  write('false.'), nl
     ).
 
@@ -79,7 +81,7 @@
        (  '$needs_bracketing'(Value, (=)) ->
 	  write('('),
 	  write_term(Value, [quoted(true), variable_names(VarList)]),
-	  write(')')       	  
+	  write(')')
        ;  write_term(Value, [quoted(true), variable_names(VarList)]),
 	  (  '$trailing_period_is_ambiguous'(Value) ->
 	     write(' ')
@@ -98,7 +100,7 @@
     '$write_eq'(G2, VarList).
 '$write_eq'(G, VarList) :-
     '$write_last_goal'(G, VarList).
-    
+
 '$graphic_token_char'(C) :-
     memberchk(C, ['#', '$', '&', '*', '+', '-', '.', ('/'), ':',
                   '<', '=', '>', '?', '@', '^', '~', ('\\')]).