From 559681ee7e3ae63b6d8e32fbdb93a1f85a687e96 Mon Sep 17 00:00:00 2001 From: Sander Spies Date: Mon, 27 Feb 2023 13:26:21 +0100 Subject: [PATCH] Change operator precedence to be left biased. --- HISTORY.md | 1 + .../unit_tests/expected_output/infix.re | 28 ++++++++--------- formatTest/unit_tests/input/infix.re | 30 +++++++++---------- .../reason_declarative_lexer.mll | 4 +-- src/reason-parser/reason_parser.mly | 10 ++++--- src/reason-parser/reason_pprint_ast.ml | 18 +++++------ 6 files changed, 45 insertions(+), 46 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9144831a0..05e6ba089 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,7 @@ ## 3.9 (unreleased) - Fix missing patterns around contraint pattern (a pattern with a type annotation). +- Make &, &&, |, ||, ++, and :: operators left associative. ## 3.8.2 diff --git a/formatTest/unit_tests/expected_output/infix.re b/formatTest/unit_tests/expected_output/infix.re index e01057497..84338d848 100644 --- a/formatTest/unit_tests/expected_output/infix.re +++ b/formatTest/unit_tests/expected_output/infix.re @@ -87,29 +87,27 @@ let minParens = let formatted = a1 := a2 := b1 === (b2 === y !== x !== z); -/* &...(left) is higher than &(right). &(right) is equal to &&(right) */ let parseTree = - a1 && a2 && b1 & b2 & y &|| x &|| z; + b1 & b2 & y &|| x &|| z && a2 && a1; let minParens = - a1 && a2 && b1 & b2 & y &|| x &|| z; + b1 & b2 & y &|| x &|| z && a2 && a1; let formatted = - a1 && a2 && b1 & b2 & y &|| x &|| z; + b1 & b2 & y &|| x &|| z && a2 && a1; /** * Now, let's try an example that resembles the above, yet would require * parenthesis everywhere. */ -/* &...(left) is higher than &(right). &(right) is equal to &&(right) */ let parseTree = - ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); + x &|| z &|| (y & (b2 & (b1 && (a1 && a2)))); let minParens = - ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); + x &|| z &|| (y & (b2 & (b1 && (a1 && a2)))); let formatted = - ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); + x &|| z &|| (y & (b2 & (b1 && (a1 && a2)))); /* **...(right) is higher than *...(left) */ let parseTree = b1 *| b2 *| (y **| (x **| z)); @@ -149,13 +147,13 @@ first + second + third; first & second & third; /* This one *shouldn't* */ -(first & second) & third; +first & (second & third); /* || is basically the same as &/&& */ first || second || third; /* This one *shouldn't* */ -(first || second) || third; +first || (second || third); /* No parens should be added/removed from the following when formatting */ let seeWhichCharacterHasHigherPrecedence = @@ -328,7 +326,7 @@ let shouldRemoveParens = ident ++ ident ++ ident; let shouldPreserveParens = ident + (ident + ident); let shouldPreserveParens = - (ident ++ ident) ++ ident; + ident ++ (ident ++ ident); /** * Since ++ is now INFIXOP1, it should have lower priority than INFIXOP2 (which * includes the single plus sign). That means no parens are required in the @@ -365,11 +363,11 @@ let parensRequired = ident + (ident +++ ident); let parensRequired = ident + (ident ++- ident); let parensRequired = ident +$ (ident ++- ident); -/* ++ and +++ have the same parsing precedence, so it's right associative. - * Parens are required if you want to group to the left, even when the tokens +/* ++ and +++ have the same parsing precedence, so it's left associative. + * Parens are required if you want to group to the right, even when the tokens * are different.*/ -let parensRequired = (ident ++ ident) +++ ident; -let parensRequired = (ident +++ ident) ++ ident; +let parensRequired = ident ++ (ident +++ ident); +let parensRequired = ident +++ (ident ++ ident); /* Add tests with IF/then mixed with infix/constructor application on left and right sides */ /** diff --git a/formatTest/unit_tests/input/infix.re b/formatTest/unit_tests/input/infix.re index 402922bdf..437e851d7 100644 --- a/formatTest/unit_tests/input/infix.re +++ b/formatTest/unit_tests/input/infix.re @@ -69,23 +69,21 @@ let minParens = a1 := a2 := b1 === ((b2 === y) !== x !== z); let formatted = a1 := a2 := b1 === ((b2 === y) !== x !== z); -/* &...(left) is higher than &(right). &(right) is equal to &&(right) */ -let parseTree = a1 && (a2 && (b1 & b2 & y &|| x &|| z)); +let parseTree = ((b1 & b2 & y &|| x &|| z) && a2) && a1; -let minParens = a1 && a2 && (b1 & b2 & y &|| x &|| z); +let minParens = (b1 & b2 & y &|| x &|| z) && a2 && a1; -let formatted = a1 && a2 && (b1 & b2 & y &|| x &|| z); +let formatted = (b1 & b2 & y &|| x &|| z) && a2 && a1; /** * Now, let's try an example that resembles the above, yet would require * parenthesis everywhere. */ -/* &...(left) is higher than &(right). &(right) is equal to &&(right) */ -let parseTree = ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); +let parseTree = (x &|| z) &|| (y & (b2 & (b1 && ((a1 && a2))))); -let minParens = ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); +let minParens = (x &|| z) &|| (y & (b2 & (b1 && ((a1 && a2))))); -let formatted = ((((a1 && a2) && b1) & b2) & y) &|| (x &|| z); +let formatted = (x &|| z) &|| (y & (b2 & (b1 && ((a1 && a2))))); /* **...(right) is higher than *...(left) */ let parseTree = ((b1 *| b2) *| (y *\*| (x *\*| z))); @@ -124,13 +122,13 @@ first + second + third; first & second & third; /* This one *shouldn't* */ -(first & second) & third; +first & (second & third); /* || is basically the same as &/&& */ first || second || third; /* This one *shouldn't* */ -(first || second) || third; +first || (second || third); /* No parens should be added/removed from the following when formatting */ let seeWhichCharacterHasHigherPrecedence = (first |> second |> third) ^> fourth; @@ -267,9 +265,9 @@ let shouldSimplifyAnythingExceptApplicationAndConstruction = call("hi") ++ (swit | _ => "hi" }) ++ "yo"; let shouldRemoveParens = (ident + ident) + ident; -let shouldRemoveParens = ident ++ (ident ++ ident); +let shouldRemoveParens = (ident ++ ident) ++ ident; let shouldPreserveParens = ident + (ident + ident); -let shouldPreserveParens = (ident ++ ident) ++ ident; +let shouldPreserveParens = ident ++ (ident ++ ident); /** * Since ++ is now INFIXOP1, it should have lower priority than INFIXOP2 (which * includes the single plus sign). That means no parens are required in the @@ -304,11 +302,11 @@ let parensRequired = ident + (ident +++ ident); let parensRequired = ident + (ident ++- ident); let parensRequired = ident +$ (ident ++- ident); -/* ++ and +++ have the same parsing precedence, so it's right associative. - * Parens are required if you want to group to the left, even when the tokens +/* ++ and +++ have the same parsing precedence, so it's left associative. + * Parens are required if you want to group to the right, even when the tokens * are different.*/ -let parensRequired = (ident ++ ident) +++ ident; -let parensRequired = (ident +++ ident) ++ ident; +let parensRequired = ident ++ (ident +++ ident); +let parensRequired = ident +++ (ident ++ ident); /* Add tests with IF/then mixed with infix/constructor application on left and right sides */ /** diff --git a/src/reason-parser/reason_declarative_lexer.mll b/src/reason-parser/reason_declarative_lexer.mll index 720dacaa1..deea4afc5 100644 --- a/src/reason-parser/reason_declarative_lexer.mll +++ b/src/reason-parser/reason_declarative_lexer.mll @@ -606,8 +606,8 @@ rule token state = parse | "^" -> POSTFIXOP "^" | op -> INFIXOP1 (unescape_operator op) } - | "++" operator_chars* - { INFIXOP1 (lexeme_operator lexbuf) } + | "++" + { PLUSPLUS } | '\\'? ['+' '-'] operator_chars* { INFIXOP2 (lexeme_operator lexbuf) } (* SLASHGREATER is an INFIXOP3 that is handled specially *) diff --git a/src/reason-parser/reason_parser.mly b/src/reason-parser/reason_parser.mly index aa98fd8d1..536e09783 100644 --- a/src/reason-parser/reason_parser.mly +++ b/src/reason-parser/reason_parser.mly @@ -1174,6 +1174,7 @@ let add_brace_attr expr = (* %token PARSER *) %token PERCENT %token PLUS +%token PLUSPLUS %token PLUSDOT %token PLUSEQ %token PREFIXOP [@recover.expr ""] [@recover.cost 2] @@ -1244,13 +1245,13 @@ conflicts. %nonassoc below_BAR (* Allows "building up" of many bars *) %left BAR (* pattern (p|p|p) *) -%right OR BARBAR (* expr (e || e || e) *) -%right AMPERSAND AMPERAMPER (* expr (e && e && e) *) +%left OR BARBAR (* expr (e || e || e) *) +%left AMPERSAND AMPERAMPER (* expr (e && e && e) *) %left INFIXOP0 LESS GREATER GREATERDOTDOTDOT (* expr (e OP e OP e) *) %left LESSDOTDOTGREATER (* expr (e OP e OP e) *) %right INFIXOP1 (* expr (e OP e OP e) *) -%right COLONCOLON (* expr (e :: e :: e) *) -%left INFIXOP2 PLUS PLUSDOT MINUS MINUSDOT PLUSEQ (* expr (e OP e OP e) *) +%left COLONCOLON (* expr (e :: e :: e) *) +%left INFIXOP2 PLUS PLUSDOT PLUSPLUS MINUS MINUSDOT PLUSEQ (* expr (e OP e OP e) *) %left PERCENT INFIXOP3 SLASHGREATER STAR (* expr (e OP e OP e) *) %right INFIXOP4 (* expr (e OP e OP e) *) @@ -4681,6 +4682,7 @@ val_ident: (* SLASHGREATER is INFIXOP3 but we needed to call it out specially *) | SLASHGREATER { "/>" } | INFIXOP4 { $1 } + | PLUSPLUS { "++" } | PLUS { "+" } | PLUSDOT { "+." } | MINUS { "-" } diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 06fe241b1..ade1ad39b 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -406,7 +406,7 @@ let rec sequentialIfBlocks x = (* Table 2.1. Precedence and associativity. - Precedence from highest to lowest: From RWOC, modified to include != + Precedence from highest to lowest: From RWOC, modified to include !=, and modified to make &, &&, or and || left associative. --------------------------------------- Operator prefix Associativity @@ -424,8 +424,8 @@ let rec sequentialIfBlocks x = =..., <..., >..., |..., &..., $... Left associative (INFIXOP0) =, <, > Left associative (IN SAME row as INFIXOP0 listed after) --- - &, && Right associative - or, || Right associative + &, && Left associative + or, || Left associative , - :=, = Right associative if - @@ -590,12 +590,12 @@ let rules = [ (TokenPrecedence, (fun s -> (Left, s = "!" ))); ]; [ - (TokenPrecedence, (fun s -> (Right, s = "::"))); + (TokenPrecedence, (fun s -> (Left, s = "::"))); ]; [ (TokenPrecedence, (fun s -> (Right, s.[0] == '@'))); (TokenPrecedence, (fun s -> (Right, s.[0] == '^'))); - (TokenPrecedence, (fun s -> (Right, String.length s > 1 && s.[0] == '+' && s.[1] == '+'))); + (TokenPrecedence, (fun s -> (Left, String.length s > 1 && s.[0] == '+' && s.[1] == '+'))); ]; [ (TokenPrecedence, (fun s -> (Left, s.[0] == '=' && not (s = "=") && not (s = "=>")))); @@ -615,12 +615,12 @@ let rules = [ (CustomPrecedence, (fun s -> (Left, s = funToken))); ]; [ - (TokenPrecedence, (fun s -> (Right, s = "&"))); - (TokenPrecedence, (fun s -> (Right, s = "&&"))); + (TokenPrecedence, (fun s -> (Left, s = "&"))); + (TokenPrecedence, (fun s -> (Left, s = "&&"))); ]; [ - (TokenPrecedence, (fun s -> (Right, s = "or"))); - (TokenPrecedence, (fun s -> (Right, s = "||"))); + (TokenPrecedence, (fun s -> (Left, s = "or"))); + (TokenPrecedence, (fun s -> (Left, s = "||"))); ]; [ (* The Left shouldn't ever matter in practice. Should never get in a