e6197b145496fe109cea789eb775d9dc38b04286 — Simon Ser 2 months ago 07e65a3
parser/arithm: fix addend and factor left-associativity

Instead of calling ourselves recursively, call the inner parser in a loop. This
is described in [1].

[1]: https://eli.thegreenplace.net/2009/03/14/some-problems-of-recursive-descent-parsers

Closes: https://github.com/emersion/mrsh/issues/53
2 files changed, 58 insertions(+), 44 deletions(-)

M parser/arithm.c
M test/arithm.sh
M parser/arithm.c => parser/arithm.c +53 -41
@@ 162,60 162,72 @@ static struct mrsh_arithm_expr *term(struct mrsh_parser *state) {
 }
 
 static struct mrsh_arithm_expr *factor(struct mrsh_parser *state) {
-	struct mrsh_arithm_expr *left = term(state);
-	if (left == NULL) {
+	struct mrsh_arithm_expr *expr = term(state);
+	if (expr == NULL) {
 		return NULL;
 	}
 
-	consume_whitespace(state);
-	enum mrsh_arithm_binop_type type;
-	if (parse_char(state, '*')) {
-		type = MRSH_ARITHM_BINOP_ASTERISK;
-	} else if (parse_char(state, '/')) {
-		type = MRSH_ARITHM_BINOP_SLASH;
-	} else if (parse_char(state, '%')) {
-		type = MRSH_ARITHM_BINOP_PERCENT;
-	} else {
-		return left;
-	}
-	consume_whitespace(state);
+	/* This loop ensures we parse factors as left-assossiative */
+	while (true) {
+		consume_whitespace(state);
+		enum mrsh_arithm_binop_type type;
+		if (parse_char(state, '*')) {
+			type = MRSH_ARITHM_BINOP_ASTERISK;
+		} else if (parse_char(state, '/')) {
+			type = MRSH_ARITHM_BINOP_SLASH;
+		} else if (parse_char(state, '%')) {
+			type = MRSH_ARITHM_BINOP_PERCENT;
+		} else {
+			return expr;
+		}
+		consume_whitespace(state);
+
+		/* Instead of calling ourselves recursively, we call term for
+		 * left-associativity */
+		struct mrsh_arithm_expr *right = term(state);
+		if (right == NULL) {
+			parser_set_error(state, "expected a term after *, / or % operator");
+			return NULL;
+		}
 
-	struct mrsh_arithm_expr *right = factor(state);
-	if (right == NULL) {
-		parser_set_error(state, "expected a term after *, / or % operator");
-		return NULL;
+		struct mrsh_arithm_binop *bo =
+			mrsh_arithm_binop_create(type, expr, right);
+		expr = &bo->expr;
 	}
-
-	struct mrsh_arithm_binop *bo = mrsh_arithm_binop_create(type, left, right);
-	return &bo->expr;
 }
 
 static struct mrsh_arithm_expr *addend(struct mrsh_parser *state) {
-	struct mrsh_arithm_expr *left = factor(state);
-	if (left == NULL) {
+	struct mrsh_arithm_expr *expr = factor(state);
+	if (expr == NULL) {
 		return NULL;
 	}
 
-	// consume_whitespace() is not needed here, since the call to factor()
-	// consumes trailing whitespace.
-	enum mrsh_arithm_binop_type type;
-	if (parse_char(state, '+')) {
-		type = MRSH_ARITHM_BINOP_PLUS;
-	} else if (parse_char(state, '-')) {
-		type = MRSH_ARITHM_BINOP_MINUS;
-	} else {
-		return left;
-	}
-	consume_whitespace(state);
+	/* This loop ensures we parse addends as left-assossiative */
+	while (true) {
+		// consume_whitespace() is not needed here, since the call to factor()
+		// consumes trailing whitespace.
+		enum mrsh_arithm_binop_type type;
+		if (parse_char(state, '+')) {
+			type = MRSH_ARITHM_BINOP_PLUS;
+		} else if (parse_char(state, '-')) {
+			type = MRSH_ARITHM_BINOP_MINUS;
+		} else {
+			return expr;
+		}
+		consume_whitespace(state);
+
+		/* Instead of calling ourselves recursively, we call factor for
+		 * left-associativity */
+		struct mrsh_arithm_expr *right = factor(state);
+		if (right == NULL) {
+			parser_set_error(state, "expected a factor after + or - operator");
+			return NULL;
+		}
 
-	struct mrsh_arithm_expr *right = addend(state);
-	if (right == NULL) {
-		parser_set_error(state, "expected a factor after + or - operator");
-		return NULL;
+		struct mrsh_arithm_binop *bo =
+			mrsh_arithm_binop_create(type, expr, right);
+		expr = &bo->expr;
 	}
-
-	struct mrsh_arithm_binop *bo = mrsh_arithm_binop_create(type, left, right);
-	return &bo->expr;
 }
 
 static struct mrsh_arithm_expr *shift(struct mrsh_parser *state) {

M test/arithm.sh => test/arithm.sh +5 -3
@@ 21,14 21,16 @@ echo "2&&5 =" $((2&&5))
 echo "2||5 =" $((2||5))
 
 # Associativity
-# https://github.com/emersion/mrsh/issues/53
 echo "1+2+3 =" $((1+2+3))
-#echo "5-1-2 =" $((5-1-2))
+echo "5-1-2 =" $((5-1-2))
 echo "1+2*3 =" $((1+2*3))
 echo "2*3+1 =" $((2*3+1))
+echo "6/3/2 =" $((6/3/2))
+# https://github.com/emersion/mrsh/issues/118
 #echo "2*(3+1) =" $((2*(3+1)))
+echo "2*(3+1)+1 =" $((2*(3+1)+1))
 #echo "2|(1||1)" $((2|(1||1))) # 3
-#echo "(2|1)||1" $(((2|1)||1)) # 1
+echo "(2|1)||1" $(((2|1)||1)) # 1
 echo "2|1||1" $((2|1||1)) # 1
 echo "1||1|2" $((1||1|2)) # 1