adjust errors to be more standard compliant

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(DeclarationError),
-    InvalidDecl(Atom, usize /* arity */),
-    InvalidOpDeclName(Term),
-    InvalidOpDeclSpecTerm(Term),
-    InvalidOpDeclSpecValue(Atom),
-    InvalidOpDeclPrec(Term),
     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<ArithmeticError> 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) => {
+            CompilationError::InvalidDecl(_) => {
-                functor!(atom!("not_a_declaration"))
+                functor!(atom!("declaration_error"))
-            }
-            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::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,

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)]

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, CompilationError> {
-    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<Term>, atom_tbl: &AtomTable) -> Result<OpDecl, CompilationError> {
+    // 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
+        Term::Literal(_, Literal::Fixnum(bi)) => match u16::try_from(bi.get_num()) {
-        ref term @ 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),
+        )),
     }
 }
 

tests-pl/invalid_decl10.pl

@@ -0,0 +1 @@
+:- op(10, xf, [example, test]).

tests-pl/invalid_decl11.pl

@@ -0,0 +1 @@
+:- op(10, xf, [example, Var]).

tests-pl/invalid_decl8.pl

@@ -0,0 +1 @@
+:- op(10, Var, example).

tests-pl/invalid_decl9.pl

@@ -0,0 +1 @@
+:- op(Var, xf, example).

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).
 
-```
+```
+
+```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).
+> ```