Skip to content

Commit

Permalink
Merge pull request #6968 from smores56/ignored-record-builder-fields
Browse files Browse the repository at this point in the history 
Ignore underscore-prefixed fields in record builders
  • Loading branch information
@smores56
smores56 authored Aug 7, 2024
2 parents 8032a98 + 00bc699 commit 698bbc3
Show file tree
Hide file tree
Showing 23 changed files with 448 additions and 143 deletions.
4 changes: 2 additions & 2 deletions crates/cli/tests/cli/combine-tasks.roc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ main =
a: Task.ok 123,
b: Task.ok "abc",
c: Task.ok [123],
d: Task.ok ["abc"],
e: Task.ok (Dict.single "a" "b"),
_d: Task.ok ["abc"],
_: Task.ok (Dict.single "a" "b"),
}!

Stdout.line! "For multiple tasks: $(Inspect.toStr multipleIn)"
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/tests/cli_run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ mod cli_run {
&[],
&[],
&[],
"For multiple tasks: {a: 123, b: \"abc\", c: [123], d: [\"abc\"], e: {\"a\": \"b\"}}\n",
"For multiple tasks: {a: 123, b: \"abc\", c: [123]}\n",
UseValgrind::No,
TestCliCommands::Run,
)
Expand Down
4 changes: 3 additions & 1 deletion crates/compiler/can/src/annotation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ pub fn find_type_def_symbols(
while let Some(assigned_field) = inner_stack.pop() {
match assigned_field {
AssignedField::RequiredValue(_, _, t)
| AssignedField::OptionalValue(_, _, t) => {
| AssignedField::OptionalValue(_, _, t)
| AssignedField::IgnoredValue(_, _, t) => {
stack.push(&t.value);
}
AssignedField::LabelOnly(_) => {}
Expand Down Expand Up @@ -1386,6 +1387,7 @@ fn can_assigned_fields<'a>(

break 'inner label;
}
IgnoredValue(_, _, _) => unreachable!(),
LabelOnly(loc_field_name) => {
// Interpret { a, b } as { a : a, b : b }
let field_name = Lowercase::from(loc_field_name.value);
Expand Down
4 changes: 3 additions & 1 deletion crates/compiler/can/src/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,9 @@ fn canonicalize_claimed_ability_impl<'a>(
// An error will already have been reported
Err(())
}
AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => {
AssignedField::SpaceBefore(_, _)
| AssignedField::SpaceAfter(_, _)
| AssignedField::IgnoredValue(_, _, _) => {
internal_error!("unreachable")
}
}
Expand Down
179 changes: 109 additions & 70 deletions crates/compiler/can/src/desugar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,44 +521,68 @@ pub fn desugar_expr<'a>(
});
}

let mut field_names = Vec::with_capacity_in(fields.len(), arena);
let mut field_vals = Vec::with_capacity_in(fields.len(), arena);
struct FieldData<'d> {
name: Loc<&'d str>,
value: &'d Loc<Expr<'d>>,
ignored: bool,
}

let mut field_data = Vec::with_capacity_in(fields.len(), arena);

for field in fields.items {
match desugar_field(arena, &field.value, src, line_info, module_path) {
AssignedField::RequiredValue(loc_name, _, loc_val) => {
field_names.push(loc_name);
field_vals.push(loc_val);
}
AssignedField::LabelOnly(loc_name) => {
field_names.push(loc_name);
field_vals.push(arena.alloc(Loc {
region: loc_name.region,
value: Expr::Var {
module_name: "",
ident: loc_name.value,
},
}));
}
AssignedField::OptionalValue(loc_name, _, loc_val) => {
return arena.alloc(Loc {
region: loc_expr.region,
value: OptionalFieldInRecordBuilder(arena.alloc(loc_name), loc_val),
});
}
AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => {
unreachable!("Should have been desugared in `desugar_field`")
}
AssignedField::Malformed(_name) => {}
}
let (name, value, ignored) =
match desugar_field(arena, &field.value, src, line_info, module_path) {
AssignedField::RequiredValue(loc_name, _, loc_val) => {
(loc_name, loc_val, false)
}
AssignedField::IgnoredValue(loc_name, _, loc_val) => {
(loc_name, loc_val, true)
}
AssignedField::LabelOnly(loc_name) => (
loc_name,
&*arena.alloc(Loc {
region: loc_name.region,
value: Expr::Var {
module_name: "",
ident: loc_name.value,
},
}),
false,
),
AssignedField::OptionalValue(loc_name, _, loc_val) => {
return arena.alloc(Loc {
region: loc_expr.region,
value: OptionalFieldInRecordBuilder(arena.alloc(loc_name), loc_val),
});
}
AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => {
unreachable!("Should have been desugared in `desugar_field`")
}
AssignedField::Malformed(_name) => continue,
};

field_data.push(FieldData {
name,
value,
ignored,
});
}

let closure_arg_from_field = |field: Loc<&'a str>| Loc {
region: field.region,
value: Pattern::Identifier {
ident: arena.alloc_str(&format!("#{}", field.value)),
},
};
let closure_arg_from_field =
|FieldData {
name,
value: _,
ignored,
}: &FieldData<'a>| Loc {
region: name.region,
value: if *ignored {
Pattern::Underscore(name.value)
} else {
Pattern::Identifier {
ident: arena.alloc_str(&format!("#{}", name.value)),
}
},
};

let combiner_closure_in_region = |region| {
let closure_body = Tuple(Collection::with_items(
Expand Down Expand Up @@ -607,57 +631,61 @@ pub fn desugar_expr<'a>(
};

let closure_args = {
if field_names.len() == 2 {
if field_data.len() == 2 {
arena.alloc_slice_copy(&[
closure_arg_from_field(field_names[0]),
closure_arg_from_field(field_names[1]),
closure_arg_from_field(&field_data[0]),
closure_arg_from_field(&field_data[1]),
])
} else {
let second_to_last_arg =
closure_arg_from_field(field_names[field_names.len() - 2]);
let last_arg = closure_arg_from_field(field_names[field_names.len() - 1]);
closure_arg_from_field(&field_data[field_data.len() - 2]);
let last_arg = closure_arg_from_field(&field_data[field_data.len() - 1]);

let mut second_arg = Pattern::Tuple(Collection::with_items(
arena.alloc_slice_copy(&[second_to_last_arg, last_arg]),
));
let mut second_arg_region =
Region::span_across(&second_to_last_arg.region, &last_arg.region);

for index in (1..(field_names.len() - 2)).rev() {
for index in (1..(field_data.len() - 2)).rev() {
second_arg =
Pattern::Tuple(Collection::with_items(arena.alloc_slice_copy(&[
closure_arg_from_field(field_names[index]),
closure_arg_from_field(&field_data[index]),
Loc::at(second_arg_region, second_arg),
])));
second_arg_region =
Region::span_across(&field_names[index].region, &second_arg_region);
Region::span_across(&field_data[index].name.region, &second_arg_region);
}

arena.alloc_slice_copy(&[
closure_arg_from_field(field_names[0]),
closure_arg_from_field(&field_data[0]),
Loc::at(second_arg_region, second_arg),
])
}
};

let record_val = Record(Collection::with_items(
Vec::from_iter_in(
field_names.iter().map(|field_name| {
Loc::at(
field_name.region,
AssignedField::RequiredValue(
Loc::at(field_name.region, field_name.value),
&[],
arena.alloc(Loc::at(
field_name.region,
Expr::Var {
module_name: "",
ident: arena.alloc_str(&format!("#{}", field_name.value)),
},
)),
),
)
}),
field_data
.iter()
.filter(|field| !field.ignored)
.map(|field| {
Loc::at(
field.name.region,
AssignedField::RequiredValue(
field.name,
&[],
arena.alloc(Loc::at(
field.name.region,
Expr::Var {
module_name: "",
ident: arena
.alloc_str(&format!("#{}", field.name.value)),
},
)),
),
)
}),
arena,
)
.into_bump_slice(),
Expand All @@ -671,14 +699,14 @@ pub fn desugar_expr<'a>(
),
});

if field_names.len() == 2 {
if field_data.len() == 2 {
return arena.alloc(Loc {
region: loc_expr.region,
value: Apply(
new_mapper,
arena.alloc_slice_copy(&[
field_vals[0],
field_vals[1],
field_data[0].value,
field_data[1].value,
record_combiner_closure,
]),
CalledVia::RecordBuilder,
Expand All @@ -688,27 +716,30 @@ pub fn desugar_expr<'a>(

let mut inner_combined = arena.alloc(Loc {
region: Region::span_across(
&field_vals[field_names.len() - 2].region,
&field_vals[field_names.len() - 1].region,
&field_data[field_data.len() - 2].value.region,
&field_data[field_data.len() - 1].value.region,
),
value: Apply(
new_mapper,
arena.alloc_slice_copy(&[
field_vals[field_names.len() - 2],
field_vals[field_names.len() - 1],
field_data[field_data.len() - 2].value,
field_data[field_data.len() - 1].value,
combiner_closure_in_region(loc_expr.region),
]),
CalledVia::RecordBuilder,
),
});

for index in (1..(field_names.len() - 2)).rev() {
for index in (1..(field_data.len() - 2)).rev() {
inner_combined = arena.alloc(Loc {
region: Region::span_across(&field_vals[index].region, &inner_combined.region),
region: Region::span_across(
&field_data[index].value.region,
&inner_combined.region,
),
value: Apply(
new_mapper,
arena.alloc_slice_copy(&[
field_vals[index],
field_data[index].value,
inner_combined,
combiner_closure_in_region(loc_expr.region),
]),
Expand All @@ -722,7 +753,7 @@ pub fn desugar_expr<'a>(
value: Apply(
new_mapper,
arena.alloc_slice_copy(&[
field_vals[0],
field_data[0].value,
inner_combined,
record_combiner_closure,
]),
Expand Down Expand Up @@ -1095,6 +1126,14 @@ fn desugar_field<'a>(
spaces,
desugar_expr(arena, loc_expr, src, line_info, module_path),
),
IgnoredValue(loc_str, spaces, loc_expr) => IgnoredValue(
Loc {
value: loc_str.value,
region: loc_str.region,
},
spaces,
desugar_expr(arena, loc_expr, src, line_info, module_path),
),
LabelOnly(loc_str) => {
// Desugar { x } into { x: x }
let loc_expr = Loc {
Expand Down
14 changes: 11 additions & 3 deletions crates/compiler/can/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1846,6 +1846,11 @@ fn canonicalize_field<'a>(
field_region: Region::span_across(&label.region, &loc_expr.region),
}),

// An ignored value, e.g. `{ _name: 123 }`
IgnoredValue(_, _, _) => {
internal_error!("Somehow an IgnoredValue record field was not desugared!");
}

// A label with no value, e.g. `{ name }` (this is sugar for { name: name })
LabelOnly(_) => {
internal_error!("Somehow a LabelOnly record field was not desugared!");
Expand Down Expand Up @@ -2433,7 +2438,8 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
}
ast::Expr::Record(fields) => fields.iter().all(|loc_field| match loc_field.value {
ast::AssignedField::RequiredValue(_label, loc_comments, loc_val)
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val) => {
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val)
| ast::AssignedField::IgnoredValue(_label, loc_comments, loc_val) => {
loc_comments.is_empty() && is_valid_interpolation(&loc_val.value)
}
ast::AssignedField::Malformed(_) | ast::AssignedField::LabelOnly(_) => true,
Expand Down Expand Up @@ -2481,7 +2487,8 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
is_valid_interpolation(&update.value)
&& fields.iter().all(|loc_field| match loc_field.value {
ast::AssignedField::RequiredValue(_label, loc_comments, loc_val)
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val) => {
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val)
| ast::AssignedField::IgnoredValue(_label, loc_comments, loc_val) => {
loc_comments.is_empty() && is_valid_interpolation(&loc_val.value)
}
ast::AssignedField::Malformed(_) | ast::AssignedField::LabelOnly(_) => true,
Expand Down Expand Up @@ -2514,7 +2521,8 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
is_valid_interpolation(&mapper.value)
&& fields.iter().all(|loc_field| match loc_field.value {
ast::AssignedField::RequiredValue(_label, loc_comments, loc_val)
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val) => {
| ast::AssignedField::OptionalValue(_label, loc_comments, loc_val)
| ast::AssignedField::IgnoredValue(_label, loc_comments, loc_val) => {
loc_comments.is_empty() && is_valid_interpolation(&loc_val.value)
}
ast::AssignedField::Malformed(_) | ast::AssignedField::LabelOnly(_) => true,
Expand Down
24 changes: 21 additions & 3 deletions crates/compiler/fmt/src/annotation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,9 @@ fn is_multiline_assigned_field_help<T: Formattable>(afield: &AssignedField<'_, T
use self::AssignedField::*;

match afield {
RequiredValue(_, spaces, ann) | OptionalValue(_, spaces, ann) => {
!spaces.is_empty() || ann.value.is_multiline()
}
RequiredValue(_, spaces, ann)
| OptionalValue(_, spaces, ann)
| IgnoredValue(_, spaces, ann) => !spaces.is_empty() || ann.value.is_multiline(),
LabelOnly(_) => false,
AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => true,
Malformed(text) => text.chars().any(|c| c == '\n'),
Expand Down Expand Up @@ -483,6 +483,24 @@ fn format_assigned_field_help<T>(
buf.spaces(1);
ann.value.format(buf, indent);
}
IgnoredValue(name, spaces, ann) => {
if is_multiline {
buf.newline();
}

buf.indent(indent);
buf.push('_');
buf.push_str(name.value);

if !spaces.is_empty() {
fmt_spaces(buf, spaces.iter(), indent);
}

buf.spaces(separator_spaces);
buf.push(':');
buf.spaces(1);
ann.value.format(buf, indent);
}
LabelOnly(name) => {
if is_multiline {
buf.newline();
Expand Down
Loading

0 comments on commit 698bbc3

Please sign in to comment.