From e7cf5d4f3da5057a1e4040c7adafcfd734f57840 Mon Sep 17 00:00:00 2001 From: Mark Thom Date: Thu, 11 Feb 2021 18:40:07 -0700 Subject: [PATCH] use machine-generated PartialEq instance for hashing Constant (#817) --- Cargo.lock | 1 + crates/prolog_parser/src/ast.rs | 42 ++++++++++++++++-------------- crates/prolog_parser/src/lexer.rs | 13 ++++++++- crates/prolog_parser/src/parser.rs | 2 +- src/machine/preprocessor.rs | 4 +-- 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 43771e59..42246954 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -881,6 +881,7 @@ dependencies = [ name = "prolog_parser" version = "0.8.68" dependencies = [ + "indexmap", "lexical", "num-rug-adapter", "ordered-float", diff --git a/crates/prolog_parser/src/ast.rs b/crates/prolog_parser/src/ast.rs index 3b9a9287..f6703d10 100644 --- a/crates/prolog_parser/src/ast.rs +++ b/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), Char(char), @@ -529,17 +529,22 @@ impl fmt::Display for Constant { } } -impl PartialEq for Constant { - fn eq(&self, other: &Constant) -> bool { - match (self, other) { - (&Constant::Atom(ref atom, _), &Constant::Char(c)) +/* + * By defining constant_eq as the PartialEq instance of Constant, we + * sometimes hash constants it considers identical to the same value, + * 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::Char(c1), &Constant::Char(c2)) => c1 == c2, - (&Constant::Fixnum(n1), &Constant::Fixnum(n2)) => n1 == n2, - (&Constant::Fixnum(n1), &Constant::Integer(ref n2)) + (&Constant::Atom(ref a1, _), &Constant::Atom(ref a2, _)) => a1.as_str() == a2.as_str(), + (&Constant::Char(c1), &Constant::Char(c2)) => c1 == c2, + (&Constant::Fixnum(n1), &Constant::Fixnum(n2)) => n1 == 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::Rational(ref n1), &Constant::Rational(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::EmptyList, &Constant::EmptyList) => true, - (&Constant::Usize(u1), &Constant::Usize(u2)) => u1 == u2, - _ => false, - } + (&Constant::Integer(ref n1), &Constant::Integer(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::String(ref s1), &Constant::String(ref s2)) => s1 == s2, + (&Constant::EmptyList, &Constant::EmptyList) => true, + (&Constant::Usize(u1), &Constant::Usize(u2)) => u1 == u2, + _ => false, } } -impl Eq for Constant {} - impl Constant { pub fn to_atom(&self) -> Option { match self { @@ -703,7 +705,7 @@ impl AsRef for ClauseName { } } -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, Clone)] pub enum Term { AnonVar, Clause( diff --git a/crates/prolog_parser/src/lexer.rs b/crates/prolog_parser/src/lexer.rs index 75a3a91e..b9bda1d0 100644 --- a/crates/prolog_parser/src/lexer.rs +++ b/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), @@ -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, pub(crate) reader: &'a mut ParsingStream, diff --git a/crates/prolog_parser/src/parser.rs b/crates/prolog_parser/src/parser.rs index 60cebc90..f3655232 100644 --- a/crates/prolog_parser/src/parser.rs +++ b/crates/prolog_parser/src/parser.rs @@ -200,7 +200,7 @@ fn read_tokens(lexer: &mut Lexer) -> Result, ParserError> loop { let token = lexer.next_token()?; - let at_end = Token::End == token; + let at_end = token.is_end(); tokens.push(token); diff --git a/src/machine/preprocessor.rs b/src/machine/preprocessor.rs index 4a8bd7b4..dc020533 100644 --- a/src/machine/preprocessor.rs +++ b/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)) -- 2.54.0