Fix precedence rules for nested subtractions and implications

src/format.rs

@@ -1,30 +1,30 @@
-struct TermDisplay<'term>
+struct TermDisplay<'parent>
 {
-	parent_precedence: u64,
-	term: &'term crate::Term,
+	parent: Option<&'parent crate::Term>,
+	term: &'parent crate::Term,
 }
 
-struct FormulaDisplay<'formula>
+struct FormulaDisplay<'parent>
 {
-	parent_precedence: u64,
-	formula: &'formula crate::Formula,
+	parent: Option<&'parent crate::Formula>,
+	formula: &'parent crate::Formula,
 }
 
-fn display_term<'term>(term: &'term crate::Term, parent_precedence: u64) -> TermDisplay<'term>
+fn display_term<'parent>(term: &'parent crate::Term, parent: Option<&'parent crate::Term>) -> TermDisplay<'parent>
 {
 	TermDisplay
 	{
-		parent_precedence,
+		parent,
 		term,
 	}
 }
 
-fn display_formula<'formula>(formula: &'formula crate::Formula, parent_precedence: u64)
-	-> FormulaDisplay<'formula>
+fn display_formula<'parent>(formula: &'parent crate::Formula, parent: Option<&'parent crate::Formula>)
+	-> FormulaDisplay<'parent>
 {
 	FormulaDisplay
 	{
-		parent_precedence,
+		parent,
 		formula,
 	}
 }
@@ -40,6 +40,32 @@ fn term_precedence(term: &crate::Term) -> u64
 	}
 }
 
+fn term_requires_parentheses(child: &crate::Term, parent: Option<&crate::Term>) -> bool
+{
+	match parent
+	{
+		None => false,
+		Some(parent) =>
+		{
+			let child_precedence = term_precedence(child);
+			let parent_precedence = term_precedence(parent);
+
+			if child_precedence != parent_precedence
+			{
+				return child_precedence > parent_precedence;
+			}
+
+			// Subtraction isn’t associative, so handle them separately
+			// TODO: only do this for right-hand side of subtractions
+			match parent
+			{
+				crate::Term::Subtract(_, _) => true,
+				_ => false,
+			}
+		},
+	}
+}
+
 fn formula_precedence(formula: &crate::Formula) -> u64
 {
 	match formula
@@ -54,16 +80,35 @@ fn formula_precedence(formula: &crate::Formula) -> u64
 	}
 }
 
+fn formula_requires_parentheses(child: &crate::Formula, parent: Option<&crate::Formula>) -> bool
+{
+	match parent
+	{
+		None => false,
+		Some(parent) =>
+		{
+			let child_precedence = formula_precedence(child);
+			let parent_precedence = formula_precedence(parent);
+
+			if child_precedence != parent_precedence
+			{
+				return child_precedence > parent_precedence;
+			}
+
+			// Implications aren’t associative, so handle them separately
+			match parent
+			{
+				crate::Formula::Implies(_, _) => true,
+				_ => false,
+			}
+		},
+	}
+}
+
 impl std::fmt::Debug for crate::VariableDeclaration
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		match &self.domain
-		{
-			crate::Domain::Program => write!(format, "X")?,
-			crate::Domain::Integer => write!(format, "N")?,
-		};
-
 		write!(format, "{}", &self.name)
 	}
 }
@@ -80,8 +125,7 @@ impl<'term> std::fmt::Debug for TermDisplay<'term>
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		let precedence = term_precedence(self.term);
-		let requires_parentheses = precedence > self.parent_precedence;
+		let requires_parentheses = term_requires_parentheses(self.term, self.parent);
 
 		if requires_parentheses
 		{
@@ -96,10 +140,10 @@ impl<'term> std::fmt::Debug for TermDisplay<'term>
 			crate::Term::Symbolic(ref value) => write!(format, "{}", value),
 			crate::Term::String(ref value) => write!(format, "\"{}\"", value),
 			crate::Term::Variable(ref declaration) => write!(format, "{:?}", declaration),
-			crate::Term::Add(ref left, ref right) => write!(format, "{:?} + {:?}", display_term(left, precedence), display_term(right, precedence)),
-			crate::Term::Subtract(ref left, ref right) => write!(format, "{:?} - {:?}", display_term(left, precedence), display_term(right, precedence)),
-			crate::Term::Multiply(ref left, ref right) => write!(format, "{:?} * {:?}", display_term(left, precedence), display_term(right, precedence)),
-			crate::Term::Negative(ref argument) => write!(format, "-{:?}", display_term(argument, precedence)),
+			crate::Term::Add(ref left, ref right) => write!(format, "{:?} + {:?}", display_term(left, Some(self.term)), display_term(right, Some(self.term))),
+			crate::Term::Subtract(ref left, ref right) => write!(format, "{:?} - {:?}", display_term(left, Some(self.term)), display_term(right, Some(self.term))),
+			crate::Term::Multiply(ref left, ref right) => write!(format, "{:?} * {:?}", display_term(left, Some(self.term)), display_term(right, Some(self.term))),
+			crate::Term::Negative(ref argument) => write!(format, "-{:?}", display_term(argument, Some(self.term))),
 		}?;
 
 		if requires_parentheses
@@ -123,8 +167,7 @@ impl<'formula> std::fmt::Debug for FormulaDisplay<'formula>
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		let precedence = formula_precedence(self.formula);
-		let requires_parentheses = precedence > self.parent_precedence;
+		let requires_parentheses = formula_requires_parentheses(self.formula, self.parent);
 
 		if requires_parentheses
 		{
@@ -146,7 +189,7 @@ impl<'formula> std::fmt::Debug for FormulaDisplay<'formula>
 					separator = ", "
 				}
 
-				write!(format, " {:?}", display_formula(&exists.argument, precedence))?;
+				write!(format, " {:?}", display_formula(&exists.argument, Some(self.formula)))?;
 			},
 			crate::Formula::ForAll(ref for_all) =>
 			{
@@ -161,16 +204,16 @@ impl<'formula> std::fmt::Debug for FormulaDisplay<'formula>
 					separator = ", "
 				}
 
-				write!(format, " {:?}", display_formula(&for_all.argument, precedence))?;
+				write!(format, " {:?}", display_formula(&for_all.argument, Some(self.formula)))?;
 			},
-			crate::Formula::Not(ref argument) => write!(format, "not {:?}", display_formula(argument, precedence))?,
+			crate::Formula::Not(ref argument) => write!(format, "not {:?}", display_formula(argument, Some(self.formula)))?,
 			crate::Formula::And(ref arguments) =>
 			{
 				let mut separator = "";
 
 				for argument in arguments
 				{
-					write!(format, "{}{:?}", separator, display_formula(argument, precedence))?;
+					write!(format, "{}{:?}", separator, display_formula(argument, Some(self.formula)))?;
 
 					separator = " and "
 				}
@@ -181,19 +224,19 @@ impl<'formula> std::fmt::Debug for FormulaDisplay<'formula>
 
 				for argument in arguments
 				{
-					write!(format, "{}{:?}", separator, display_formula(argument, precedence))?;
+					write!(format, "{}{:?}", separator, display_formula(argument, Some(self.formula)))?;
 
 					separator = " or "
 				}
 			},
-			crate::Formula::Implies(ref left, ref right) => write!(format, "{:?} -> {:?}", display_formula(left, precedence), display_formula(right, precedence))?,
-			crate::Formula::Biconditional(ref left, ref right) => write!(format, "{:?} <-> {:?}", display_formula(left, precedence), display_formula(right, precedence))?,
-			crate::Formula::Less(ref left, ref right) => write!(format, "{:?} < {:?}", display_term(left, 1000), display_term(right, 1000))?,
-			crate::Formula::LessOrEqual(ref left, ref right) => write!(format, "{:?} <= {:?}", display_term(left, 1000), display_term(right, 1000))?,
-			crate::Formula::Greater(ref left, ref right) => write!(format, "{:?} > {:?}", display_term(left, 1000), display_term(right, 1000))?,
-			crate::Formula::GreaterOrEqual(ref left, ref right) => write!(format, "{:?} >= {:?}", display_term(left, 1000), display_term(right, 1000))?,
-			crate::Formula::Equal(ref left, ref right) => write!(format, "{:?} = {:?}", display_term(left, 1000), display_term(right, 1000))?,
-			crate::Formula::NotEqual(ref left, ref right) => write!(format, "{:?} != {:?}", display_term(left, 1000), display_term(right, 1000))?,
+			crate::Formula::Implies(ref left, ref right) => write!(format, "{:?} -> {:?}", display_formula(left, Some(self.formula)), display_formula(right, Some(self.formula)))?,
+			crate::Formula::Biconditional(ref left, ref right) => write!(format, "{:?} <-> {:?}", display_formula(left, Some(self.formula)), display_formula(right, Some(self.formula)))?,
+			crate::Formula::Less(ref left, ref right) => write!(format, "{:?} < {:?}", display_term(left, None), display_term(right, None))?,
+			crate::Formula::LessOrEqual(ref left, ref right) => write!(format, "{:?} <= {:?}", display_term(left, None), display_term(right, None))?,
+			crate::Formula::Greater(ref left, ref right) => write!(format, "{:?} > {:?}", display_term(left, None), display_term(right, None))?,
+			crate::Formula::GreaterOrEqual(ref left, ref right) => write!(format, "{:?} >= {:?}", display_term(left, None), display_term(right, None))?,
+			crate::Formula::Equal(ref left, ref right) => write!(format, "{:?} = {:?}", display_term(left, None), display_term(right, None))?,
+			crate::Formula::NotEqual(ref left, ref right) => write!(format, "{:?} != {:?}", display_term(left, None), display_term(right, None))?,
 			crate::Formula::Boolean(value) =>
 				match value
 				{
@@ -212,7 +255,7 @@ impl<'formula> std::fmt::Debug for FormulaDisplay<'formula>
 
 					for argument in &predicate.arguments
 					{
-						write!(format, "{}{:?}", separator, display_term(argument, 1000))?;
+						write!(format, "{}{:?}", separator, display_term(argument, None))?;
 
 						separator = ", "
 					}
@@ -243,7 +286,7 @@ impl std::fmt::Debug for crate::Formula
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		write!(format, "{:?}", display_formula(&self, 1000))
+		write!(format, "{:?}", display_formula(&self, None))
 	}
 }
 
@@ -251,7 +294,7 @@ impl std::fmt::Display for crate::Formula
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		write!(format, "{}", display_formula(&self, 1000))
+		write!(format, "{}", display_formula(&self, None))
 	}
 }
 
@@ -259,7 +302,7 @@ impl std::fmt::Debug for crate::Term
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		write!(format, "{:?}", display_term(&self, 1000))
+		write!(format, "{:?}", display_term(&self, None))
 	}
 }
 
@@ -267,6 +310,6 @@ impl std::fmt::Display for crate::Term
 {
 	fn fmt(&self, format: &mut std::fmt::Formatter) -> std::fmt::Result
 	{
-		write!(format, "{}", display_term(&self, 1000))
+		write!(format, "{}", display_term(&self, None))
 	}
 }

src/lib.rs

@@ -1,6 +1,6 @@
 mod ast;
 pub mod format;
-mod parse;
+pub mod parse;
 
 pub use ast::{Domain, Exists, Formula, ForAll, Predicate, PredicateDeclaration, VariableDeclaration, Term};
 pub use parse::{formula, formulas, term};

src/parse.rs

@@ -2,10 +2,10 @@ use nom::
 {
 	IResult,
 	bytes::complete::{take_while, take_while_m_n, is_not},
-	character::complete::{digit1, multispace0},
-	sequence::{preceded, delimited, pair},
-	combinator::{map, map_res},
-	multi::{many0, separated_list},
+	character::complete::{digit1, multispace0, space1, not_line_ending, line_ending},
+	sequence::{preceded, delimited, pair, terminated},
+	combinator::{map, map_res, recognize},
+	multi::{many0, many0_count, separated_list},
 	branch::alt,
 	bytes::complete::tag,
 };
@@ -17,11 +17,53 @@ enum TermOperator
 	Multiply,
 }
 
+fn comment(i: &str) -> IResult<&str, &str>
+{
+	terminated
+	(
+		preceded
+		(
+			tag("%"),
+			not_line_ending,
+		),
+		line_ending,
+	)(i)
+}
+
+pub fn whitespace0(i: &str) -> IResult<&str, ()>
+{
+	let (i, _) = preceded
+	(
+		multispace0,
+		many0_count
+		(
+			preceded
+			(
+				comment,
+				multispace0,
+			)
+		),
+	)(i)?;
+
+	Ok((i, ()))
+}
+
+pub fn whitespace_single(i: &str) -> IResult<&str, ()>
+{
+	let (i, _) = alt
+	((
+		space1,
+		line_ending,
+	))(i)?;
+
+	Ok((i, ()))
+}
+
 fn infimum(i: &str) -> IResult<&str, crate::Term>
 {
 	map
 	(
-		delimited(multispace0, tag("#inf"), multispace0),
+		delimited(whitespace0, tag("#inf"), whitespace0),
 		|_| crate::Term::Infimum
 	)(i)
 }
@@ -30,7 +72,7 @@ fn supremum(i: &str) -> IResult<&str, crate::Term>
 {
 	map
 	(
-		delimited(multispace0, tag("#sup"), multispace0),
+		delimited(whitespace0, tag("#sup"), whitespace0),
 		|_| crate::Term::Supremum
 	)(i)
 }
@@ -41,7 +83,7 @@ fn integer(i: &str) -> IResult<&str, crate::Term>
 	(
 		map_res
 		(
-			delimited(multispace0, digit1, multispace0),
+			delimited(whitespace0, digit1, whitespace0),
 			std::str::FromStr::from_str
 		),
 		crate::Term::Integer
@@ -53,6 +95,11 @@ fn is_lowercase_alphanumeric(c: char) -> bool
 	c.is_alphanumeric() && c.is_lowercase()
 }
 
+fn is_uppercase_alphanumeric(c: char) -> bool
+{
+	c.is_alphanumeric() && c.is_uppercase()
+}
+
 fn symbolic_identifier(i: &str) -> IResult<&str, String>
 {
 	let (i, symbolic_identifier) = map
@@ -72,7 +119,7 @@ fn symbolic(i: &str) -> IResult<&str, crate::Term>
 {
 	map
 	(
-		delimited(multispace0, symbolic_identifier, multispace0),
+		delimited(whitespace0, symbolic_identifier, whitespace0),
 		crate::Term::Symbolic
 	)(i)
 }
@@ -83,114 +130,74 @@ fn string(i: &str) -> IResult<&str, crate::Term>
 	(
 		delimited
 		(
-			multispace0,
+			whitespace0,
 			delimited
 			(
 				tag("\""),
 				is_not("\""),
 				tag("\""),
 			),
-			multispace0
+			whitespace0
 		),
 		|s: &str| crate::Term::String(s.to_string())
 	)(i)
 }
 
-fn program_variable_identifier(i: &str) -> IResult<&str, String>
+fn variable_identifier(i: &str) -> IResult<&str, String>
 {
 	map
 	(
 		delimited
 		(
-			multispace0,
-			preceded
+			whitespace0,
+			recognize
 			(
-				tag("X"),
+				pair
+				(
+					take_while_m_n(1, 1, is_uppercase_alphanumeric),
 					take_while(char::is_alphanumeric)
+				)
 			),
-			multispace0
+			whitespace0
 		),
 		|s: &str| s.to_string()
 	)(i)
 }
 
-fn integer_variable_identifier(i: &str) -> IResult<&str, String>
-{
-	map
-	(
-		delimited
-		(
-			multispace0,
-			preceded
-			(
-				tag("N"),
-				take_while(char::is_alphanumeric)
-			),
-			multispace0
-		),
-		|s: &str| s.to_string()
-	)(i)
-}
-
-fn program_variable_declaration(i: &str) -> IResult<&str, crate::VariableDeclaration>
-{
-	map
-	(
-		program_variable_identifier,
-		|s| crate::VariableDeclaration{name: s, domain: crate::Domain::Program}
-	)(i)
-}
-
-fn integer_variable_declaration(i: &str) -> IResult<&str, crate::VariableDeclaration>
-{
-	map
-	(
-		integer_variable_identifier,
-		|s| crate::VariableDeclaration{name: s, domain: crate::Domain::Integer}
-	)(i)
-}
-
 fn variable_declaration(i: &str) -> IResult<&str, crate::VariableDeclaration>
-{
-	alt
-	((
-		program_variable_declaration,
-		integer_variable_declaration
-	))(i)
-}
-
-fn program_variable(i: &str) -> IResult<&str, crate::Term>
 {
 	map
 	(
-		program_variable_identifier,
-		|s| crate::Term::Variable(crate::VariableDeclaration{name: s, domain: crate::Domain::Program})
-	)(i)
-}
-
-fn integer_variable(i: &str) -> IResult<&str, crate::Term>
+		variable_identifier,
+			|name|
 			{
-	map
-	(
-		integer_variable_identifier,
-		|s| crate::Term::Variable(crate::VariableDeclaration{name: s, domain: crate::Domain::Integer})
+				let domain = match name.chars().next()
+				{
+					Some('X') | Some('Y') | Some('Z') => crate::Domain::Program,
+					Some('I') | Some('N') | Some('M') => crate::Domain::Integer,
+					Some(other) => panic!("variable “{}” starts with character “{}”, which is not allowed", name, other),
+					None => panic!("unexpected variable name, please report to bug tracker"),
+				};
+
+				crate::VariableDeclaration{name, domain}
+			}
 	)(i)
 }
 
 fn variable(i: &str) -> IResult<&str, crate::Term>
 {
-	alt
-	((
-		program_variable,
-		integer_variable
-	))(i)
+	map
+	(
+		variable_declaration,
+		|variable_declaration| crate::Term::Variable(variable_declaration)
+	)(i)
 }
 
 fn predicate_0_ary(i: &str) -> IResult<&str, crate::Formula>
 {
 	map
 	(
-		delimited(multispace0, symbolic_identifier, multispace0),
+		delimited(whitespace0, symbolic_identifier, whitespace0),
 		|name| crate::Formula::Predicate(
 			crate::Predicate
 			{
@@ -211,17 +218,17 @@ fn predicate_n_ary(i: &str) -> IResult<&str, crate::Formula>
 	(
 		pair
 		(
-			delimited(multispace0, symbolic_identifier, multispace0),
+			delimited(whitespace0, symbolic_identifier, whitespace0),
 			delimited
 			(
-				multispace0,
+				whitespace0,
 				delimited
 				(
 					tag("("),
 					separated_list(tag(","), term),
 					tag(")")
 				),
-				multispace0
+				whitespace0
 			)
 		),
 		|(name, arguments)| crate::Formula::Predicate(
@@ -253,13 +260,13 @@ fn boolean(i: &str) -> IResult<&str, crate::Formula>
 	(
 		delimited
 		(
-			multispace0,
+			whitespace0,
 			alt
 			((
 				map(tag("#true"), |_| true),
 				map(tag("#false"), |_| false)
 			)),
-			multispace0
+			whitespace0
 		),
 		|value| crate::Formula::Boolean(value)
 	)(i)
@@ -360,14 +367,14 @@ fn term_parenthesized(i: &str) -> IResult<&str, crate::Term>
 {
 	delimited
 	(
-		multispace0,
+		whitespace0,
 		delimited
 		(
 			tag("("),
 			term,
 			tag(")")
 		),
-		multispace0
+		whitespace0
 	)(i)
 }
 
@@ -409,9 +416,9 @@ fn term_precedence_1(i: &str) -> IResult<&str, crate::Term>
 		(
 			delimited
 			(
-				multispace0,
+				whitespace0,
 				preceded(tag("-"), term_precedence_0),
-				multispace0
+				whitespace0
 			),
 			|term| crate::Term::Negative(Box::new(term))
 		),
@@ -468,14 +475,14 @@ fn formula_parenthesized(i: &str) -> IResult<&str, crate::Formula>
 {
 	delimited
 	(
-		multispace0,
+		whitespace0,
 		delimited
 		(
 			tag("("),
 			formula,
 			tag(")")
 		),
-		multispace0
+		whitespace0
 	)(i)
 }
 
@@ -496,10 +503,14 @@ fn exists(i: &str) -> IResult<&str, crate::Formula>
 	(
 		delimited
 		(
-			multispace0,
+			whitespace0,
 			preceded
+			(
+				terminated
 				(
 					tag("exists"),
+					whitespace_single,
+				),
 				pair
 				(
 					separated_list
@@ -510,7 +521,7 @@ fn exists(i: &str) -> IResult<&str, crate::Formula>
 					formula_precedence_1
 				)
 			),
-			multispace0
+			whitespace0
 		),
 		|(parameters, argument)| crate::Formula::Exists(
 			crate::Exists
@@ -527,10 +538,14 @@ fn for_all(i: &str) -> IResult<&str, crate::Formula>
 	(
 		delimited
 		(
-			multispace0,
+			whitespace0,
 			preceded
+			(
+				terminated
 				(
 					tag("forall"),
+					whitespace_single,
+				),
 				pair
 				(
 					separated_list
@@ -541,7 +556,7 @@ fn for_all(i: &str) -> IResult<&str, crate::Formula>
 					formula_precedence_1
 				)
 			),
-			multispace0
+			whitespace0
 		),
 		|(parameters, argument)| crate::Formula::ForAll(
 			crate::ForAll
@@ -570,9 +585,17 @@ fn formula_precedence_2(i: &str) -> IResult<&str, crate::Formula>
 		(
 			delimited
 			(
-				multispace0,
-				preceded(tag("not "), formula_precedence_1),
-				multispace0
+				whitespace0,
+				preceded
+				(
+					terminated
+					(
+						tag("not"),
+						whitespace_single,
+					),
+					formula_precedence_1
+				),
+				whitespace0
 			),
 			|argument| crate::Formula::Not(Box::new(argument))
 		),