From: Mark Thom Date: Fri, 25 Jan 2019 06:38:53 +0000 (-0700) Subject: fix unsafe variable handling X-Git-Tag: v0.8.110~288 X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=ef3d3aa01ab4f4b7c772c49728324e3f950cf6f1;p=scryer-prolog.git fix unsafe variable handling --- diff --git a/Cargo.lock b/Cargo.lock index 0179cf3d..eeba3966 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -108,7 +108,7 @@ dependencies = [ [[package]] name = "rusty-wam" -version = "0.7.16" +version = "0.7.17" dependencies = [ "downcast 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)", "num 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/prolog/and_stack.rs b/src/prolog/and_stack.rs index e012b631..520fe452 100644 --- a/src/prolog/and_stack.rs +++ b/src/prolog/and_stack.rs @@ -46,18 +46,18 @@ impl AndStack { pub fn clear(&mut self) { self.0.clear() } - - pub fn resize(&mut self, fr: usize, n: usize) { - let len = self[fr].perms.len(); - + + pub fn resize(&mut self, fr: usize, n: usize) { + let len = self[fr].perms.len(); + if len < n { self[fr].perms.reserve(n - len); - for i in len .. n { + for i in len .. n { self[fr].perms.push(Addr::StackCell(fr, i)); } } - } + } } impl Index for AndStack { diff --git a/src/prolog/codegen.rs b/src/prolog/codegen.rs index 66e94f1e..9409bf46 100644 --- a/src/prolog/codegen.rs +++ b/src/prolog/codegen.rs @@ -43,6 +43,30 @@ impl<'a> ConjunctInfo<'a> fn perm_var_offset(&self) -> usize { self.has_deep_cut as usize } + + fn mark_unsafe_vars(&self, mut unsafe_var_marker: UnsafeVarMarker, code: &mut Code) { + // target the last goal of the rule for handling unsafe variables. + // we use this weird logic to find the last goal. + let index = if let &Line::Control(_) = code.last().unwrap() { + code.len() - 2 + } else { + code.len() - 1 + }; + + if let Line::Query(_) = &code[index] { + unsafe_var_marker.record_unsafe_vars(&self.perm_vs); + + for line in code.iter_mut() { + if let &mut Line::Query(ref mut query) = line { + unsafe_var_marker.mark_safe_vars(query); + } + } + } + + if let &mut Line::Query(ref mut query) = &mut code[index] { + unsafe_var_marker.mark_unsafe_vars(query); + } + } } impl<'a, TermMarker: Allocator<'a>> CodeGenerator @@ -544,9 +568,12 @@ impl<'a, TermMarker: Allocator<'a>> CodeGenerator self.compile_seq_prelude(&conjunct_info, &mut code); let iter = FactIterator::from_rule_head_clause(args); - let fact = self.compile_target(iter, GenContext::Head, false); + let mut fact = self.compile_target(iter, GenContext::Head, false); + + let mut unsafe_var_marker = UnsafeVarMarker::new(); if !fact.is_empty() { + unsafe_var_marker = self.mark_unsafe_fact_vars(&mut fact); code.push(Line::Fact(fact)); } @@ -554,23 +581,14 @@ impl<'a, TermMarker: Allocator<'a>> CodeGenerator try!(self.compile_seq(iter, &conjunct_info, &mut code, false)); if conjunct_info.allocates() { - let index = if let &Line::Control(_) = code.last().unwrap() { - code.len() - 2 - } else { - code.len() - 1 - }; - - if let &mut Line::Query(ref mut query) = &mut code[index] { - let head_iter = FactIterator::from_rule_head_clause(args); - conjunct_info.perm_vs.mark_unsafe_vars_in_rule(head_iter, query); - } + conjunct_info.mark_unsafe_vars(unsafe_var_marker, &mut code); } Self::compile_cleanup(&mut code, &conjunct_info, clauses.last().unwrap_or(p1)); Ok(code) } - fn mark_unsafe_fact_vars(&self, fact: &mut CompiledFact) + fn mark_unsafe_fact_vars(&self, fact: &mut CompiledFact) -> UnsafeVarMarker { let mut unsafe_vars = HashMap::new(); @@ -587,14 +605,15 @@ impl<'a, TermMarker: Allocator<'a>> CodeGenerator *fact_instr = FactInstruction::UnifyLocalValue(reg); } }, - &mut FactInstruction::UnifyVariable(reg) => { + &mut FactInstruction::UnifyVariable(reg) => if let Some(found) = unsafe_vars.get_mut(®) { *found = true; - } - }, + }, _ => {} }; } + + UnsafeVarMarker { unsafe_vars } } pub fn compile_fact<'b: 'a>(&mut self, term: &'b Term) -> Code @@ -653,15 +672,7 @@ impl<'a, TermMarker: Allocator<'a>> CodeGenerator try!(self.compile_seq(iter, &conjunct_info, &mut code, true)); if conjunct_info.allocates() { - let index = if let &Line::Control(_) = code.last().unwrap() { - code.len() - 2 - } else { - code.len() - 1 - }; - - if let &mut Line::Query(ref mut query) = &mut code[index] { - conjunct_info.perm_vs.mark_unsafe_vars_in_query(query); - } + conjunct_info.mark_unsafe_vars(UnsafeVarMarker::new(), &mut code); } if let Some(query_term) = query.last() { diff --git a/src/prolog/compile.rs b/src/prolog/compile.rs index 6cd23f92..6160489f 100644 --- a/src/prolog/compile.rs +++ b/src/prolog/compile.rs @@ -122,7 +122,7 @@ fn compile_query(terms: Vec, queue: VecDeque, flags: Machin let mut code = try!(cg.compile_query(&terms)); compile_appendix(&mut code, &queue, false, flags)?; - + Ok((code, cg.take_vars())) } diff --git a/src/prolog/fixtures.rs b/src/prolog/fixtures.rs index 1cbcd713..71805c5c 100644 --- a/src/prolog/fixtures.rs +++ b/src/prolog/fixtures.rs @@ -174,25 +174,58 @@ impl<'a> VariableFixtures<'a> } } } +} + +pub struct UnsafeVarMarker { + pub unsafe_vars: HashMap +} + +impl UnsafeVarMarker { + pub fn new() -> Self { + UnsafeVarMarker { + unsafe_vars: HashMap::new() + } + } + + pub fn record_unsafe_vars(&mut self, fixtures: &VariableFixtures) { + for &(_, ref cb) in fixtures.values() { + if let Some(index) = cb.first() { + if !self.unsafe_vars.contains_key(&index.get().norm()) { + self.unsafe_vars.insert(index.get().norm(), false); + } + } + } + } + + pub fn mark_safe_vars(&mut self, query: &mut CompiledQuery) { + for query_instr in query.iter_mut() { + match query_instr { + &mut QueryInstruction::PutVariable(RegType::Temp(r), _) => + if let Some(found) = self.unsafe_vars.get_mut(&RegType::Temp(r)) { + *found = true; + }, + &mut QueryInstruction::SetVariable(reg) => + if let Some(found) = self.unsafe_vars.get_mut(®) { + *found = true; + }, + _ => {} + } + } + } - fn mark_unsafe_vars(&self, unsafe_vars: &mut HashMap, query: &mut CompiledQuery) + pub fn mark_unsafe_vars(&mut self, query: &mut CompiledQuery) { for query_instr in query.iter_mut() { match query_instr { &mut QueryInstruction::PutValue(RegType::Perm(i), arg) => - if let Some(found) = unsafe_vars.get_mut(&RegType::Perm(i)) { + if let Some(found) = self.unsafe_vars.get_mut(&RegType::Perm(i)) { if !*found { *found = true; *query_instr = QueryInstruction::PutUnsafeValue(i, arg); } }, - &mut QueryInstruction::SetVariable(reg) - | &mut QueryInstruction::PutVariable(reg, _) => - if let Some(found) = unsafe_vars.get_mut(®) { - *found = true; - }, &mut QueryInstruction::SetValue(reg) => - if let Some(found) = unsafe_vars.get_mut(®) { + if let Some(found) = self.unsafe_vars.get_mut(®) { if !*found { *found = true; *query_instr = QueryInstruction::SetLocalValue(reg); @@ -202,43 +235,4 @@ impl<'a> VariableFixtures<'a> }; } } - - fn record_unsafe_vars(&self, unsafe_vars: &mut HashMap) { - for &(_, ref cb) in self.values() { - match cb.first() { - Some(index) => { - unsafe_vars.insert(index.get().norm(), false); - }, - None => {} - }; - } - } - - fn mark_head_vars_as_safe(&self, head_iter: FactIterator<'a>, unsafe_vars: &mut HashMap) - { - for term_ref in head_iter { - match term_ref { - TermRef::Var(Level::Shallow, cell, _) => { - unsafe_vars.remove(&cell.get().norm()); - }, - _ => {} - }; - } - } - - pub fn mark_unsafe_vars_in_query(&self, query: &mut CompiledQuery) { - let mut unsafe_vars = HashMap::new(); - - self.record_unsafe_vars(&mut unsafe_vars); - self.mark_unsafe_vars(&mut unsafe_vars, query); - } - - pub fn mark_unsafe_vars_in_rule(&self, head_iter: FactIterator<'a>, query: &mut CompiledQuery) - { - let mut unsafe_vars = HashMap::new(); - - self.record_unsafe_vars(&mut unsafe_vars); - self.mark_head_vars_as_safe(head_iter, &mut unsafe_vars); - self.mark_unsafe_vars(&mut unsafe_vars, query); - } } diff --git a/src/prolog/heap_print.rs b/src/prolog/heap_print.rs index 3d85657e..28e6e48f 100644 --- a/src/prolog/heap_print.rs +++ b/src/prolog/heap_print.rs @@ -628,7 +628,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> pub fn print(mut self, addr: Addr) -> Outputter { let mut iter = self.machine_st.pre_order_iter(addr); - + loop { if let Some(loc_data) = self.state_stack.pop() { match loc_data { diff --git a/src/prolog/instructions.rs b/src/prolog/instructions.rs index 21bfa59a..70c9022c 100644 --- a/src/prolog/instructions.rs +++ b/src/prolog/instructions.rs @@ -4,7 +4,6 @@ use prolog_parser::tabled_rc::*; use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap, VecDeque}; use std::cmp::Ordering; -use std::fmt; use std::ops::{Add, AddAssign, Index, IndexMut, Sub}; use std::rc::Rc; @@ -495,12 +494,12 @@ impl BuiltInClauseType { impl ClauseType { pub fn spec(&self) -> Option<(usize, Specifier)> { - match self { + match self { &ClauseType::Op(ref op_decl, _) => Some((op_decl.0, op_decl.1)), &ClauseType::Inlined(InlinedClauseType::CompareNumber(..)) | &ClauseType::BuiltIn(BuiltInClauseType::Is(..)) - | &ClauseType::BuiltIn(BuiltInClauseType::CompareTerm(_)) + | &ClauseType::BuiltIn(BuiltInClauseType::CompareTerm(_)) | &ClauseType::BuiltIn(BuiltInClauseType::NotEq) | &ClauseType::BuiltIn(BuiltInClauseType::Eq) => Some((700, XFX)), @@ -715,18 +714,6 @@ pub enum Addr { Str(usize) } -impl fmt::Display for Addr { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - &Addr::Con(ref c) => write!(f, "Addr::Con({})", c), - &Addr::Lis(l) => write!(f, "Addr::Lis({})", l), - &Addr::HeapCell(h) => write!(f, "Addr::HeapCell({})", h), - &Addr::StackCell(fr, sc)=> write!(f, "Addr::StackCell({}, {})", fr, sc), - &Addr::Str(s) => write!(f, "Addr::Str({})", s) - } - } -} - impl PartialEq for Addr { fn eq(&self, r: &Ref) -> bool { self.as_var() == Some(*r) @@ -1500,7 +1487,7 @@ impl OpDecl { pub fn name(&self) -> ClauseName { self.2.clone() } - + pub fn submit(&self, module: ClauseName, op_dir: &mut OpDir) -> Result<(), SessionError> { let (prec, spec, name) = (self.0, self.1, self.2.clone()); diff --git a/src/prolog/lib/builtins.pl b/src/prolog/lib/builtins.pl index 1260f642..211bbc58 100644 --- a/src/prolog/lib/builtins.pl +++ b/src/prolog/lib/builtins.pl @@ -202,9 +202,9 @@ get_args([Arg|Args], Func, I0, N) :- % write, write_canonical, writeq, write_term. is_write_option(Functor) :- - Functor =.. [Name, Arg | Args], - ( Args == [], Arg == true -> true - ; Args == [], Arg == false -> true + Functor =.. [Name, Arg], + ( Arg == true -> true + ; Arg == false -> true ; throw(error(domain_error(write_option, Functor), write_term/2)) ), % 8.14.2.3 e) ( Name == ignore_ops -> true ; Name == quoted -> true diff --git a/src/prolog/machine/machine_state_impl.rs b/src/prolog/machine/machine_state_impl.rs index d709540a..6dc36ceb 100644 --- a/src/prolog/machine/machine_state_impl.rs +++ b/src/prolog/machine/machine_state_impl.rs @@ -59,6 +59,13 @@ impl MachineState { } } + #[allow(dead_code)] + pub fn print_heap(&self) { + for h in 0 .. self.heap.h { + println!("{} : {}", h, self.heap[h]); + } + } + #[inline] pub fn machine_flags(&self) -> MachineFlags { self.flags @@ -1260,7 +1267,8 @@ impl MachineState { if let Addr::HeapCell(hc) = addr { if hc < h { - self.heap.push(HeapCellValue::Addr(addr)); + let heap_val = self.heap[hc].clone(); + self.heap.push(heap_val); return; } } @@ -2112,14 +2120,14 @@ impl MachineState { .unwrap_or(0); if and_gi > or_gi { - let index = self.e + 1; + let new_e = self.e + 1; - self.and_stack[index].e = self.e; - self.and_stack[index].cp = self.cp.clone(); - self.and_stack[index].global_index = gi; + self.and_stack[new_e].e = self.e; + self.and_stack[new_e].cp = self.cp.clone(); + self.and_stack[new_e].global_index = gi; - self.and_stack.resize(index, num_cells); - self.e = index; + self.and_stack.resize(new_e, num_cells); + self.e = new_e; return; } diff --git a/src/prolog/machine/mod.rs b/src/prolog/machine/mod.rs index cc3e0b06..d301eda9 100644 --- a/src/prolog/machine/mod.rs +++ b/src/prolog/machine/mod.rs @@ -627,7 +627,7 @@ impl MachineState { } self.query_stepper(indices, policies, code_repo); - + match self.p { CodePtr::Local(LocalCodePtr::TopLevel(_, p)) if p > 0 => {}, _ => { diff --git a/src/prolog/machine/system_calls.rs b/src/prolog/machine/system_calls.rs index 00c7dd72..5de4f397 100644 --- a/src/prolog/machine/system_calls.rs +++ b/src/prolog/machine/system_calls.rs @@ -516,9 +516,9 @@ impl MachineState { &SystemClauseType::WriteTerm => { let addr = self[temp_v!(1)].clone(); - let ignore_ops = self[temp_v!(2)].clone(); - let numbervars = self[temp_v!(3)].clone(); - let quoted = self[temp_v!(4)].clone(); + let ignore_ops = self.store(self.deref(self[temp_v!(2)].clone())); + let numbervars = self.store(self.deref(self[temp_v!(3)].clone())); + let quoted = self.store(self.deref(self[temp_v!(4)].clone())); let mut printer = HCPrinter::new(&self, PrinterOutputter::new()); diff --git a/src/prolog/write.rs b/src/prolog/write.rs index 798e0ff1..76fb48fd 100644 --- a/src/prolog/write.rs +++ b/src/prolog/write.rs @@ -127,6 +127,32 @@ impl fmt::Display for ClauseType { } } +impl fmt::Display for HeapCellValue { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + &HeapCellValue::Addr(ref addr) => + write!(f, "{}", addr), + &HeapCellValue::NamedStr(arity, ref name, Some((priority, spec))) => + write!(f, "{}/{} (op, priority: {}, spec: {})", name.as_str(), arity, + priority, spec), + &HeapCellValue::NamedStr(arity, ref name, None) => + write!(f, "{}/{}", name.as_str(), arity) + } + } +} + +impl fmt::Display for Addr { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + &Addr::Con(ref c) => write!(f, "Addr::Con({})", c), + &Addr::Lis(l) => write!(f, "Addr::Lis({})", l), + &Addr::HeapCell(h) => write!(f, "Addr::HeapCell({})", h), + &Addr::StackCell(fr, sc)=> write!(f, "Addr::StackCell({}, {})", fr, sc), + &Addr::Str(s) => write!(f, "Addr::Str({})", s) + } + } +} + impl fmt::Display for ControlInstruction { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { diff --git a/src/tests.rs b/src/tests.rs index e49829e2..4e5ab35e 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1493,7 +1493,7 @@ fn test_queries_on_builtins() ["N = 5", "Xs = [_4, _8, _12, _16, _20]"]], 6); - assert_prolog_success!(&mut wam, "?- length(Xs, 3).", [["Xs = [_4, _8, _12]"]]); + assert_prolog_success!(&mut wam, "?- length(Xs, 3).", [["Xs = [_5, _9, _13]"]]); assert_prolog_success!(&mut wam, "?- length([], N).", [["N = 0"]]); assert_prolog_success!(&mut wam, "?- length(Xs, 0).", [["Xs = []"]]); assert_prolog_success!(&mut wam, "?- length([a,b,[a,b,c]], 3)."); @@ -1630,7 +1630,7 @@ fn test_queries_on_builtins() assert_prolog_failure!(&mut wam, "?- Pairs = [a-a|Pairs], keysort(Pairs, _)."); assert_prolog_success!(&mut wam, "?- Pairs = [a-a|Pairs], catch(keysort(Pairs, _), error(E, _), true).", - [["E = type_error(list, [a-a | _22])", "Pairs = [a-a | Pairs]"]]); + [["E = type_error(list, [a-a | _25])", "Pairs = [a-a | Pairs]"]]); assert_prolog_success!(&mut wam, "?- keysort([], L).", [["L = []"]]); @@ -1639,9 +1639,9 @@ fn test_queries_on_builtins() assert_prolog_success!(&mut wam, "?- catch(keysort([],[a|a]),error(Pat, _),true).", [["Pat = type_error(list, [a | a])"]]); assert_prolog_success!(&mut wam, "?- catch(keysort(_, _), error(E, _), true).", - [["E = type_error(list, _13)"]]); + [["E = type_error(list, _16)"]]); assert_prolog_success!(&mut wam, "?- catch(keysort([a-1], [_|b]), error(E, _), true).", - [["E = type_error(list, [_24 | b])"]]); + [["E = type_error(list, [_27 | b])"]]); assert_prolog_success!(&mut wam, "?- catch(keysort([a-1], [a-b,c-d,a]), error(E, _), true).", [["E = type_error(pair, a)"]]); assert_prolog_success!(&mut wam, "?- catch(keysort([a], [a-b]), error(E, _), true).", @@ -1654,7 +1654,7 @@ fn test_queries_on_builtins() assert_prolog_success!(&mut wam, "?- sort([], L).", [["L = []"]]); assert_prolog_success!(&mut wam, "?- catch(sort(_, []), error(E, _), true).", - [["E = type_error(list, _13)"]]); + [["E = type_error(list, _16)"]]); assert_prolog_success!(&mut wam, "?- catch(sort([a,b,c], not_a_list), error(E, _), true).", [["E = type_error(list, not_a_list)"]]);