From 3c72441f68ab79592b1a35fa8dd9ba5e6e140a04 Mon Sep 17 00:00:00 2001 From: Mark Thom Date: Wed, 2 May 2018 21:33:03 -0600 Subject: [PATCH] add proper error messages for keysort and sort, (re: issues #30, #32) --- src/prolog/builtins.rs | 2 + src/prolog/machine/machine_errors.rs | 85 +++++++++++++++++++++--- src/prolog/machine/machine_state.rs | 4 ++ src/prolog/machine/machine_state_impl.rs | 34 +++------- 4 files changed, 94 insertions(+), 31 deletions(-) diff --git a/src/prolog/builtins.rs b/src/prolog/builtins.rs index 2805148e..f23694f6 100644 --- a/src/prolog/builtins.rs +++ b/src/prolog/builtins.rs @@ -19,6 +19,7 @@ pub enum ValidType { Integer, List, Number, + Pair, PredicateIndicator, Variable } @@ -38,6 +39,7 @@ impl ValidType { ValidType::Integer => "integer", ValidType::List => "list", ValidType::Number => "number", + ValidType::Pair => "pair", ValidType::PredicateIndicator => "predicate_indicator", ValidType::Variable => "variable" } diff --git a/src/prolog/machine/machine_errors.rs b/src/prolog/machine/machine_errors.rs index 08444bda..9fe771cd 100644 --- a/src/prolog/machine/machine_errors.rs +++ b/src/prolog/machine/machine_errors.rs @@ -7,11 +7,80 @@ use std::rc::Rc; pub(super) type MachineError = Vec; +// used by '$skip_max_list'. +pub(super) enum CycleSearchResult { + EmptyList, + NotList, + PartialList(usize, usize), // the list length (up to max), and an offset into the heap. + ProperList(usize), // the list length. + UntouchedList(usize) // the address of an uniterated Addr::Lis(address). +} + impl MachineState { + // see 8.4.3 of Draft Technical Corrigendum 2. + pub(super) fn check_sort_errors(&self) -> Result<(), MachineError> { + let list = self.store(self.deref(self[temp_v!(1)].clone())); + let sorted = self.store(self.deref(self[temp_v!(2)].clone())); + + match self.detect_cycles(usize::max_value(), list.clone()) { + CycleSearchResult::PartialList(..) => + Err(self.error_form(self.instantiation_error())), + CycleSearchResult::NotList => + Err(self.error_form(self.type_error(ValidType::List, list))), + _ => Ok(()) + }?; + + match self.detect_cycles(usize::max_value(), sorted.clone()) { + CycleSearchResult::NotList if !sorted.is_ref() => + Err(self.error_form(self.type_error(ValidType::List, sorted))), + _ => Ok(()) + } + } + + // see 8.4.4 of Draft Technical Corrigendum 2. + pub(super) fn check_keysort_errors(&self) -> Result<(), MachineError> { + let pairs = self.store(self.deref(self[temp_v!(1)].clone())); + let sorted = self.store(self.deref(self[temp_v!(2)].clone())); + + match self.detect_cycles(usize::max_value(), pairs.clone()) { + CycleSearchResult::PartialList(..) => + Err(self.error_form(self.instantiation_error())), + CycleSearchResult::NotList => + Err(self.error_form(self.type_error(ValidType::List, pairs))), + _ => Ok(()) + }?; + + match self.detect_cycles(usize::max_value(), sorted.clone()) { + CycleSearchResult::NotList if !sorted.is_ref() => + Err(self.error_form(self.type_error(ValidType::List, sorted))), + _ => { + let mut addr = sorted; + + while let Addr::Lis(mut l) = self.store(self.deref(addr)) { + loop { + match self.heap[l].clone() { + HeapCellValue::Addr(Addr::Str(new_l)) => l = new_l, + HeapCellValue::NamedStr(2, ref name, Some(Fixity::In)) + if name.as_str() == "-" => break, + HeapCellValue::Addr(Addr::HeapCell(_)) => break, + HeapCellValue::Addr(Addr::StackCell(..)) => break, + _ => return Err(self.error_form(self.type_error(ValidType::Pair, + Addr::HeapCell(l)))) + }; + } + + addr = Addr::HeapCell(l + 2); + } + + Ok(()) + } + } + } + pub(super) fn evaluation_error(&self, eval_error: EvalError) -> MachineError { functor!("evaluation_error", 1, [heap_atom!(eval_error.as_str())]) } - + pub(super) fn type_error(&self, valid_type: ValidType, culprit: Addr) -> MachineError { functor!("type_error", 2, [heap_atom!(valid_type.as_str()), HeapCellValue::Addr(culprit)]) } @@ -19,10 +88,10 @@ impl MachineState { pub(super) fn existence_error(&self, name: ClauseName, arity: usize) -> MachineError { let name = HeapCellValue::Addr(Addr::Con(Constant::Atom(name))); let h = self.heap.h; - + let mut error = functor!("existence_error", 2, [heap_atom!("procedure"), heap_str!(3 + h)]); error.append(&mut functor!("/", 2, [name, heap_integer!(arity)], Fixity::In)); - + error } @@ -39,11 +108,11 @@ impl MachineState { let mut error_form = functor!("error", 2, [HeapCellValue::Addr(Addr::HeapCell(h + 3)), HeapCellValue::Addr(Addr::HeapCell(h + 2))]); - + error_form.append(&mut err); error_form } - + pub(super) fn throw_exception(&mut self, err: MachineError) { let h = self.heap.h; @@ -52,7 +121,7 @@ impl MachineState { self.heap.append(err); - self.registers[1] = Addr::HeapCell(h); + self.registers[1] = Addr::HeapCell(h); self.goto_throw(); - } -} + } +} diff --git a/src/prolog/machine/machine_state.rs b/src/prolog/machine/machine_state.rs index 17560ef4..8d15a82d 100644 --- a/src/prolog/machine/machine_state.rs +++ b/src/prolog/machine/machine_state.rs @@ -486,6 +486,8 @@ pub(crate) trait CallPolicy: Any { &ClauseType::Sort => { let mut list = machine_st.try_from_list(temp_v!(1))?; + machine_st.check_sort_errors()?; + list.sort_unstable_by(|a1, a2| machine_st.compare_term_test(a1, a2)); machine_st.term_dedup(&mut list); @@ -500,6 +502,8 @@ pub(crate) trait CallPolicy: Any { let mut list = machine_st.try_from_list(temp_v!(1))?; let mut key_pairs = Vec::new(); + machine_st.check_keysort_errors()?; + for val in list { let key = machine_st.project_onto_key(val.clone())?; key_pairs.push((key, val.clone())); diff --git a/src/prolog/machine/machine_state_impl.rs b/src/prolog/machine/machine_state_impl.rs index 12aa099f..e393470a 100644 --- a/src/prolog/machine/machine_state_impl.rs +++ b/src/prolog/machine/machine_state_impl.rs @@ -28,15 +28,6 @@ macro_rules! try_or_fail { }} } -// used by '$skip_max_list'. -enum CycleSearchResult { - EmptyList, - NotList, - PartialList(usize, usize), // the list length (up to max), and an offset into the heap. - ProperList(usize), // the list length. - UntouchedList(usize) // the address of an uniterated Addr::Lis(address). -} - impl MachineState { pub(super) fn new(atom_tbl: TabledData) -> MachineState { MachineState { @@ -467,7 +458,7 @@ impl MachineState { { match (n1, n2) { (Number::Integer(n1), Number::Integer(n2)) => - Ok(self.signed_bitwise_op(&*n1, &*n2, |u_n1, u_n2| u_n1 & u_n2)), + Ok(self.signed_bitwise_op(&*n1, &*n2, |u_n1, u_n2| u_n1 & u_n2)), (Number::Integer(_), n2) => Err(self.error_form(self.type_error(ValidType::Integer, Addr::Con(Constant::Number(n2))))), @@ -517,7 +508,7 @@ impl MachineState { { match (n1, n2) { (Number::Integer(n1), Number::Integer(n2)) => - Ok(self.signed_bitwise_op(&*n1, &*n2, |u_n1, u_n2| u_n1 & u_n2)), + Ok(self.signed_bitwise_op(&*n1, &*n2, |u_n1, u_n2| u_n1 & u_n2)), (Number::Integer(_), n2) => Err(self.error_form(self.type_error(ValidType::Integer, Addr::Con(Constant::Number(n2))))), @@ -991,7 +982,7 @@ impl MachineState { if narity + arity > 63 { let representation_error = self.error_form(self.representation_error(RepFlag::MaxArity)); - + self.throw_exception(representation_error); return None; @@ -1015,13 +1006,13 @@ impl MachineState { Addr::HeapCell(_) | Addr::StackCell(_, _) => { let instantiation_error = self.error_form(self.instantiation_error()); self.throw_exception(instantiation_error); - + return None; }, _ => { let type_error = self.error_form(self.type_error(ValidType::Callable, addr)); self.throw_exception(type_error); - + return None; } }; @@ -1709,12 +1700,6 @@ impl MachineState { { let a1 = self.store(self.deref(self[r].clone())); - // detect cycles. - match self.detect_cycles(usize::max_value(), a1.clone()) { - CycleSearchResult::ProperList(_) => {}, - _ => return Err(functor!("type_error", 2, [heap_atom!("list"), HeapCellValue::Addr(a1)])) - }; - match a1.clone() { Addr::Lis(mut l) => { let mut result = Vec::new(); @@ -1744,21 +1729,24 @@ impl MachineState { } } + // see 8.4.4.3 of Draft Technical Corrigendum 2 for an error guide. pub(super) fn project_onto_key(&self, a: Addr) -> Result { match self.store(self.deref(a)) { + Addr::HeapCell(_) | Addr::StackCell(..) => + Err(self.error_form(self.instantiation_error())), Addr::Str(s) => match self.heap[s].clone() { HeapCellValue::NamedStr(2, ref name, Some(Fixity::In)) if *name == clause_name!("-") => Ok(Addr::HeapCell(s+1)), - _ => - panic!("Addr::Str doesn't point to NamedStr.") + _ => Err(self.error_form(self.type_error(ValidType::Pair, + self.heap[s].as_addr(s)))) }, a => Err(self.error_form(self.type_error(ValidType::Callable, a))) } } - fn detect_cycles(&self, max_steps: usize, addr: Addr) -> CycleSearchResult + pub(super) fn detect_cycles(&self, max_steps: usize, addr: Addr) -> CycleSearchResult { let addr = self.store(self.deref(addr)); -- 2.54.0