From 2486ce0e483966f9b860af86bcf3499c74d53be1 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 2 Sep 2024 11:54:34 -0300 Subject: [PATCH 1/5] fmt: Add parens around `as` patterns in arguments --- crates/compiler/fmt/src/expr.rs | 43 ++++++++++++++ .../pass/arg_pattern_as.expr.formatted.roc | 2 + .../pass/arg_pattern_as.expr.result-ast | 57 +++++++++++++++++++ .../snapshots/pass/arg_pattern_as.expr.roc | 2 + .../test_syntax/tests/test_snapshots.rs | 1 + 5 files changed, 105 insertions(+) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.roc diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index fad6f02a4d..53670a54e9 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -1240,8 +1240,51 @@ fn fmt_closure<'a>( let mut it = loc_patterns.iter().peekable(); while let Some(loc_pattern) = it.next() { + let requires_parens = { + let mut curr = loc_pattern.value; + loop { + match curr { + Pattern::SpaceAfter(pattern, _) | Pattern::SpaceBefore(pattern, _) => { + curr = *pattern; + } + Pattern::As(_, _) => { + break true; + } + Pattern::Identifier { .. } + | Pattern::QualifiedIdentifier { .. } + | Pattern::Tag(_) + | Pattern::OpaqueRef(_) + | Pattern::Apply(_, _) + | Pattern::RecordDestructure(_) + | Pattern::Tuple(_) + | Pattern::Underscore(_) + | Pattern::RequiredField(_, _) + | Pattern::OptionalField(_, _) + | Pattern::NumLiteral(_) + | Pattern::NonBase10Literal { .. } + | Pattern::FloatLiteral(_) + | Pattern::StrLiteral(_) + | Pattern::SingleQuote(_) + | Pattern::List(_) + | Pattern::ListRest(_) + | Pattern::Malformed(_) + | Pattern::MalformedIdent(_, _) => { + break false; + } + } + } + }; + + if requires_parens { + buf.push('('); + } + loc_pattern.format(buf, indent); + if requires_parens { + buf.push(')'); + } + if it.peek().is_some() { buf.indent(indent); if arguments_are_multiline { diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.formatted.roc new file mode 100644 index 0000000000..f0657bfe44 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.formatted.roc @@ -0,0 +1,2 @@ +\({ x, y } as point), (@Location inner as outer) -> + crash "" \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.result-ast new file mode 100644 index 0000000000..dd339a58e5 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.result-ast @@ -0,0 +1,57 @@ +SpaceAfter( + Closure( + [ + @2-19 As( + @2-10 RecordDestructure( + [ + @4-5 Identifier { + ident: "x", + }, + @7-8 Identifier { + ident: "y", + }, + ], + ), + PatternAs { + spaces_before: [], + identifier: @14-19 "point", + }, + ), + @23-47 As( + @23-38 Apply( + @23-32 OpaqueRef( + "@Location", + ), + [ + @33-38 Identifier { + ident: "inner", + }, + ], + ), + PatternAs { + spaces_before: [], + identifier: @42-47 "outer", + }, + ), + ], + @56-64 SpaceBefore( + Apply( + @56-61 Crash, + [ + @62-64 Str( + PlainLine( + "", + ), + ), + ], + Space, + ), + [ + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.roc new file mode 100644 index 0000000000..33fc763c72 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/arg_pattern_as.expr.roc @@ -0,0 +1,2 @@ +\({ x, y } as point), (@Location inner as outer) -> + crash "" diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index a41dc389cd..cba55850b9 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -281,6 +281,7 @@ mod test_snapshots { pass/apply_two_args.expr, pass/apply_unary_negation.expr, pass/apply_unary_not.expr, + pass/arg_pattern_as.expr, pass/basic_apply.expr, pass/basic_docs.expr, pass/basic_field.expr, From 4e19753189775c51b9176e366d860535087290ac Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 2 Sep 2024 12:02:01 -0300 Subject: [PATCH 2/5] fmt: Use format_with_options Parens for as pattern arguments --- crates/compiler/fmt/src/annotation.rs | 1 + crates/compiler/fmt/src/expr.rs | 45 +-------------------------- crates/compiler/fmt/src/pattern.rs | 10 ++++++ 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/crates/compiler/fmt/src/annotation.rs b/crates/compiler/fmt/src/annotation.rs index b279a8fc9c..3e6b18a158 100644 --- a/crates/compiler/fmt/src/annotation.rs +++ b/crates/compiler/fmt/src/annotation.rs @@ -38,6 +38,7 @@ pub enum Parens { InFunctionType, InApply, InOperator, + InAsPattern, } /// In an AST node, do we show newlines around it diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 53670a54e9..a46141df80 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -1240,50 +1240,7 @@ fn fmt_closure<'a>( let mut it = loc_patterns.iter().peekable(); while let Some(loc_pattern) = it.next() { - let requires_parens = { - let mut curr = loc_pattern.value; - loop { - match curr { - Pattern::SpaceAfter(pattern, _) | Pattern::SpaceBefore(pattern, _) => { - curr = *pattern; - } - Pattern::As(_, _) => { - break true; - } - Pattern::Identifier { .. } - | Pattern::QualifiedIdentifier { .. } - | Pattern::Tag(_) - | Pattern::OpaqueRef(_) - | Pattern::Apply(_, _) - | Pattern::RecordDestructure(_) - | Pattern::Tuple(_) - | Pattern::Underscore(_) - | Pattern::RequiredField(_, _) - | Pattern::OptionalField(_, _) - | Pattern::NumLiteral(_) - | Pattern::NonBase10Literal { .. } - | Pattern::FloatLiteral(_) - | Pattern::StrLiteral(_) - | Pattern::SingleQuote(_) - | Pattern::List(_) - | Pattern::ListRest(_) - | Pattern::Malformed(_) - | Pattern::MalformedIdent(_, _) => { - break false; - } - } - } - }; - - if requires_parens { - buf.push('('); - } - - loc_pattern.format(buf, indent); - - if requires_parens { - buf.push(')'); - } + loc_pattern.format_with_options(buf, Parens::InAsPattern, Newlines::No, indent); if it.peek().is_some() { buf.indent(indent); diff --git a/crates/compiler/fmt/src/pattern.rs b/crates/compiler/fmt/src/pattern.rs index 015c739a58..dbde68c8fa 100644 --- a/crates/compiler/fmt/src/pattern.rs +++ b/crates/compiler/fmt/src/pattern.rs @@ -244,9 +244,19 @@ impl<'a> Formattable for Pattern<'a> { } As(pattern, pattern_as) => { + let needs_parens = parens == Parens::InAsPattern; + + if needs_parens { + buf.push('('); + } + fmt_pattern(buf, &pattern.value, indent, parens); pattern_as.format(buf, indent + INDENT); + + if needs_parens { + buf.push(')'); + } } // Space From ada24e4fd9f49f93df7245b449b00e2d1bbbec92 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 2 Sep 2024 12:17:53 -0300 Subject: [PATCH 3/5] Add mono tests for record pattern with `as` --- .../generated/as_pattern_in_closure_arg.txt | 31 +++++++++++++++++++ crates/compiler/test_mono/src/tests.rs | 13 ++++++++ 2 files changed, 44 insertions(+) create mode 100644 crates/compiler/test_mono/generated/as_pattern_in_closure_arg.txt diff --git a/crates/compiler/test_mono/generated/as_pattern_in_closure_arg.txt b/crates/compiler/test_mono/generated/as_pattern_in_closure_arg.txt new file mode 100644 index 0000000000..5d25c983bf --- /dev/null +++ b/crates/compiler/test_mono/generated/as_pattern_in_closure_arg.txt @@ -0,0 +1,31 @@ +procedure Num.19 (#Attr.2, #Attr.3): + let Num.282 : I64 = lowlevel NumAdd #Attr.2 #Attr.3; + ret Num.282; + +procedure Test.1 (Test.12): + let Test.6 : I64 = StructAtIndex 0 Test.12; + let Test.5 : I64 = StructAtIndex 1 Test.12; + let Test.3 : I64 = StructAtIndex 2 Test.12; + let Test.4 : I64 = StructAtIndex 3 Test.12; + let Test.18 : I64 = CallByName Num.19 Test.3 Test.5; + let Test.19 : I64 = CallByName Num.19 Test.4 Test.6; + let Test.17 : {I64, I64} = Struct {Test.18, Test.19}; + ret Test.17; + +procedure Test.2 (Test.9): + let Test.7 : I64 = StructAtIndex 2 Test.9; + let Test.8 : I64 = StructAtIndex 3 Test.9; + let Test.16 : {I64, I64} = CallByName Test.1 Test.9; + let Test.10 : I64 = StructAtIndex 0 Test.16; + let Test.11 : I64 = StructAtIndex 1 Test.16; + let Test.15 : {I64, I64, I64, I64} = Struct {Test.7, Test.8, Test.10, Test.11}; + ret Test.15; + +procedure Test.0 (): + let Test.20 : I64 = 4i64; + let Test.21 : I64 = 3i64; + let Test.22 : I64 = 1i64; + let Test.23 : I64 = 2i64; + let Test.14 : {I64, I64, I64, I64} = Struct {Test.20, Test.21, Test.22, Test.23}; + let Test.13 : {I64, I64, I64, I64} = CallByName Test.2 Test.14; + ret Test.13; diff --git a/crates/compiler/test_mono/src/tests.rs b/crates/compiler/test_mono/src/tests.rs index 0c69d37d31..8b2b2134f6 100644 --- a/crates/compiler/test_mono/src/tests.rs +++ b/crates/compiler/test_mono/src/tests.rs @@ -657,6 +657,19 @@ fn record_optional_field_function_use_default() { " } +#[mono_test] +fn as_pattern_in_closure_arg() { + r" + g = \{x, y, w, h} -> (x + w, y + h) + + f = \({ x, y } as box) -> + (right, bottom) = g box + (x, y, right, bottom) + + f { x: 1, y: 2, w: 3, h: 4 } + " +} + #[mono_test] fn quicksort_help() { // do we still need with_larger_debug_stack? From e2bd31a549b41d86089432139b56387d5625dbab Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 2 Sep 2024 12:23:25 -0300 Subject: [PATCH 4/5] Add mono tests for opaque pattern with `as` --- .../opaque_as_pattern_in_closure_arg.txt | 18 +++++++++++++++++ .../record_as_pattern_in_closure_arg.txt | 17 ++++++++++++++++ crates/compiler/test_mono/src/tests.rs | 20 +++++++++++++++---- 3 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 crates/compiler/test_mono/generated/opaque_as_pattern_in_closure_arg.txt create mode 100644 crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt diff --git a/crates/compiler/test_mono/generated/opaque_as_pattern_in_closure_arg.txt b/crates/compiler/test_mono/generated/opaque_as_pattern_in_closure_arg.txt new file mode 100644 index 0000000000..834b3ee0f5 --- /dev/null +++ b/crates/compiler/test_mono/generated/opaque_as_pattern_in_closure_arg.txt @@ -0,0 +1,18 @@ +procedure Num.21 (#Attr.2, #Attr.3): + let Num.281 : U64 = lowlevel NumMul #Attr.2 #Attr.3; + ret Num.281; + +procedure Test.2 (Test.8): + let Test.14 : U64 = 2i64; + let Test.13 : U64 = CallByName Num.21 Test.8 Test.14; + ret Test.13; + +procedure Test.3 (Test.7): + let Test.12 : U64 = CallByName Test.2 Test.7; + let Test.11 : {U64, U64} = Struct {Test.7, Test.12}; + ret Test.11; + +procedure Test.0 (): + let Test.10 : U64 = 42i64; + let Test.9 : {U64, U64} = CallByName Test.3 Test.10; + ret Test.9; diff --git a/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt b/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt new file mode 100644 index 0000000000..4369344661 --- /dev/null +++ b/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt @@ -0,0 +1,17 @@ +procedure Test.2 (Test.9): + let Test.7 : I64 = StructAtIndex 2 Test.9; + let Test.8 : I64 = StructAtIndex 3 Test.9; + let Test.17 : {I64, I64, I64, I64} = CallByName Test.2 Test.9; + let Test.10 : I64 = StructAtIndex 0 Test.17; + let Test.11 : I64 = StructAtIndex 1 Test.17; + let Test.16 : {I64, I64, I64, I64} = Struct {Test.7, Test.8, Test.10, Test.11}; + ret Test.16; + +procedure Test.0 (): + let Test.18 : I64 = 4i64; + let Test.19 : I64 = 3i64; + let Test.20 : I64 = 1i64; + let Test.21 : I64 = 2i64; + let Test.14 : {I64, I64, I64, I64} = Struct {Test.18, Test.19, Test.20, Test.21}; + let Test.13 : {I64, I64, I64, I64} = CallByName Test.2 Test.14; + ret Test.13; diff --git a/crates/compiler/test_mono/src/tests.rs b/crates/compiler/test_mono/src/tests.rs index 8b2b2134f6..e23c4789ff 100644 --- a/crates/compiler/test_mono/src/tests.rs +++ b/crates/compiler/test_mono/src/tests.rs @@ -658,15 +658,27 @@ fn record_optional_field_function_use_default() { } #[mono_test] -fn as_pattern_in_closure_arg() { +fn record_as_pattern_in_closure_arg() { r" - g = \{x, y, w, h} -> (x + w, y + h) + f = \{x, y, w, h} -> (x + w, y + h) - f = \({ x, y } as box) -> + g = \({ x, y } as box) -> (right, bottom) = g box (x, y, right, bottom) - f { x: 1, y: 2, w: 3, h: 4 } + g { x: 1, y: 2, w: 3, h: 4 } + " +} + +#[mono_test] +fn opaque_as_pattern_in_closure_arg() { + r" + Opaque := U64 + + f = \(@Opaque x) -> x * 2 + g = \(@Opaque x as s) -> (x, f s) + + g (@Opaque 42) " } From 458878dbeafd94c28f0407ef99f791c8e743b446 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 2 Sep 2024 12:29:14 -0300 Subject: [PATCH 5/5] Fix record as pattern mono test --- .../record_as_pattern_in_closure_arg.txt | 34 +++++++++++++------ crates/compiler/test_mono/src/tests.rs | 2 +- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt b/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt index 4369344661..5d25c983bf 100644 --- a/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt +++ b/crates/compiler/test_mono/generated/record_as_pattern_in_closure_arg.txt @@ -1,17 +1,31 @@ +procedure Num.19 (#Attr.2, #Attr.3): + let Num.282 : I64 = lowlevel NumAdd #Attr.2 #Attr.3; + ret Num.282; + +procedure Test.1 (Test.12): + let Test.6 : I64 = StructAtIndex 0 Test.12; + let Test.5 : I64 = StructAtIndex 1 Test.12; + let Test.3 : I64 = StructAtIndex 2 Test.12; + let Test.4 : I64 = StructAtIndex 3 Test.12; + let Test.18 : I64 = CallByName Num.19 Test.3 Test.5; + let Test.19 : I64 = CallByName Num.19 Test.4 Test.6; + let Test.17 : {I64, I64} = Struct {Test.18, Test.19}; + ret Test.17; + procedure Test.2 (Test.9): let Test.7 : I64 = StructAtIndex 2 Test.9; let Test.8 : I64 = StructAtIndex 3 Test.9; - let Test.17 : {I64, I64, I64, I64} = CallByName Test.2 Test.9; - let Test.10 : I64 = StructAtIndex 0 Test.17; - let Test.11 : I64 = StructAtIndex 1 Test.17; - let Test.16 : {I64, I64, I64, I64} = Struct {Test.7, Test.8, Test.10, Test.11}; - ret Test.16; + let Test.16 : {I64, I64} = CallByName Test.1 Test.9; + let Test.10 : I64 = StructAtIndex 0 Test.16; + let Test.11 : I64 = StructAtIndex 1 Test.16; + let Test.15 : {I64, I64, I64, I64} = Struct {Test.7, Test.8, Test.10, Test.11}; + ret Test.15; procedure Test.0 (): - let Test.18 : I64 = 4i64; - let Test.19 : I64 = 3i64; - let Test.20 : I64 = 1i64; - let Test.21 : I64 = 2i64; - let Test.14 : {I64, I64, I64, I64} = Struct {Test.18, Test.19, Test.20, Test.21}; + let Test.20 : I64 = 4i64; + let Test.21 : I64 = 3i64; + let Test.22 : I64 = 1i64; + let Test.23 : I64 = 2i64; + let Test.14 : {I64, I64, I64, I64} = Struct {Test.20, Test.21, Test.22, Test.23}; let Test.13 : {I64, I64, I64, I64} = CallByName Test.2 Test.14; ret Test.13; diff --git a/crates/compiler/test_mono/src/tests.rs b/crates/compiler/test_mono/src/tests.rs index e23c4789ff..3e3fbad4e5 100644 --- a/crates/compiler/test_mono/src/tests.rs +++ b/crates/compiler/test_mono/src/tests.rs @@ -663,7 +663,7 @@ fn record_as_pattern_in_closure_arg() { f = \{x, y, w, h} -> (x + w, y + h) g = \({ x, y } as box) -> - (right, bottom) = g box + (right, bottom) = f box (x, y, right, bottom) g { x: 1, y: 2, w: 3, h: 4 }