From 3b222fe8e86a132074d8bd9c2f5dc1a1d03f1b51 Mon Sep 17 00:00:00 2001 From: Marcus Roberts Date: Sat, 30 Jul 2022 12:14:35 +0100 Subject: [PATCH 1/4] Format rational numbers with parens when taking part in an infix divide --- compiler/src/formatting/format.re | 30 ++++++++++++++- compiler/test/stdlib/number.test.gr | 59 ++++++++++++++++------------- 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/compiler/src/formatting/format.re b/compiler/src/formatting/format.re index 3c977ef078..78394bc55f 100644 --- a/compiler/src/formatting/format.re +++ b/compiler/src/formatting/format.re @@ -78,6 +78,22 @@ let get_original_code = (location: Location.t, source: array(string)) => { }; }; +let is_rational_number = (expr: Parsetree.expression_desc) => { + switch (expr) { + | PExpConstant(const) => + switch (const) { + | PConstNumber(nt) => + switch (nt) { + | PConstNumberRational(_, _) => true + | _ => false + } + | _ => false + } + + | _ => false + }; +}; + // Be AWARE! This is only to be called when you know the comments list is not empty. // Moved here in case we want to change the implementation in future let get_last_item_in_list = comments => @@ -1691,6 +1707,14 @@ and print_infix_application = | _ => false }; + // wrap rational numbers in params when in an divide infix operation + // to ensure correct precedence + + let left_is_rational = + function_name == "/" && is_rational_number(first.pexp_desc); + let right_is_rational = + function_name == "/" && is_rational_number(second.pexp_desc); + let (left_grouping_required, right_grouping_required) = switch (first.pexp_desc, second.pexp_desc) { | (PExpApp(fn1, _), PExpApp(fn2, _)) => @@ -1726,8 +1750,10 @@ and print_infix_application = | _ => (false, false) }; - let left_needs_parens = left_is_if || left_grouping_required; - let right_needs_parens = right_is_if || right_grouping_required; + let left_needs_parens = + left_is_if || left_is_rational || left_grouping_required; + let right_needs_parens = + right_is_if || right_is_rational || right_grouping_required; let wrapped_left = if (left_needs_parens) { diff --git a/compiler/test/stdlib/number.test.gr b/compiler/test/stdlib/number.test.gr index a7b0c2bf9f..a812b28125 100644 --- a/compiler/test/stdlib/number.test.gr +++ b/compiler/test/stdlib/number.test.gr @@ -21,7 +21,7 @@ assert Number.add(25, 5) == 30 assert 2/3 + 4 == 14/3 assert 2/3 + Int32.toNumber(4l) == 14/3 assert 2/3 + Int64.toNumber(4L) == 14/3 - assert 4 + (2/3) == 14/3 + assert 4 + 2/3 == 14/3 assert Int32.toNumber(4l) + 2/3 == 14/3 assert Int64.toNumber(4L) + 2/3 == 14/3 assert 2/3 + 4.0 == 4.666666666666667 @@ -34,25 +34,25 @@ assert Number.sub(25, 5) == 20 assert 2/3 - 4 == -10/3 assert 2/3 - Int32.toNumber(4l) == -10/3 assert 2/3 - Int64.toNumber(4L) == -10/3 - assert 4 - (2/3) == 10/3 + assert 4 - 2/3 == 10/3 assert Int32.toNumber(4l) - 2/3 == 10/3 assert Int64.toNumber(4L) - 2/3 == 10/3 - assert (2/3) - 4.0 == -3.3333333333333335 - assert 4.0 - (2/3) == 3.3333333333333335 + assert 2/3 - 4.0 == -3.3333333333333335 + assert 4.0 - 2/3 == 3.3333333333333335 } // mul assert Number.mul(5, 5) == 25 // Rational mult tests { let (*) = Number.mul - assert (2/3) * 4 == 8/3 - assert (2/3) * Int32.toNumber(4l) == 8/3 - assert (2/3) * Int64.toNumber(4L) == 8/3 - assert 4 * (2/3) == 8/3 - assert Int32.toNumber(4l) * (2/3) == 8/3 - assert Int64.toNumber(4L) * (2/3) == 8/3 - assert (2/3) * 4.0 < 2.666666666666667 - assert 2.6666666666666 < (2/3) * 4.0 + assert 2/3 * 4 == 8/3 + assert 2/3 * Int32.toNumber(4l) == 8/3 + assert 2/3 * Int64.toNumber(4L) == 8/3 + assert 4 * 2 / 3 == 8/3 + assert Int32.toNumber(4l) * 2 / 3 == 8/3 + assert Int64.toNumber(4L) * 2 / 3 == 8/3 + assert 2/3 * 4.0 < 2.666666666666667 + assert 2.6666666666666 < 2/3 * 4.0 } // div assert Number.div(25, 5) == 5 @@ -65,7 +65,7 @@ assert Number.div(25, 5) == 5 assert 4 / (2/3) == 6 assert Int32.toNumber(4l) / (2/3) == 6 assert Int64.toNumber(4L) / (2/3) == 6 - assert (2/3) / 4.0 == 0.16666666666666666 + assert (2/3) / 4.0 == 0.16666666666666666 assert 4.0 / (2/3) == 6.0 } // sqrt @@ -86,7 +86,8 @@ assert Number.max(5, 6) == 6 assert Number.max(1/2, 1/4) == 1/2 assert Number.max(0.5, 0.25) == 0.5 assert Number.max(BI.toNumber(1234t), BI.toNumber(12t)) == BI.toNumber(1234t) -assert Number.max(355894508425808343204914141312, 6) == 355894508425808343204914141312 +assert Number.max(355894508425808343204914141312, 6) == +355894508425808343204914141312 // ceil assert Number.ceil(-25.5) == -25 assert Number.ceil(25.5) == 26 @@ -124,9 +125,9 @@ assert Number.neg(BI.toNumber(1234t)) == BI.toNumber(-1234t) assert Number.neg(BI.toNumber(-1234t)) == BI.toNumber(1234t) // isFinite -assert Number.isFinite(0.0/0.0) == false // NaN -assert Number.isFinite(1.0/0.0) == false // infinity -assert Number.isFinite(-1.0/0.0) == false // -infinity +assert Number.isFinite(0.0 / 0.0) == false // NaN +assert Number.isFinite(1.0 / 0.0) == false // infinity +assert Number.isFinite(-1.0 / 0.0) == false // -infinity assert Number.isFinite(1) assert Number.isFinite(1.0) assert Number.isFinite(0) @@ -140,7 +141,7 @@ assert Number.isFinite(-1/2) assert Number.isFinite(BI.toNumber(-141435902485091384901384t)) // isNaN -assert Number.isNaN(0.0/0.0) +assert Number.isNaN(0.0 / 0.0) assert Number.isNaN(1) == false assert Number.isNaN(1.0) == false assert Number.isNaN(0) == false @@ -151,14 +152,14 @@ assert Number.isNaN(25.76) == false assert Number.isNaN(-25.00) == false assert Number.isNaN(1/2) == false assert Number.isNaN(-1/2) == false -assert Number.isNaN(1.0/0.0) == false // infinity -assert Number.isNaN(-1.0/0.0) == false // -infinity +assert Number.isNaN(1.0 / 0.0) == false // infinity +assert Number.isNaN(-1.0 / 0.0) == false // -infinity assert Number.isNaN(BI.toNumber(1t)) == false // isInfinite -assert Number.isInfinite(1.0/0.0) // infinity -assert Number.isInfinite(-1.0/0.0) // -infinity -assert Number.isInfinite(0.0/0.0) == false // NaN +assert Number.isInfinite(1.0 / 0.0) // infinity +assert Number.isInfinite(-1.0 / 0.0) // -infinity +assert Number.isInfinite(0.0 / 0.0) == false // NaN assert Number.isInfinite(1) == false assert Number.isInfinite(1.0) == false assert Number.isInfinite(0) == false @@ -178,12 +179,16 @@ assert Number.parseInt("_0___42___", 10) == Ok(42) assert Number.parseInt("-42", 10) == Ok(-42) assert Number.parseInt("-042", 10) == Ok(-42) assert Number.parseInt("-_0___42___", 10) == Ok(-42) -assert Number.parseInt("1073741823", 10) == Ok(1073741823) // grain simple number max -assert Number.parseInt("-1073741824", 10) == Ok(-1073741824) // grain simple number min +assert Number.parseInt("1073741823", 10) == +Ok(1073741823) // grain simple number max +assert Number.parseInt("-1073741824", 10) == +Ok(-1073741824) // grain simple number min assert Number.parseInt("2147483647", 10) == Ok(2147483647) // i32 max assert Number.parseInt("-2147483648", 10) == Ok(-2147483648) // i32 min -assert Number.parseInt("9223372036854775807", 10) == Ok(9223372036854775807) // i64 max -assert Number.parseInt("-9223372036854775808", 10) == Ok(-9223372036854775808) // i64 min +assert Number.parseInt("9223372036854775807", 10) == +Ok(9223372036854775807) // i64 max +assert Number.parseInt("-9223372036854775808", 10) == +Ok(-9223372036854775808) // i64 min assert Number.parseInt("0xabcdef", 10) == Ok(0xabcdef) assert Number.parseInt("0Xabcdef", 10) == Ok(0xabcdef) assert Number.parseInt("abcdef", 16) == Ok(0xabcdef) From b35bc09e1da0e4c973d26a066e99bae6ba8f8cdb Mon Sep 17 00:00:00 2001 From: Marcus Roberts Date: Sat, 30 Jul 2022 19:01:34 +0100 Subject: [PATCH 2/4] add parens based on rational being on rhs --- compiler/src/formatting/format.re | 16 ++------ compiler/test/stdlib/number.test.gr | 59 +++++++++++++---------------- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/compiler/src/formatting/format.re b/compiler/src/formatting/format.re index 78394bc55f..303208dd5a 100644 --- a/compiler/src/formatting/format.re +++ b/compiler/src/formatting/format.re @@ -1707,14 +1707,6 @@ and print_infix_application = | _ => false }; - // wrap rational numbers in params when in an divide infix operation - // to ensure correct precedence - - let left_is_rational = - function_name == "/" && is_rational_number(first.pexp_desc); - let right_is_rational = - function_name == "/" && is_rational_number(second.pexp_desc); - let (left_grouping_required, right_grouping_required) = switch (first.pexp_desc, second.pexp_desc) { | (PExpApp(fn1, _), PExpApp(fn2, _)) => @@ -1747,13 +1739,11 @@ and print_infix_application = (false, false); }; - | _ => (false, false) + | _ => (false, is_rational_number(second.pexp_desc)) }; - let left_needs_parens = - left_is_if || left_is_rational || left_grouping_required; - let right_needs_parens = - right_is_if || right_is_rational || right_grouping_required; + let left_needs_parens = left_is_if || left_grouping_required; + let right_needs_parens = right_is_if || right_grouping_required; let wrapped_left = if (left_needs_parens) { diff --git a/compiler/test/stdlib/number.test.gr b/compiler/test/stdlib/number.test.gr index a812b28125..a7b0c2bf9f 100644 --- a/compiler/test/stdlib/number.test.gr +++ b/compiler/test/stdlib/number.test.gr @@ -21,7 +21,7 @@ assert Number.add(25, 5) == 30 assert 2/3 + 4 == 14/3 assert 2/3 + Int32.toNumber(4l) == 14/3 assert 2/3 + Int64.toNumber(4L) == 14/3 - assert 4 + 2/3 == 14/3 + assert 4 + (2/3) == 14/3 assert Int32.toNumber(4l) + 2/3 == 14/3 assert Int64.toNumber(4L) + 2/3 == 14/3 assert 2/3 + 4.0 == 4.666666666666667 @@ -34,25 +34,25 @@ assert Number.sub(25, 5) == 20 assert 2/3 - 4 == -10/3 assert 2/3 - Int32.toNumber(4l) == -10/3 assert 2/3 - Int64.toNumber(4L) == -10/3 - assert 4 - 2/3 == 10/3 + assert 4 - (2/3) == 10/3 assert Int32.toNumber(4l) - 2/3 == 10/3 assert Int64.toNumber(4L) - 2/3 == 10/3 - assert 2/3 - 4.0 == -3.3333333333333335 - assert 4.0 - 2/3 == 3.3333333333333335 + assert (2/3) - 4.0 == -3.3333333333333335 + assert 4.0 - (2/3) == 3.3333333333333335 } // mul assert Number.mul(5, 5) == 25 // Rational mult tests { let (*) = Number.mul - assert 2/3 * 4 == 8/3 - assert 2/3 * Int32.toNumber(4l) == 8/3 - assert 2/3 * Int64.toNumber(4L) == 8/3 - assert 4 * 2 / 3 == 8/3 - assert Int32.toNumber(4l) * 2 / 3 == 8/3 - assert Int64.toNumber(4L) * 2 / 3 == 8/3 - assert 2/3 * 4.0 < 2.666666666666667 - assert 2.6666666666666 < 2/3 * 4.0 + assert (2/3) * 4 == 8/3 + assert (2/3) * Int32.toNumber(4l) == 8/3 + assert (2/3) * Int64.toNumber(4L) == 8/3 + assert 4 * (2/3) == 8/3 + assert Int32.toNumber(4l) * (2/3) == 8/3 + assert Int64.toNumber(4L) * (2/3) == 8/3 + assert (2/3) * 4.0 < 2.666666666666667 + assert 2.6666666666666 < (2/3) * 4.0 } // div assert Number.div(25, 5) == 5 @@ -65,7 +65,7 @@ assert Number.div(25, 5) == 5 assert 4 / (2/3) == 6 assert Int32.toNumber(4l) / (2/3) == 6 assert Int64.toNumber(4L) / (2/3) == 6 - assert (2/3) / 4.0 == 0.16666666666666666 + assert (2/3) / 4.0 == 0.16666666666666666 assert 4.0 / (2/3) == 6.0 } // sqrt @@ -86,8 +86,7 @@ assert Number.max(5, 6) == 6 assert Number.max(1/2, 1/4) == 1/2 assert Number.max(0.5, 0.25) == 0.5 assert Number.max(BI.toNumber(1234t), BI.toNumber(12t)) == BI.toNumber(1234t) -assert Number.max(355894508425808343204914141312, 6) == -355894508425808343204914141312 +assert Number.max(355894508425808343204914141312, 6) == 355894508425808343204914141312 // ceil assert Number.ceil(-25.5) == -25 assert Number.ceil(25.5) == 26 @@ -125,9 +124,9 @@ assert Number.neg(BI.toNumber(1234t)) == BI.toNumber(-1234t) assert Number.neg(BI.toNumber(-1234t)) == BI.toNumber(1234t) // isFinite -assert Number.isFinite(0.0 / 0.0) == false // NaN -assert Number.isFinite(1.0 / 0.0) == false // infinity -assert Number.isFinite(-1.0 / 0.0) == false // -infinity +assert Number.isFinite(0.0/0.0) == false // NaN +assert Number.isFinite(1.0/0.0) == false // infinity +assert Number.isFinite(-1.0/0.0) == false // -infinity assert Number.isFinite(1) assert Number.isFinite(1.0) assert Number.isFinite(0) @@ -141,7 +140,7 @@ assert Number.isFinite(-1/2) assert Number.isFinite(BI.toNumber(-141435902485091384901384t)) // isNaN -assert Number.isNaN(0.0 / 0.0) +assert Number.isNaN(0.0/0.0) assert Number.isNaN(1) == false assert Number.isNaN(1.0) == false assert Number.isNaN(0) == false @@ -152,14 +151,14 @@ assert Number.isNaN(25.76) == false assert Number.isNaN(-25.00) == false assert Number.isNaN(1/2) == false assert Number.isNaN(-1/2) == false -assert Number.isNaN(1.0 / 0.0) == false // infinity -assert Number.isNaN(-1.0 / 0.0) == false // -infinity +assert Number.isNaN(1.0/0.0) == false // infinity +assert Number.isNaN(-1.0/0.0) == false // -infinity assert Number.isNaN(BI.toNumber(1t)) == false // isInfinite -assert Number.isInfinite(1.0 / 0.0) // infinity -assert Number.isInfinite(-1.0 / 0.0) // -infinity -assert Number.isInfinite(0.0 / 0.0) == false // NaN +assert Number.isInfinite(1.0/0.0) // infinity +assert Number.isInfinite(-1.0/0.0) // -infinity +assert Number.isInfinite(0.0/0.0) == false // NaN assert Number.isInfinite(1) == false assert Number.isInfinite(1.0) == false assert Number.isInfinite(0) == false @@ -179,16 +178,12 @@ assert Number.parseInt("_0___42___", 10) == Ok(42) assert Number.parseInt("-42", 10) == Ok(-42) assert Number.parseInt("-042", 10) == Ok(-42) assert Number.parseInt("-_0___42___", 10) == Ok(-42) -assert Number.parseInt("1073741823", 10) == -Ok(1073741823) // grain simple number max -assert Number.parseInt("-1073741824", 10) == -Ok(-1073741824) // grain simple number min +assert Number.parseInt("1073741823", 10) == Ok(1073741823) // grain simple number max +assert Number.parseInt("-1073741824", 10) == Ok(-1073741824) // grain simple number min assert Number.parseInt("2147483647", 10) == Ok(2147483647) // i32 max assert Number.parseInt("-2147483648", 10) == Ok(-2147483648) // i32 min -assert Number.parseInt("9223372036854775807", 10) == -Ok(9223372036854775807) // i64 max -assert Number.parseInt("-9223372036854775808", 10) == -Ok(-9223372036854775808) // i64 min +assert Number.parseInt("9223372036854775807", 10) == Ok(9223372036854775807) // i64 max +assert Number.parseInt("-9223372036854775808", 10) == Ok(-9223372036854775808) // i64 min assert Number.parseInt("0xabcdef", 10) == Ok(0xabcdef) assert Number.parseInt("0Xabcdef", 10) == Ok(0xabcdef) assert Number.parseInt("abcdef", 16) == Ok(0xabcdef) From b86ce0bf00d17b3a913f2288cf09ae6693046adf Mon Sep 17 00:00:00 2001 From: Marcus Roberts Date: Sat, 30 Jul 2022 19:08:46 +0100 Subject: [PATCH 3/4] Add tests --- compiler/test/formatter_inputs/rationals.gr | 5 +++++ compiler/test/formatter_outputs/rationals.gr | 5 +++++ compiler/test/suites/formatter.re | 1 + 3 files changed, 11 insertions(+) create mode 100644 compiler/test/formatter_inputs/rationals.gr create mode 100644 compiler/test/formatter_outputs/rationals.gr diff --git a/compiler/test/formatter_inputs/rationals.gr b/compiler/test/formatter_inputs/rationals.gr new file mode 100644 index 0000000000..5124e98e79 --- /dev/null +++ b/compiler/test/formatter_inputs/rationals.gr @@ -0,0 +1,5 @@ +let x = 3 * (2/3) + +let y = 2/3 + 4 + +assert 4 * (2/3) == 8/3 diff --git a/compiler/test/formatter_outputs/rationals.gr b/compiler/test/formatter_outputs/rationals.gr new file mode 100644 index 0000000000..5124e98e79 --- /dev/null +++ b/compiler/test/formatter_outputs/rationals.gr @@ -0,0 +1,5 @@ +let x = 3 * (2/3) + +let y = 2/3 + 4 + +assert 4 * (2/3) == 8/3 diff --git a/compiler/test/suites/formatter.re b/compiler/test/suites/formatter.re index 24c7cfd7a7..579388480c 100644 --- a/compiler/test/suites/formatter.re +++ b/compiler/test/suites/formatter.re @@ -45,4 +45,5 @@ describe("formatter", ({test, testSkip}) => { assertFormatOutput("parens", "parens"); assertFormatOutput("windows", "windows"); assertFormatOutput("patterns", "patterns"); + assertFormatOutput("rationals", "rationals"); }); From 44c10df0aeebb567c466d5db19d2850843ca77d9 Mon Sep 17 00:00:00 2001 From: Oscar Spencer Date: Sun, 31 Jul 2022 11:40:36 -0400 Subject: [PATCH 4/4] Handle all rational cases --- compiler/src/formatting/format.re | 65 ++++++-------------- compiler/test/formatter_inputs/rationals.gr | 10 +++ compiler/test/formatter_outputs/rationals.gr | 10 +++ 3 files changed, 38 insertions(+), 47 deletions(-) diff --git a/compiler/src/formatting/format.re b/compiler/src/formatting/format.re index 303208dd5a..43164f83d7 100644 --- a/compiler/src/formatting/format.re +++ b/compiler/src/formatting/format.re @@ -78,22 +78,6 @@ let get_original_code = (location: Location.t, source: array(string)) => { }; }; -let is_rational_number = (expr: Parsetree.expression_desc) => { - switch (expr) { - | PExpConstant(const) => - switch (const) { - | PConstNumber(nt) => - switch (nt) { - | PConstNumberRational(_, _) => true - | _ => false - } - | _ => false - } - - | _ => false - }; -}; - // Be AWARE! This is only to be called when you know the comments list is not empty. // Moved here in case we want to change the implementation in future let get_last_item_in_list = comments => @@ -1707,39 +1691,26 @@ and print_infix_application = | _ => false }; - let (left_grouping_required, right_grouping_required) = - switch (first.pexp_desc, second.pexp_desc) { - | (PExpApp(fn1, _), PExpApp(fn2, _)) => - let left_prec = op_precedence(get_function_name(fn1)); - let right_prec = op_precedence(get_function_name(fn2)); - let parent_prec = op_precedence(function_name); - - // the equality check is needed for the function on the right - // as we process from the left by default when the same prededence - - let needed_left = left_prec < parent_prec; - let needed_right = right_prec <= parent_prec; - - (needed_left, needed_right); + let parent_prec = op_precedence(function_name); - | (PExpApp(fn1, _), _) => - let left_prec = op_precedence(get_function_name(fn1)); - let parent_prec = op_precedence(function_name); - if (left_prec < parent_prec) { - (true, false); - } else { - (false, false); - }; - | (_, PExpApp(fn2, _)) => - let parent_prec = op_precedence(function_name); - let right_prec = op_precedence(get_function_name(fn2)); - if (right_prec <= parent_prec) { - (false, true); - } else { - (false, false); - }; + let left_grouping_required = + switch (first.pexp_desc) { + | PExpApp(fn1, _) => + op_precedence(get_function_name(fn1)) < parent_prec + | PExpConstant(PConstNumber(PConstNumberRational(_, _))) => + op_precedence("/") < parent_prec + | _ => false + }; - | _ => (false, is_rational_number(second.pexp_desc)) + let right_grouping_required = + // the equality check is needed for the value on the right + // as we process from the left by default when the same prededence + switch (second.pexp_desc) { + | PExpApp(fn1, _) => + op_precedence(get_function_name(fn1)) <= parent_prec + | PExpConstant(PConstNumber(PConstNumberRational(_, _))) => + op_precedence("/") <= parent_prec + | _ => false }; let left_needs_parens = left_is_if || left_grouping_required; diff --git a/compiler/test/formatter_inputs/rationals.gr b/compiler/test/formatter_inputs/rationals.gr index 5124e98e79..6ca3a5d954 100644 --- a/compiler/test/formatter_inputs/rationals.gr +++ b/compiler/test/formatter_inputs/rationals.gr @@ -1,5 +1,15 @@ +import Int32 from "int32" + +let a = Int32.toNumber(3l) * (2/3) + +let b = 2/3 + Int32.toNumber(4l) + +let c = Int32.toNumber(3l) + (2/3) + let x = 3 * (2/3) let y = 2/3 + 4 +let z = 3 + (2/3) + assert 4 * (2/3) == 8/3 diff --git a/compiler/test/formatter_outputs/rationals.gr b/compiler/test/formatter_outputs/rationals.gr index 5124e98e79..9ab3eb56d8 100644 --- a/compiler/test/formatter_outputs/rationals.gr +++ b/compiler/test/formatter_outputs/rationals.gr @@ -1,5 +1,15 @@ +import Int32 from "int32" + +let a = Int32.toNumber(3l) * (2/3) + +let b = 2/3 + Int32.toNumber(4l) + +let c = Int32.toNumber(3l) + 2/3 + let x = 3 * (2/3) let y = 2/3 + 4 +let z = 3 + 2/3 + assert 4 * (2/3) == 8/3