From 880e22d1646c32499f798dbb114eecc4bbdf87a1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bennet=20Ble=C3=9Fmann?= Date: Sat, 3 Aug 2024 22:07:48 +0200 Subject: [PATCH] adjust errors to be more standard compliant --- src/machine/machine_errors.rs | 92 +++++++++---------- src/machine/machine_state.rs | 30 ++++++ src/machine/preprocessor.rs | 32 +++++-- tests-pl/invalid_decl10.pl | 1 + tests-pl/invalid_decl11.pl | 1 + tests-pl/invalid_decl8.pl | 1 + tests-pl/invalid_decl9.pl | 1 + .../cli/src_tests/declaration_errors.md | 47 ++++++++-- 8 files changed, 139 insertions(+), 66 deletions(-) create mode 100644 tests-pl/invalid_decl10.pl create mode 100644 tests-pl/invalid_decl11.pl create mode 100644 tests-pl/invalid_decl8.pl create mode 100644 tests-pl/invalid_decl9.pl diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index bc88e017..899424b4 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -38,6 +38,7 @@ pub(crate) enum ValidType { Callable, Character, Compound, + Declaration, Evaluable, Float, InByte, @@ -73,6 +74,7 @@ impl ValidType { // ValidType::PredicateIndicator => atom!("predicate_indicator"), // ValidType::Variable => atom!("variable") ValidType::TcpListener => atom!("tcp_listener"), + ValidType::Declaration => atom!("declaration"), } } } @@ -393,6 +395,21 @@ impl MachineState { from: ErrorProvenance::Constructed, } } + ExistenceError::Declaration(name, arity) => { + let culprit = functor!(atom!("/"), [atom(name), fixnum(arity)]); + + let stub = functor!( + atom!("existence_error"), + [atom(atom!("declaration")), str(self.heap.len(), 0)], + [culprit] + ); + + MachineError { + stub, + location: None, + from: ErrorProvenance::Constructed, + } + } ExistenceError::ModuleSource(source) => { let source_stub = source.as_functor_stub(); @@ -571,9 +588,13 @@ impl MachineState { return self.arithmetic_error(err); } + if let CompilationError::InvalidDecl(err) = err { + return self.declaration_error(err); + } + let location = err.line_and_col_num(); let len = self.heap.len(); - let stub = err.as_functor(&mut self.heap); + let stub = err.as_functor(); let stub = functor!(atom!("syntax_error"), [str(len, 0)], [stub]); @@ -691,12 +712,7 @@ pub enum CompilationError { ExpectedRel, InadmissibleFact, InadmissibleQueryTerm, - ExpectedDecl(Term), - InvalidDecl(Atom, usize /* arity */), - InvalidOpDeclName(Term), - InvalidOpDeclSpecTerm(Term), - InvalidOpDeclSpecValue(Atom), - InvalidOpDeclPrec(Term), + InvalidDecl(DeclarationError), InvalidMetaPredicateDecl, InvalidModuleDecl, InvalidModuleExport, @@ -706,6 +722,17 @@ pub enum CompilationError { UnreadableTerm, } +#[derive(Debug)] +pub enum DeclarationError { + ExpectedDecl(Term), + InvalidDecl(Atom, usize /* arity */), + InvalidOpDeclNameType(Term), + InvalidOpDeclSpecDomain(Term), + InvalidOpDeclSpecValue(Atom), + InvalidOpDeclPrecType(Term), + InvalidOpDeclPrecDomain(Fixnum), +} + impl From for CompilationError { #[inline] fn from(err: ArithmeticError) -> CompilationError { @@ -728,7 +755,7 @@ impl CompilationError { } } - pub(crate) fn as_functor(&self, heap: &mut Heap) -> MachineStub { + pub(crate) fn as_functor(&self) -> MachineStub { match self { CompilationError::Arithmetic(..) => { functor!(atom!("arithmetic_error")) @@ -750,48 +777,8 @@ impl CompilationError { // TODO: type_error(callable, _). functor!(atom!("inadmissible_query_term")) } - CompilationError::ExpectedDecl(_term) => { - functor!(atom!("not_a_declaration")) - } - CompilationError::InvalidDecl(name, arity) => { - let culprit = functor_stub(*name, *arity); - functor!( - atom!("existence_error"), - [atom(atom!("declaration")), str(heap.len() + 2, 0)], - [culprit] - ) - } - CompilationError::InvalidOpDeclName(_term) => { - functor!( - atom!("invalid_op_decl"), - [atom(atom!("name")), atom(atom!("expected_string_or_atom"))] - ) - } - CompilationError::InvalidOpDeclSpecTerm(_term) => { - functor!( - atom!("invalid_op_decl"), - [ - atom(atom!("specification")), - atom(atom!("expected_string_or_atom")) - ] - ) - } - CompilationError::InvalidOpDeclSpecValue(spec) => { - let functor = functor!(atom!("invalid_value"), [atom(spec)]); - functor!( - atom!("invalid_op_decl"), - [atom(atom!("specification")), str(heap.len() + 2, 0)], - [functor] - ) - } - CompilationError::InvalidOpDeclPrec(_term) => { - functor!( - atom!("invalid_op_decl"), - [ - atom(atom!("precedence")), - atom(atom!("expected_integer_in_range_0_to_1200")) - ] - ) + CompilationError::InvalidDecl(_) => { + functor!(atom!("declaration_error")) } CompilationError::InvalidMetaPredicateDecl => { functor!(atom!("invalid_meta_predicate_decl")) @@ -855,6 +842,8 @@ pub(crate) enum DomainErrorType { SourceSink, Stream, StreamOrAlias, + OperatorSpecifier, + OperatorPriority, } impl DomainErrorType { @@ -866,6 +855,8 @@ impl DomainErrorType { DomainErrorType::SourceSink => atom!("source_sink"), DomainErrorType::Stream => atom!("stream"), DomainErrorType::StreamOrAlias => atom!("stream_or_alias"), + DomainErrorType::OperatorSpecifier => atom!("operator_specifier"), + DomainErrorType::OperatorPriority => atom!("operator_priority"), } } } @@ -1044,6 +1035,7 @@ pub enum ExistenceError { Module(Atom), ModuleSource(ModuleSource), Procedure(Atom, usize), + Declaration(Atom, usize), QualifiedProcedure { module_name: Atom, name: Atom, diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index e90b8644..4f03c8e0 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -978,6 +978,36 @@ impl MachineState { } ); } + + pub(crate) fn declaration_error(&mut self, err: DeclarationError) -> MachineError { + match err { + DeclarationError::ExpectedDecl(_term) => self.type_error( + ValidType::Declaration, + atom_as_cell!(atom!("todo_insert_invalid_term_here")), + ), + DeclarationError::InvalidDecl(name, arity) => { + self.existence_error(ExistenceError::Declaration(name, arity)) + } + DeclarationError::InvalidOpDeclNameType(_term) => self.type_error( + ValidType::List, + atom_as_cell!(atom!("todo_insert_invalid_term_here")), + ), + DeclarationError::InvalidOpDeclSpecDomain(_term) => self.domain_error( + DomainErrorType::OperatorSpecifier, + atom_as_cell!(atom!("todo_insert_invalid_term_here")), + ), + DeclarationError::InvalidOpDeclSpecValue(atom) => { + self.domain_error(DomainErrorType::OperatorSpecifier, atom_as_cell!(atom)) + } + DeclarationError::InvalidOpDeclPrecType(_term) => self.type_error( + ValidType::Integer, + atom_as_cell!(atom!("todo_insert_invalid_term_here")), + ), + DeclarationError::InvalidOpDeclPrecDomain(num) => { + self.domain_error(DomainErrorType::OperatorPriority, fixnum_as_cell!(num)) + } + } + } } #[allow(clippy::upper_case_acronyms)] diff --git a/src/machine/preprocessor.rs b/src/machine/preprocessor.rs index 4c9c412e..4fcdf9b7 100644 --- a/src/machine/preprocessor.rs +++ b/src/machine/preprocessor.rs @@ -16,37 +16,47 @@ pub(crate) fn to_op_decl(prec: u16, spec: OpDeclSpec, name: Atom) -> OpDecl { } pub(crate) fn to_op_decl_spec(spec: Atom) -> Result { - OpDeclSpec::try_from(spec).map_err(|_err| CompilationError::InvalidOpDeclSpecValue(spec)) + OpDeclSpec::try_from(spec).map_err(|_err| { + CompilationError::InvalidDecl(DeclarationError::InvalidOpDeclSpecValue(spec)) + }) } fn setup_op_decl(mut terms: Vec, atom_tbl: &AtomTable) -> Result { + // should allow non-partial lists? let name = match terms.pop().unwrap() { Term::Literal(_, Literal::Atom(name)) => name, Term::Literal(_, Literal::Char(c)) => AtomTable::build_with(atom_tbl, &c.to_string()), other => { - return Err(CompilationError::InvalidOpDeclName(other)); + return Err(CompilationError::InvalidDecl( + DeclarationError::InvalidOpDeclNameType(other), + )); } }; let spec = match terms.pop().unwrap() { Term::Literal(_, Literal::Atom(name)) => name, other => { - return Err(CompilationError::InvalidOpDeclSpecTerm(other)); + return Err(CompilationError::InvalidDecl( + DeclarationError::InvalidOpDeclSpecDomain(other), + )) } }; let spec = to_op_decl_spec(spec)?; let prec = match terms.pop().unwrap() { - // once msrv is >= 1.78 we can remove the ref and the clone of term below - ref term @ Term::Literal(_, Literal::Fixnum(bi)) => match u16::try_from(bi.get_num()) { + Term::Literal(_, Literal::Fixnum(bi)) => match u16::try_from(bi.get_num()) { Ok(n) if n <= 1200 => n, _ => { - return Err(CompilationError::InvalidOpDeclPrec(term.clone())); + return Err(CompilationError::InvalidDecl( + DeclarationError::InvalidOpDeclPrecDomain(bi), + )); } }, other => { - return Err(CompilationError::InvalidOpDeclPrec(other)); + return Err(CompilationError::InvalidDecl( + DeclarationError::InvalidOpDeclPrecType(other), + )); } }; @@ -335,9 +345,13 @@ pub(super) fn setup_declaration<'a, LS: LoadState<'a>>( let (module_name, name, meta_specs) = setup_meta_predicate(terms, loader)?; Ok(Declaration::MetaPredicate(module_name, name, meta_specs)) } - _ => Err(CompilationError::InvalidDecl(name, terms.len())), + _ => Err(CompilationError::InvalidDecl( + DeclarationError::InvalidDecl(name, terms.len()), + )), }, - other => Err(CompilationError::ExpectedDecl(other)), + other => Err(CompilationError::InvalidDecl( + DeclarationError::ExpectedDecl(other), + )), } } diff --git a/tests-pl/invalid_decl10.pl b/tests-pl/invalid_decl10.pl new file mode 100644 index 00000000..82838ee6 --- /dev/null +++ b/tests-pl/invalid_decl10.pl @@ -0,0 +1 @@ +:- op(10, xf, [example, test]). \ No newline at end of file diff --git a/tests-pl/invalid_decl11.pl b/tests-pl/invalid_decl11.pl new file mode 100644 index 00000000..9d4c6193 --- /dev/null +++ b/tests-pl/invalid_decl11.pl @@ -0,0 +1 @@ +:- op(10, xf, [example, Var]). \ No newline at end of file diff --git a/tests-pl/invalid_decl8.pl b/tests-pl/invalid_decl8.pl new file mode 100644 index 00000000..67432dbb --- /dev/null +++ b/tests-pl/invalid_decl8.pl @@ -0,0 +1 @@ +:- op(10, Var, example). \ No newline at end of file diff --git a/tests-pl/invalid_decl9.pl b/tests-pl/invalid_decl9.pl new file mode 100644 index 00000000..0721f18b --- /dev/null +++ b/tests-pl/invalid_decl9.pl @@ -0,0 +1 @@ +:- op(Var, xf, example). \ No newline at end of file diff --git a/tests/scryer/cli/src_tests/declaration_errors.md b/tests/scryer/cli/src_tests/declaration_errors.md index 68482427..caa30412 100644 --- a/tests/scryer/cli/src_tests/declaration_errors.md +++ b/tests/scryer/cli/src_tests/declaration_errors.md @@ -1,35 +1,68 @@ ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl1.pl -g halt - error(syntax_error(invalid_op_decl(specification,invalid_value(moin))),load/1). + error(domain_error(operator_specifier,moin),load/1). ``` ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl2.pl -g halt - error(syntax_error(invalid_op_decl(precedence,expected_integer_in_range_0_to_1200)),load/1). + error(domain_error(operator_priority,4000),load/1). ``` ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl3.pl -g halt - error(syntax_error(invalid_op_decl(name,expected_string_or_atom)),load/1). + error(type_error(list,todo_insert_invalid_term_here),load/1). ``` ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl4.pl -g halt - error(syntax_error(existence_error(declaration,op/4)),load/1). + error(existence_error(declaration,op/4),load/1). ``` ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl5.pl -g halt - error(syntax_error(existence_error(declaration,(;)/2)),load/1). + error(existence_error(declaration,(;)/2),load/1). ``` ```trycmd $ scryer-prolog -f --no-add-history tests-pl/invalid_decl6.pl -g halt - error(syntax_error(not_a_declaration),load/1). + error(type_error(declaration,todo_insert_invalid_term_here),load/1). -``` \ No newline at end of file +``` + +```trycmd +$ scryer-prolog -f --no-add-history tests-pl/invalid_decl7.pl -g halt + error(domain_error(operator_specifier,todo_insert_invalid_term_here),load/1). + +``` + +```trycmd +$ scryer-prolog -f --no-add-history tests-pl/invalid_decl8.pl -g halt +% Warning: singleton variables Var at line 0 of invalid_decl8.pl + error(domain_error(operator_specifier,todo_insert_invalid_term_here),load/1). + +``` + +```trycmd +$ scryer-prolog -f --no-add-history tests-pl/invalid_decl9.pl -g halt +% Warning: singleton variables Var at line 0 of invalid_decl9.pl + error(type_error(integer,todo_insert_invalid_term_here),load/1). + +``` + +```trycmd +$ scryer-prolog -f --no-add-history tests-pl/invalid_decl10.pl -g halt + +``` + +The following test doesn't appear to terminate so its moved to a block quote for now + +> ```trycmd +> $ scryer-prolog -f --no-add-history tests-pl/invalid_decl11.pl -g halt +> % Warning: singleton variables Var at line 0 of invalid_decl11.pl +> error(instantiation_error,load/1). +> ``` -- 2.54.0