Merge pull request #2463 from Skgland/split-up-inconsitent-entry-error

Split up inconsistent entry error

src/atom_table.rs

@@ -56,6 +56,18 @@ impl indexmap::Equivalent<Atom> for str {
     }
 }
 
+impl PartialEq<str> for Atom {
+    fn eq(&self, other: &str) -> bool {
+        self.as_str().deref() == other
+    }
+}
+
+impl PartialEq<&str> for Atom {
+    fn eq(&self, &other: &&str) -> bool {
+        self.as_str().deref() == other
+    }
+}
+
 const ATOM_TABLE_INIT_SIZE: usize = 1 << 16;
 const ATOM_TABLE_ALIGN: usize = 8;
 

src/forms.rs

@@ -24,8 +24,6 @@ use std::fmt;
 use std::ops::{AddAssign, Deref, DerefMut};
 use std::path::PathBuf;
 
-use crate::{is_infix, is_postfix};
-
 pub type PredicateKey = (Atom, usize); // name, arity.
 
 /*
@@ -403,16 +401,6 @@ pub struct OpDecl {
     pub(crate) name: Atom,
 }
 
-#[inline(always)]
-pub(crate) fn fixity(spec: u32) -> Fixity {
-    match spec {
-        XFY | XFX | YFX => Fixity::In,
-        XF | YF => Fixity::Post,
-        FX | FY => Fixity::Pre,
-        _ => unreachable!(),
-    }
-}
-
 impl OpDecl {
     #[inline]
     pub(crate) fn new(op_desc: OpDesc, name: Atom) -> Self {
@@ -429,7 +417,7 @@ impl OpDecl {
     }
 
     pub(crate) fn insert_into_op_dir(&self, op_dir: &mut OpDir) -> Option<OpDesc> {
-        let key = (self.name, fixity(self.op_desc.get_spec() as u32));
+        let key = (self.name, self.op_desc.get_spec().fixity());
 
         if let Some(cell) = op_dir.get_mut(&key) {
             let (old_prec, old_spec) = cell.get();
@@ -447,7 +435,7 @@ impl OpDecl {
     ) -> Result<(), SessionError> {
         let (spec, name) = (self.op_desc.get_spec(), self.name);
 
-        if is_infix!(spec as u32) {
+        if spec.is_infix() {
             if let Some(desc) = existing_desc {
                 if desc.post > 0 {
                     return Err(SessionError::OpIsInfixAndPostFix(name));
@@ -455,7 +443,7 @@ impl OpDecl {
             }
         }
 
-        if is_postfix!(spec as u32) {
+        if spec.is_postfix() {
             if let Some(desc) = existing_desc {
                 if desc.inf > 0 {
                     return Err(SessionError::OpIsInfixAndPostFix(name));

src/heap_print.rs

@@ -6,8 +6,8 @@ use crate::parser::dashu::integer::Sign;
 use crate::parser::dashu::{ibig, Integer, Rational};
 use crate::{
     alpha_numeric_char, capital_letter_char, cut_char, decimal_digit_char, graphic_token_char,
-    is_fx, is_infix, is_postfix, is_prefix, is_xf, is_xfx, is_xfy, is_yfx, semicolon_char,
+    semicolon_char, sign_char, single_quote_char, small_letter_char, solo_char,
-    sign_char, single_quote_char, small_letter_char, solo_char, variable_indicator_char,
+    variable_indicator_char,
 };
 
 use crate::forms::*;
@@ -51,7 +51,7 @@ impl DirectedOp {
     fn is_prefix(&self) -> bool {
         match self {
             &DirectedOp::Left(_name, cell) | &DirectedOp::Right(_name, cell) => {
-                is_prefix!(cell.get_spec() as u32)
+                cell.get_spec().is_prefix()
             }
         }
     }
@@ -60,7 +60,7 @@ impl DirectedOp {
     fn is_negative_sign(&self) -> bool {
         match self {
             &DirectedOp::Left(name, cell) | &DirectedOp::Right(name, cell) => {
-                name == atom!("-") && is_prefix!(cell.get_spec() as u32)
+                name == atom!("-") && cell.get_spec().is_prefix()
             }
         }
     }
@@ -78,24 +78,24 @@ fn needs_bracketing(child_desc: OpDesc, op: &DirectedOp) -> bool {
 
             if &*name.as_str() == "-" {
                 let child_assoc = child_desc.get_spec();
-                if is_prefix!(spec) && (is_postfix!(child_assoc) || is_infix!(child_assoc)) {
+                if spec.is_prefix() && (child_assoc.is_postfix() || child_assoc.is_infix()) {
                     return true;
                 }
             }
 
-            let is_strict_right = is_yfx!(spec) || is_xfx!(spec) || is_fx!(spec);
+            let is_strict_right = spec.is_strict_right();
             child_desc.get_prec() > priority
                 || (child_desc.get_prec() == priority && is_strict_right)
         }
         DirectedOp::Right(_, cell) => {
             let (priority, spec) = cell.get();
-            let is_strict_left = is_xfx!(spec) || is_xfy!(spec) || is_xf!(spec);
+            let is_strict_left = spec.is_strict_left();
 
             if child_desc.get_prec() > priority
                 || (child_desc.get_prec() == priority && is_strict_left)
             {
                 true
-            } else if (is_postfix!(spec) || is_infix!(spec)) && !is_postfix!(child_desc.get_spec())
+            } else if (spec.is_postfix() || spec.is_infix()) && !child_desc.get_spec().is_postfix()
             {
                 *cell != child_desc && child_desc.get_prec() == priority
             } else {
@@ -121,7 +121,7 @@ impl<'a, ElideLists> StackfulPreOrderHeapIter<'a, ElideLists> {
             None => return false,
         };
 
-        let mut parent_spec = DirectedOp::Left(atom!("-"), OpDesc::build_with(200, FY as u8));
+        let mut parent_spec = DirectedOp::Left(atom!("-"), OpDesc::build_with(200, FY));
 
         loop {
             let cell = self.read_cell(h);
@@ -131,7 +131,7 @@ impl<'a, ElideLists> StackfulPreOrderHeapIter<'a, ElideLists> {
                     read_heap_cell!(self.heap[s],
                         (HeapCellValueTag::Atom, (name, _arity)) => {
                             if let Some(spec) = fetch_atom_op_spec(name, None, op_dir) {
-                                if is_postfix!(spec.get_spec() as u32) || is_infix!(spec.get_spec() as u32) {
+                                if spec.get_spec().is_postfix()  || spec.get_spec().is_infix() {
                                     if needs_bracketing(spec, &parent_spec) {
                                         return false;
                                     } else {
@@ -592,7 +592,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
     }
 
     fn enqueue_op(&mut self, mut max_depth: usize, name: Atom, spec: OpDesc) {
-        if is_postfix!(spec.get_spec()) {
+        if spec.get_spec().is_postfix() {
             if self.max_depth_exhausted(max_depth) {
                 self.iter.pop_stack();
                 self.state_stack.push(TokenOrRedirect::Atom(atom!("...")));
@@ -610,7 +610,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
                     right_directed_op,
                 ));
             }
-        } else if is_prefix!(spec.get_spec()) {
+        } else if spec.get_spec().is_prefix() {
             if self.max_depth_exhausted(max_depth) {
                 self.iter.pop_stack();
                 self.state_stack.push(TokenOrRedirect::Atom(atom!("...")));
@@ -639,7 +639,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
 
                 self.state_stack.push(TokenOrRedirect::Atom(atom!("...")));
             } else if self.check_max_depth(&mut max_depth) {
-                if is_xfy!(spec.get_spec()) {
+                if matches!(spec.get_spec(), XFY) {
                     let left_directed_op = DirectedOp::Left(name, spec);
 
                     self.state_stack
@@ -791,7 +791,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
         let dot_atom = atom!(".");
 
         if let Some(spec) = op_desc {
-            if dot_atom == name && is_infix!(spec.get_spec()) && !self.ignore_ops {
+            if dot_atom == name && spec.get_spec().is_infix() && !self.ignore_ops {
                 self.push_list(max_depth);
                 return true;
             }
@@ -1328,7 +1328,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
 
     fn close_list(&mut self, switch: Rc<Cell<(bool, usize)>>) -> Option<Rc<Cell<(bool, usize)>>> {
         if let Some(TokenOrRedirect::Op(_, op_desc)) = self.state_stack.last() {
-            if is_postfix!(op_desc.get_spec()) || is_infix!(op_desc.get_spec()) {
+            if op_desc.get_spec().is_postfix() || op_desc.get_spec().is_infix() {
                 self.state_stack.push(TokenOrRedirect::ChildCloseList);
                 return None;
             }
@@ -1775,7 +1775,7 @@ impl<'a, Outputter: HCValueOutputter> HCPrinter<'a, Outputter> {
                 TokenOrRedirect::Op(atom, op) => {
                     self.print_op(&atom.as_str());
 
-                    if is_prefix!(op.get_spec()) {
+                    if op.get_spec().is_prefix() {
                         self.set_parent_of_first_op(Some(DirectedOp::Left(atom, op)));
                     }
                 }
@@ -2120,9 +2120,9 @@ mod tests {
         all_cells_unmarked(&wam.machine_st.heap);
 
         wam.op_dir
-            .insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+            .insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX));
         wam.op_dir
-            .insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+            .insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX));
 
         assert_eq!(
             &wam.parse_and_print_term("[a|[] + b].").unwrap(),
@@ -2139,10 +2139,10 @@ mod tests {
         all_cells_unmarked(&wam.machine_st.heap);
 
         wam.op_dir
-            .insert((atom!("fy"), Fixity::Pre), OpDesc::build_with(9, FY as u8));
+            .insert((atom!("fy"), Fixity::Pre), OpDesc::build_with(9, FY));
 
         wam.op_dir
-            .insert((atom!("yf"), Fixity::Post), OpDesc::build_with(9, YF as u8));
+            .insert((atom!("yf"), Fixity::Post), OpDesc::build_with(9, YF));
 
         assert_eq!(
             &wam.parse_and_print_term("(fy (fy 1)yf)yf.").unwrap(),

src/machine/arithmetic_ops.rs

@@ -1419,11 +1419,11 @@ mod tests {
         let mut wam = MachineState::new();
         let mut op_dir = default_op_dir();
 
-        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("-"), Fixity::Pre), OpDesc::build_with(200, FY as u8));
+        op_dir.insert((atom!("-"), Fixity::Pre), OpDesc::build_with(200, FY));
-        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX));
-        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX));
 
         let term_write_result =
             parse_and_write_parsed_term_to_heap(&mut wam, "3 + 4 - 1 + 2.", &op_dir).unwrap();

src/machine/load_state.rs

@@ -555,10 +555,8 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> {
                         }
                     }
                     ModuleExport::OpDecl(op_decl) => {
-                        let op_dir_value_opt = op_dir.swap_remove(&(
+                        let op_dir_value_opt = op_dir
-                            op_decl.name,
+                            .swap_remove(&(op_decl.name, op_decl.op_desc.get_spec().fixity()));
-                            fixity(op_decl.op_desc.get_spec() as u32),
-                        ));
 
                         if let Some(op_desc) = op_dir_value_opt {
                             retraction_info.push_record(op_retractor(*op_decl, op_desc));

src/machine/machine_errors.rs

@@ -266,6 +266,26 @@ impl DomainError for HeapCellValue {
     }
 }
 
+impl DomainError for FunctorStub {
+    fn domain_error(
+        self,
+        machine_st: &mut MachineState,
+        valid_type: DomainErrorType,
+    ) -> MachineError {
+        let stub = functor!(
+            atom!("domain_error"),
+            [atom(valid_type.as_atom()), str(machine_st.heap.len(), 0)],
+            [self]
+        );
+
+        MachineError {
+            stub,
+            location: None,
+            from: ErrorProvenance::Constructed,
+        }
+    }
+}
+
 impl DomainError for Number {
     fn domain_error(self, machine_st: &mut MachineState, error: DomainErrorType) -> MachineError {
         let stub = functor!(
@@ -571,10 +591,15 @@ impl MachineState {
             return self.arithmetic_error(err);
         }
 
+        if let CompilationError::InvalidDirective(err) = err {
+            return self.directive_error(err);
+        }
+
         let location = err.line_and_col_num();
+        let len = self.heap.len();
         let stub = err.as_functor();
 
-        let stub = functor!(atom!("syntax_error"), [str(self.heap.len(), 0)], [stub]);
+        let stub = functor!(atom!("syntax_error"), [str(len, 0)], [stub]);
 
         MachineError {
             stub,
@@ -690,7 +715,7 @@ pub enum CompilationError {
     ExpectedRel,
     InadmissibleFact,
     InadmissibleQueryTerm,
-    InconsistentEntry,
+    InvalidDirective(DirectiveError),
     InvalidMetaPredicateDecl,
     InvalidModuleDecl,
     InvalidModuleExport,
@@ -700,6 +725,19 @@ pub enum CompilationError {
     UnreadableTerm,
 }
 
+#[derive(Debug)]
+pub enum DirectiveError {
+    ExpectedDirective(Term),
+    InvalidDirective(Atom, usize /* arity */),
+    InvalidOpDeclNameType(Term),
+    InvalidOpDeclSpecDomain(Term),
+    InvalidOpDeclSpecValue(Atom),
+    InvalidOpDeclPrecType(Term),
+    InvalidOpDeclPrecDomain(Fixnum),
+    ShallNotCreate(Atom),
+    ShallNotModify(Atom),
+}
+
 impl From<ArithmeticError> for CompilationError {
     #[inline]
     fn from(err: ArithmeticError) -> CompilationError {
@@ -744,8 +782,8 @@ impl CompilationError {
                 // TODO: type_error(callable, _).
                 functor!(atom!("inadmissible_query_term"))
             }
-            CompilationError::InconsistentEntry => {
+            CompilationError::InvalidDirective(_) => {
-                functor!(atom!("inconsistent_entry"))
+                functor!(atom!("directive_error"))
             }
             CompilationError::InvalidMetaPredicateDecl => {
                 functor!(atom!("invalid_meta_predicate_decl"))
@@ -809,6 +847,9 @@ pub(crate) enum DomainErrorType {
     SourceSink,
     Stream,
     StreamOrAlias,
+    OperatorSpecifier,
+    OperatorPriority,
+    Directive,
 }
 
 impl DomainErrorType {
@@ -820,6 +861,9 @@ 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"),
+            DomainErrorType::Directive => atom!("directive"),
         }
     }
 }

src/machine/machine_state.rs

@@ -978,6 +978,42 @@ impl MachineState {
             }
         );
     }
+
+    pub(crate) fn directive_error(&mut self, err: DirectiveError) -> MachineError {
+        match err {
+            DirectiveError::ExpectedDirective(_term) => self.domain_error(
+                DomainErrorType::Directive,
+                atom_as_cell!(atom!("todo_insert_invalid_term_here")),
+            ),
+            DirectiveError::InvalidDirective(name, arity) => {
+                self.domain_error(DomainErrorType::Directive, functor_stub(name, arity))
+            }
+            DirectiveError::InvalidOpDeclNameType(_term) => self.type_error(
+                ValidType::List,
+                atom_as_cell!(atom!("todo_insert_invalid_term_here")),
+            ),
+            DirectiveError::InvalidOpDeclSpecDomain(_term) => self.domain_error(
+                DomainErrorType::OperatorSpecifier,
+                atom_as_cell!(atom!("todo_insert_invalid_term_here")),
+            ),
+            DirectiveError::InvalidOpDeclSpecValue(atom) => {
+                self.domain_error(DomainErrorType::OperatorSpecifier, atom_as_cell!(atom))
+            }
+            DirectiveError::InvalidOpDeclPrecType(_term) => self.type_error(
+                ValidType::Integer,
+                atom_as_cell!(atom!("todo_insert_invalid_term_here")),
+            ),
+            DirectiveError::InvalidOpDeclPrecDomain(num) => {
+                self.domain_error(DomainErrorType::OperatorPriority, fixnum_as_cell!(num))
+            }
+            DirectiveError::ShallNotCreate(atom) => {
+                self.permission_error(Permission::Create, atom!("operator"), atom)
+            }
+            DirectiveError::ShallNotModify(atom) => {
+                self.permission_error(Permission::Modify, atom!("operator"), atom)
+            }
+        }
+    }
 }
 
 #[allow(clippy::upper_case_acronyms)]

src/machine/mock_wam.rs

@@ -264,11 +264,11 @@ mod tests {
         let mut wam = MachineState::new();
         let mut op_dir = default_op_dir();
 
-        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX));
-        op_dir.insert((atom!("="), Fixity::In), OpDesc::build_with(700, XFX as u8));
+        op_dir.insert((atom!("="), Fixity::In), OpDesc::build_with(700, XFX));
 
         {
             parse_and_write_parsed_term_to_heap(&mut wam, "f(X,X).", &op_dir).unwrap();
@@ -485,10 +485,10 @@ mod tests {
         let mut wam = MachineState::new();
         let mut op_dir = default_op_dir();
 
-        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("+"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX as u8));
+        op_dir.insert((atom!("-"), Fixity::In), OpDesc::build_with(500, YFX));
-        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+        op_dir.insert((atom!("*"), Fixity::In), OpDesc::build_with(400, YFX));
-        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX as u8));
+        op_dir.insert((atom!("/"), Fixity::In), OpDesc::build_with(400, YFX));
 
         {
             parse_and_write_parsed_term_to_heap(&mut wam, "f(X,X).", &op_dir).unwrap();

src/machine/preprocessor.rs

@@ -11,42 +11,74 @@ use indexmap::IndexSet;
 
 use std::cell::Cell;
 use std::convert::TryFrom;
+pub(crate) fn to_op_decl(prec: u16, spec: OpDeclSpec, name: Atom) -> OpDecl {
+    OpDecl::new(OpDesc::build_with(prec, spec), name)
+}
 
-pub(crate) fn to_op_decl(prec: u16, spec: Atom, name: Atom) -> Result<OpDecl, CompilationError> {
+pub(crate) fn to_op_decl_spec(spec: Atom) -> Result<OpDeclSpec, CompilationError> {
-    match spec {
+    OpDeclSpec::try_from(spec).map_err(|_err| {
-        atom!("xfx") => Ok(OpDecl::new(OpDesc::build_with(prec, XFX as u8), name)),
+        CompilationError::InvalidDirective(DirectiveError::InvalidOpDeclSpecValue(spec))
-        atom!("xfy") => Ok(OpDecl::new(OpDesc::build_with(prec, XFY as u8), name)),
+    })
-        atom!("yfx") => Ok(OpDecl::new(OpDesc::build_with(prec, YFX as u8), name)),
-        atom!("fx") => Ok(OpDecl::new(OpDesc::build_with(prec, FX as u8), name)),
-        atom!("fy") => Ok(OpDecl::new(OpDesc::build_with(prec, FY as u8), name)),
-        atom!("xf") => Ok(OpDecl::new(OpDesc::build_with(prec, XF as u8), name)),
-        atom!("yf") => Ok(OpDecl::new(OpDesc::build_with(prec, YF as u8), name)),
-        _ => Err(CompilationError::InconsistentEntry),
-    }
 }
 
 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()),
-        _ => return Err(CompilationError::InconsistentEntry),
+        other => {
+            return Err(CompilationError::InvalidDirective(
+                DirectiveError::InvalidOpDeclNameType(other),
+            ));
+        }
     };
 
     let spec = 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::InconsistentEntry),
+            return Err(CompilationError::InvalidDirective(
+                DirectiveError::InvalidOpDeclSpecDomain(other),
+            ))
+        }
     };
 
+    let spec = to_op_decl_spec(spec)?;
+
     let prec = match terms.pop().unwrap() {
         Term::Literal(_, Literal::Fixnum(bi)) => match u16::try_from(bi.get_num()) {
             Ok(n) if n <= 1200 => n,
-            _ => return Err(CompilationError::InconsistentEntry),
+            _ => {
+                return Err(CompilationError::InvalidDirective(
+                    DirectiveError::InvalidOpDeclPrecDomain(bi),
+                ));
+            }
         },
-        _ => return Err(CompilationError::InconsistentEntry),
+        other => {
+            return Err(CompilationError::InvalidDirective(
+                DirectiveError::InvalidOpDeclPrecType(other),
+            ));
+        }
     };
 
-    to_op_decl(prec, spec, name)
+    if name == "[]" || name == "{}" {
+        return Err(CompilationError::InvalidDirective(
+            DirectiveError::ShallNotCreate(name),
+        ));
+    }
+
+    if name == "," {
+        return Err(CompilationError::InvalidDirective(
+            DirectiveError::ShallNotModify(name),
+        ));
+    }
+
+    if name == "|" && (prec < 1001 || !spec.is_infix()) {
+        return Err(CompilationError::InvalidDirective(
+            DirectiveError::ShallNotCreate(name),
+        ));
+    }
+
+    Ok(to_op_decl(prec, spec, name))
 }
 
 fn setup_predicate_indicator(term: &mut Term) -> Result<PredicateKey, CompilationError> {
@@ -331,9 +363,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::InconsistentEntry),
+            _ => Err(CompilationError::InvalidDirective(
+                DirectiveError::InvalidDirective(name, terms.len()),
+            )),
         },
-        _ => Err(CompilationError::InconsistentEntry),
+        other => Err(CompilationError::InvalidDirective(
+            DirectiveError::ExpectedDirective(other),
+        )),
     }
 }
 

src/machine/system_calls.rs

@@ -24,7 +24,6 @@ use crate::machine::machine_errors::*;
 use crate::machine::machine_indices::*;
 use crate::machine::machine_state::*;
 use crate::machine::partial_string::*;
-use crate::machine::preprocessor::to_op_decl;
 use crate::machine::stack::*;
 use crate::machine::streams::*;
 use crate::machine::{get_structure_index, Machine, VERIFY_ATTR_INTERRUPT_LOC};
@@ -102,6 +101,8 @@ use warp::hyper::{HeaderMap, Method};
 use warp::{Buf, Filter};
 
 use super::libraries;
+use super::preprocessor::to_op_decl;
+use super::preprocessor::to_op_decl_spec;
 
 #[cfg(feature = "repl")]
 pub(crate) fn get_key() -> KeyEvent {
@@ -3987,19 +3988,6 @@ impl Machine {
     pub(crate) fn get_next_op_db_ref(&mut self) {
         let prec = self.deref_register(1);
 
-        fn get_spec(op_spec: u8) -> Atom {
-            match op_spec as u32 {
-                XFX => atom!("xfx"),
-                XFY => atom!("xfy"),
-                YFX => atom!("yfx"),
-                FX => atom!("fx"),
-                FY => atom!("fy"),
-                XF => atom!("xf"),
-                YF => atom!("yf"),
-                _ => unreachable!(),
-            }
-        }
-
         let h = self.machine_st.heap.len();
 
         fn write_op_functors_to_heap(
@@ -4016,7 +4004,7 @@ impl Machine {
                     continue;
                 }
 
-                let spec_atom = get_spec(op_desc.get_spec());
+                let spec_atom = op_desc.get_spec().get_spec();
 
                 heap.extend(functor!(
                     atom!("op"),
@@ -4038,17 +4026,14 @@ impl Machine {
             let orig_op = self.deref_register(3);
 
             let spec_num = if spec.get_tag() == HeapCellValueTag::Atom {
-                (match cell_as_atom!(spec) {
+                Some(
-                    atom!("xfx") => XFX,
+                    OpDeclSpec::try_from(cell_as_atom!(spec))
-                    atom!("xfy") => XFY,
+                        .ok()
-                    atom!("yfx") => YFX,
+                        .filter(|spec| matches!(spec, XFX | XFY | YFX | FX | FY | XF))
-                    atom!("fx") => FX,
+                        .expect("we should only get valid values != YF here"),
-                    atom!("fy") => FY,
+                )
-                    atom!("xf") => XF,
-                    _ => unreachable!(),
-                }) as u8
             } else {
-                0
+                None
             };
 
             let num_functors = if !orig_op.is_var() {
@@ -4111,7 +4096,7 @@ impl Machine {
                     }
 
                     if (!orig_op.is_var() && atom_as_cell!(name) != orig_op)
-                        || (!spec.is_var() && other_spec != spec_num)
+                        || (!spec.is_var() && Some(other_spec) != spec_num)
                     {
                         return None;
                     }
@@ -5051,8 +5036,9 @@ impl Machine {
             }
         );
 
-        let result = to_op_decl(priority, specifier, op)
+        let result = to_op_decl_spec(specifier)
             .map_err(SessionError::from)
+            .map(|specifier| to_op_decl(priority, specifier, op))
             .and_then(|mut op_decl| {
                 if op_decl.op_desc.get_prec() == 0 {
                     op_decl.remove(&mut self.indices.op_dir);

src/parser/ast.rs

@@ -26,13 +26,112 @@ pub type Specifier = u32;
 
 pub const MAX_ARITY: usize = 1023;
 
-pub const XFX: u32 = 0x0001;
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-pub const XFY: u32 = 0x0002;
+pub enum OpDeclSpec {
-pub const YFX: u32 = 0x0004;
+    XFX = 0x0001,
-pub const XF: u32 = 0x0010;
+    XFY = 0x0002,
-pub const YF: u32 = 0x0020;
+    YFX = 0x0004,
-pub const FX: u32 = 0x0040;
+    XF = 0x0010,
-pub const FY: u32 = 0x0080;
+    YF = 0x0020,
+    FX = 0x0040,
+    FY = 0x0080,
+}
+
+pub use OpDeclSpec::*;
+
+impl OpDeclSpec {
+    pub const fn value(self) -> u32 {
+        self as u32
+    }
+
+    pub fn get_spec(self) -> Atom {
+        match self {
+            XFX => atom!("xfx"),
+            XFY => atom!("xfy"),
+            YFX => atom!("yfx"),
+            FX => atom!("fx"),
+            FY => atom!("fy"),
+            XF => atom!("xf"),
+            YF => atom!("yf"),
+        }
+    }
+
+    pub const fn is_prefix(self) -> bool {
+        matches!(self, Self::FX | Self::FY)
+    }
+
+    pub const fn is_postfix(self) -> bool {
+        matches!(self, Self::XF | Self::YF)
+    }
+
+    pub const fn is_infix(self) -> bool {
+        matches!(self, Self::XFX | Self::XFY | Self::YFX)
+    }
+
+    pub const fn is_strict_left(self) -> bool {
+        matches!(self, Self::XFX | Self::XFY | Self::XF)
+    }
+
+    pub const fn is_strict_right(self) -> bool {
+        matches!(self, Self::XFX | Self::YFX | Self::FX)
+    }
+
+    #[inline(always)]
+    pub(crate) fn fixity(self) -> Fixity {
+        match self {
+            XFY | XFX | YFX => Fixity::In,
+            XF | YF => Fixity::Post,
+            FX | FY => Fixity::Pre,
+        }
+    }
+}
+
+impl From<OpDeclSpec> for u8 {
+    fn from(value: OpDeclSpec) -> Self {
+        value as u8
+    }
+}
+
+impl From<OpDeclSpec> for u32 {
+    fn from(value: OpDeclSpec) -> Self {
+        value as u32
+    }
+}
+
+impl TryFrom<u8> for OpDeclSpec {
+    type Error = ();
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        Ok(match value {
+            0x0001 => XFX,
+            0x0002 => XFY,
+            0x0004 => YFX,
+            0x0010 => XF,
+            0x0020 => YF,
+            0x0040 => FX,
+            0x0080 => FY,
+            _ => return Err(()),
+        })
+    }
+}
+
+impl TryFrom<Atom> for OpDeclSpec {
+    type Error = ();
+
+    fn try_from(value: Atom) -> Result<Self, Self::Error> {
+        Ok(match value {
+            atom!("xfx") => Self::XFX,
+            atom!("xfy") => Self::XFY,
+            atom!("yfx") => Self::YFX,
+            atom!("fx") => Self::FX,
+            atom!("fy") => Self::FY,
+            atom!("xf") => Self::XF,
+            atom!("yf") => Self::YF,
+            _ => return Err(()),
+        })
+    }
+}
+
 pub const DELIMITER: u32 = 0x0100;
 pub const TERM: u32 = 0x1000;
 pub const LTERM: u32 = 0x3000;
@@ -64,13 +163,13 @@ macro_rules! is_lterm {
 macro_rules! is_op {
     ($x:expr) => {
         $x as u32
-            & ($crate::parser::ast::XF
+            & ($crate::parser::ast::XF as u32
-                | $crate::parser::ast::YF
+                | $crate::parser::ast::YF as u32
-                | $crate::parser::ast::FX
+                | $crate::parser::ast::FX as u32
-                | $crate::parser::ast::FY
+                | $crate::parser::ast::FY as u32
-                | $crate::parser::ast::XFX
+                | $crate::parser::ast::XFX as u32
-                | $crate::parser::ast::XFY
+                | $crate::parser::ast::XFY as u32
-                | $crate::parser::ast::YFX)
+                | $crate::parser::ast::YFX as u32)
             != 0
     };
 }
@@ -84,14 +183,14 @@ macro_rules! is_negate {
 #[macro_export]
 macro_rules! is_prefix {
     ($x:expr) => {
-        $x as u32 & ($crate::parser::ast::FX | $crate::parser::ast::FY) != 0
+        $x as u32 & ($crate::parser::ast::FX as u32 | $crate::parser::ast::FY as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_postfix {
     ($x:expr) => {
-        $x as u32 & ($crate::parser::ast::XF | $crate::parser::ast::YF) != 0
+        $x as u32 & ($crate::parser::ast::XF as u32 | $crate::parser::ast::YF as u32) != 0
     };
 }
 
@@ -99,7 +198,9 @@ macro_rules! is_postfix {
 macro_rules! is_infix {
     ($x:expr) => {
         ($x as u32
-            & ($crate::parser::ast::XFX | $crate::parser::ast::XFY | $crate::parser::ast::YFX))
+            & ($crate::parser::ast::XFX as u32
+                | $crate::parser::ast::XFY as u32
+                | $crate::parser::ast::YFX as u32))
             != 0
     };
 }
@@ -107,49 +208,48 @@ macro_rules! is_infix {
 #[macro_export]
 macro_rules! is_xfx {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::XFX) != 0
+        ($x as u32 & $crate::parser::ast::XFX as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_xfy {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::XFY) != 0
+        ($x as u32 & $crate::parser::ast::XFY as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_yfx {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::YFX) != 0
+        ($x as u32 & $crate::parser::ast::YFX as u32) != 0
     };
 }
-
 #[macro_export]
 macro_rules! is_yf {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::YF) != 0
+        ($x as u32 & $crate::parser::ast::YF as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_xf {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::XF) != 0
+        ($x as u32 & $crate::parser::ast::XF as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_fx {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::FX) != 0
+        ($x as u32 & $crate::parser::ast::FX as u32) != 0
     };
 }
 
 #[macro_export]
 macro_rules! is_fy {
     ($x:expr) => {
-        ($x as u32 & $crate::parser::ast::FY) != 0
+        ($x as u32 & $crate::parser::ast::FY as u32) != 0
     };
 }
 
@@ -264,18 +364,18 @@ pub struct OpDesc {
 
 impl OpDesc {
     #[inline]
-    pub fn build_with(prec: u16, spec: u8) -> Self {
+    pub fn build_with(prec: u16, spec: OpDeclSpec) -> Self {
-        OpDesc::new().with_spec(spec).with_prec(prec)
+        OpDesc::new().with_spec(spec as u8).with_prec(prec)
     }
 
     #[inline]
-    pub fn get(self) -> (u16, u8) {
+    pub fn get(self) -> (u16, OpDeclSpec) {
-        (self.prec(), self.spec())
+        (self.prec(), self.get_spec())
     }
 
-    pub fn set(&mut self, prec: u16, spec: u8) {
+    pub fn set(&mut self, prec: u16, spec: OpDeclSpec) {
         self.set_prec(prec);
-        self.set_spec(spec);
+        self.set_spec(spec as u8);
     }
 
     #[inline]
@@ -284,13 +384,13 @@ impl OpDesc {
     }
 
     #[inline]
-    pub fn get_spec(self) -> u8 {
+    pub fn get_spec(self) -> OpDeclSpec {
-        self.spec()
+        OpDeclSpec::try_from(self.spec()).expect("OpDecl always contains a valud OpDeclSpec")
     }
 
     #[inline]
     pub fn arity(self) -> usize {
-        if self.spec() as u32 & (XFX | XFY | YFX) == 0 {
+        if !self.get_spec().is_infix() {
             1
         } else {
             2
@@ -354,22 +454,10 @@ impl Unknown {
 pub fn default_op_dir() -> OpDir {
     let mut op_dir = OpDir::with_hasher(FxBuildHasher::default());
 
-    op_dir.insert(
+    op_dir.insert((atom!(":-"), Fixity::In), OpDesc::build_with(1200, XFX));
-        (atom!(":-"), Fixity::In),
+    op_dir.insert((atom!(":-"), Fixity::Pre), OpDesc::build_with(1200, FX));
-        OpDesc::build_with(1200, XFX as u8),
+    op_dir.insert((atom!("?-"), Fixity::Pre), OpDesc::build_with(1200, FX));
-    );
+    op_dir.insert((atom!(","), Fixity::In), OpDesc::build_with(1000, XFY));
-    op_dir.insert(
-        (atom!(":-"), Fixity::Pre),
-        OpDesc::build_with(1200, FX as u8),
-    );
-    op_dir.insert(
-        (atom!("?-"), Fixity::Pre),
-        OpDesc::build_with(1200, FX as u8),
-    );
-    op_dir.insert(
-        (atom!(","), Fixity::In),
-        OpDesc::build_with(1000, XFY as u8),
-    );
 
     op_dir
 }

src/parser/parser.rs

@@ -385,10 +385,10 @@ impl<'a, R: CharRead> Parser<'a, R> {
         }
     }
 
-    fn push_unary_op(&mut self, td: TokenDesc, spec: Specifier, assoc: u32) {
+    fn push_unary_op(&mut self, td: TokenDesc, spec: Specifier, assoc: OpDeclSpec) {
         if let Some(mut arg1) = self.terms.pop() {
             if let Some(mut name) = self.terms.pop() {
-                if is_postfix!(assoc) {
+                if assoc.is_postfix() {
                     mem::swap(&mut arg1, &mut name);
                 }
 
@@ -912,7 +912,11 @@ impl<'a, R: CharRead> Parser<'a, R> {
                         // can't be prefix, so either inf == 0
                         // or post == 0.
                         self.reduce_op(inf + post);
-                        self.promote_atom_op(name, inf + post, spec & (XFX | XFY | YFX | YF | XF));
+                        self.promote_atom_op(
+                            name,
+                            inf + post,
+                            spec & (XFX as u32 | XFY as u32 | YFX as u32 | YF as u32 | XF as u32),
+                        );
                     }
                     _ => {
                         self.reduce_op(inf + post);
@@ -923,14 +927,22 @@ impl<'a, R: CharRead> Parser<'a, R> {
                                 self.promote_atom_op(
                                     name,
                                     inf + post,
-                                    spec & (XFX | XFY | YFX | XF | YF),
+                                    spec & (XFX as u32
+                                        | XFY as u32
+                                        | YFX as u32
+                                        | XF as u32
+                                        | YF as u32),
                                 );
 
                                 return Ok(true);
                             }
                         }
 
-                        self.promote_atom_op(name, pre, spec & (FX | FY | NEGATIVE_SIGN));
+                        self.promote_atom_op(
+                            name,
+                            pre,
+                            spec & (FX as u32 | FY as u32 | NEGATIVE_SIGN),
+                        );
                     }
                 }
             } else {
@@ -1066,7 +1078,7 @@ impl<'a, R: CharRead> Parser<'a, R> {
             }
             Token::Comma => {
                 self.reduce_op(1000);
-                self.shift(Token::Comma, 1000, XFY);
+                self.shift(Token::Comma, 1000, XFY as u32);
             }
             Token::End => match self.stack.last().map(|t| t.tt) {
                 Some(TokenType::Open)

tests-pl/invalid_decl1.pl

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

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_decl12.pl

@@ -0,0 +1 @@
+:- op(500, xfy, {}).

tests-pl/invalid_decl13.pl

@@ -0,0 +1 @@
+:- op(500, xfy, [{}]).

tests-pl/invalid_decl14.pl

@@ -0,0 +1 @@
+:- op(1000, xfy, '|').

tests-pl/invalid_decl15.pl

@@ -0,0 +1 @@
+:- op(1150, fx, '|').

tests-pl/invalid_decl16.pl

@@ -0,0 +1 @@
+:- op(500, yfx, ',').

tests-pl/invalid_decl2.pl

@@ -0,0 +1 @@
+:- op(4000, xfx, example).

tests-pl/invalid_decl3.pl

@@ -0,0 +1 @@
+:- op(5, xfx, example(a,b)).

tests-pl/invalid_decl4.pl

@@ -0,0 +1 @@
+:- op(5, xfx, example, too_many_arguments).

tests-pl/invalid_decl5.pl

@@ -0,0 +1 @@
+:- (test -> set_prolog_flag(double_quotes, codes); true).

tests-pl/invalid_decl6.pl

@@ -0,0 +1 @@
+:- 9001.

tests-pl/invalid_decl7.pl

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

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/directive_errors.md

@@ -0,0 +1,98 @@
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl1.pl -g halt
+   error(domain_error(operator_specifier,moin),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl2.pl -g halt
+   error(domain_error(operator_priority,4000),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl3.pl -g halt
+   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(domain_error(directive,op/4),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl5.pl -g halt
+   error(domain_error(directive,(;)/2),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl6.pl -g halt
+   error(domain_error(directive,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).
+> ```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl12.pl -g halt
+   error(permission_error(create,operator,{}),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl13.pl -g halt
+   error(permission_error(create,operator,{}),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl14.pl -g halt
+   error(permission_error(create,operator,'|'),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl15.pl -g halt
+   error(permission_error(create,operator,'|'),load/1).
+
+```
+
+```trycmd
+$ scryer-prolog -f --no-add-history tests-pl/invalid_decl16.pl -g halt
+   error(permission_error(modify,operator,','),load/1).
+
+```

tests/scryer/main.rs

@@ -23,5 +23,6 @@ fn cli_tests() {
         .default_bin_name("scryer-prolog")
         .case("tests/scryer/cli/issues/*.toml")
         .skip("tests/scryer/cli/issues/singleton_warning.toml") // wrong line number
-        .case("tests/scryer/cli/src_tests/*.toml");
+        .case("tests/scryer/cli/src_tests/*.toml")
+        .case("tests/scryer/cli/src_tests/*.md");
 }