use machine-generated PartialEq instance for hashing Constant (#817)

Cargo.lock

@@ -881,6 +881,7 @@ dependencies = [
 name = "prolog_parser"
 version = "0.8.68"
 dependencies = [
+ "indexmap",
  "lexical",
  "num-rug-adapter",
  "ordered-float",

crates/prolog_parser/src/ast.rs

@@ -494,7 +494,7 @@ impl Hash for SharedOpDesc {
     }
 }
 
-#[derive(Debug, Clone, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum Constant {
     Atom(ClauseName, Option<SharedOpDesc>),
     Char(char),
@@ -529,17 +529,22 @@ impl fmt::Display for Constant {
     }
 }
 
-impl PartialEq for Constant {
+/*
-    fn eq(&self, other: &Constant) -> bool {
+ * By defining constant_eq as the PartialEq instance of Constant, we
-        match (self, other) {
+ * sometimes hash constants it considers identical to the same value,
-            (&Constant::Atom(ref atom, _), &Constant::Char(c))
+ * which can make the WAM fail erroneously. This can be avoided by
+ * using the machine-generated PartialEq for hashing.
+ */
+pub fn constant_eq(arg: &Constant, other: &Constant) -> bool {
+    match (arg, other) {
+        (&Constant::Atom(ref atom, _), &Constant::Char(c))
             | (&Constant::Char(c), &Constant::Atom(ref atom, _)) => {
                 atom.is_char() && atom.as_str().starts_with(c)
             }
-            (&Constant::Atom(ref a1, _), &Constant::Atom(ref a2, _)) => a1.as_str() == a2.as_str(),
+        (&Constant::Atom(ref a1, _), &Constant::Atom(ref a2, _)) => a1.as_str() == a2.as_str(),
-            (&Constant::Char(c1), &Constant::Char(c2)) => c1 == c2,
+        (&Constant::Char(c1), &Constant::Char(c2)) => c1 == c2,
-            (&Constant::Fixnum(n1), &Constant::Fixnum(n2)) => n1 == n2,
+        (&Constant::Fixnum(n1), &Constant::Fixnum(n2)) => n1 == n2,
-            (&Constant::Fixnum(n1), &Constant::Integer(ref n2))
+        (&Constant::Fixnum(n1), &Constant::Integer(ref n2))
             | (&Constant::Integer(ref n2), &Constant::Fixnum(n1)) => {
                 if let Some(n2) = n2.to_isize() {
                     n1 == n2
@@ -547,19 +552,16 @@ impl PartialEq for Constant {
                     false
                 }
             }
-            (&Constant::Integer(ref n1), &Constant::Integer(ref n2)) => n1 == n2,
+        (&Constant::Integer(ref n1), &Constant::Integer(ref n2)) => n1 == n2,
-            (&Constant::Rational(ref n1), &Constant::Rational(ref n2)) => n1 == n2,
+        (&Constant::Rational(ref n1), &Constant::Rational(ref n2)) => n1 == n2,
-            (&Constant::Float(ref n1), &Constant::Float(ref n2)) => n1 == n2,
+        (&Constant::Float(ref n1), &Constant::Float(ref n2)) => n1 == n2,
-            (&Constant::String(ref s1), &Constant::String(ref s2)) => s1 == s2,
+        (&Constant::String(ref s1), &Constant::String(ref s2)) => s1 == s2,
-            (&Constant::EmptyList, &Constant::EmptyList) => true,
+        (&Constant::EmptyList, &Constant::EmptyList) => true,
-            (&Constant::Usize(u1), &Constant::Usize(u2)) => u1 == u2,
+        (&Constant::Usize(u1), &Constant::Usize(u2)) => u1 == u2,
-            _ => false,
+        _ => false,
-        }
     }
 }
 
-impl Eq for Constant {}
-
 impl Constant {
     pub fn to_atom(&self) -> Option<ClauseName> {
         match self {
@@ -703,7 +705,7 @@ impl AsRef<str> for ClauseName {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, Clone)]
 pub enum Term {
     AnonVar,
     Clause(

crates/prolog_parser/src/lexer.rs

@@ -33,7 +33,7 @@ macro_rules! consume_chars_with {
     };
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone)]
 pub enum Token {
     Constant(Constant),
     Var(Rc<Atom>),
@@ -49,6 +49,17 @@ pub enum Token {
     End,
 }
 
+impl Token {
+    #[inline]
+    pub(super) fn is_end(&self) -> bool {
+        if let Token::End = self {
+            true
+        } else {
+            false
+        }
+    }
+}
+
 pub struct Lexer<'a, R: Read> {
     pub(crate) atom_tbl: TabledData<Atom>,
     pub(crate) reader: &'a mut ParsingStream<R>,

crates/prolog_parser/src/parser.rs

@@ -200,7 +200,7 @@ fn read_tokens<R: Read>(lexer: &mut Lexer<R>) -> Result<Vec<Token>, ParserError>
 
     loop {
         let token = lexer.next_token()?;
-        let at_end = Token::End == token;
+        let at_end = token.is_end();
 
         tokens.push(token);
 

src/machine/preprocessor.rs

@@ -174,7 +174,7 @@ pub(super) fn setup_module_export_list(
         export_list = *t2;
     }
 
-    if export_list.into_constant() != Some(Constant::EmptyList) {
+    if export_list.into_constant().map(|c| !constant_eq(&c, &Constant::EmptyList)).unwrap_or(true) {
         Err(CompilationError::InvalidModuleDecl)
     } else {
         Ok(exports)
@@ -273,7 +273,7 @@ fn setup_qualified_import(
         export_list = *t2;
     }
 
-    if export_list.into_constant() != Some(Constant::EmptyList) {
+    if export_list.into_constant().map(|c| !constant_eq(&c, &Constant::EmptyList)).unwrap_or(true) {
         Err(CompilationError::InvalidModuleDecl)
     } else {
         Ok((module_src, exports))