Skip to content

Commit c3dfdda

Browse files
sobolevnlpil
sobolevn
authored and
lpil
committed
Set proper type for named field
1 parent d2cfd24 commit c3dfdda

File tree

4 files changed

+160
-94
lines changed

4 files changed

+160
-94
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@
166166

167167
([Aaron Christiansen](https://github.com/AaronC81))
168168

169+
- Fault tolerance for analysis of labeled fields in constructor patterns has
170+
been improved.
171+
([sobolevn](https://github.com/sobolevn))
172+
169173
### Build tool
170174

171175
- New projects are generated using OTP28 on GitHub Actions.

‎compiler-core/src/type_/pattern.rs‎

Lines changed: 94 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use super::*;
1111
use crate::{
1212
analyse::{self, Inferred, name::check_name_case},
1313
ast::{
14-
AssignName, BitArrayOption, BitArraySize, ImplicitCallArgOrigin, Layer, Pattern,
15-
TypedBitArraySize, UntypedPatternBitArraySegment,
14+
AssignName, BitArrayOption, BitArraySize, ImplicitCallArgOrigin, Layer, TypedBitArraySize,
15+
UntypedPatternBitArraySegment,
1616
},
1717
parse::PatternPosition,
1818
reference::ReferenceKind,
@@ -886,7 +886,11 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
886886
location,
887887
name_location,
888888
name,
889-
arguments: self.infer_pattern_call_arguments(pattern_arguments, &[]),
889+
arguments: self.infer_pattern_call_arguments(
890+
pattern_arguments,
891+
&[],
892+
None,
893+
),
890894
module,
891895
constructor: Inferred::Unknown,
892896
spread,
@@ -911,11 +915,78 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
911915
self.error_encountered = true;
912916
};
913917
}
914-
insert_spread_call_arguments(
915-
&mut pattern_arguments,
916-
field_map,
917-
spread_location,
918-
);
918+
919+
// Insert discard variables to match the unspecified fields
920+
// In order to support both positional and labelled arguments we have to insert
921+
// them after all positional variables and before the labelled ones. This means
922+
// we have calculate that index and then insert() the discards. It would be faster
923+
// if we could put the discards anywhere which would let us use push().
924+
// Potential future optimisation.
925+
let index_of_first_labelled_arg = pattern_arguments
926+
.iter()
927+
.position(|argument| argument.label.is_some())
928+
.unwrap_or(pattern_arguments.len());
929+
930+
// In Gleam we can pass in positional unlabelled args to a constructor
931+
// even if the field was defined as labelled
932+
//
933+
// pub type Wibble {
934+
// Wibble(Int, two: Int, three: Int, four: Int)
935+
// }
936+
// Wibble(1, 2, 3, 4)
937+
//
938+
// When using `..` to ignore some fields the compiler needs to add a
939+
// placeholder implicit discard pattern for each one of the ignored
940+
// arguments. To give those discards the proper missing label we need to
941+
// know how many of the labelled fields were provided as unlabelled.
942+
//
943+
// That's why we want to keep track of the number of unlabelled argument
944+
// that have been supplied to the pattern and all the labels that have
945+
// been explicitly supplied.
946+
//
947+
// Wibble(a, b, four: c, ..)
948+
// ┬─── ┬──────
949+
// │ ╰ We supplied 1 labelled arg
950+
// ╰ We supplied 2 unlabelled args
951+
//
952+
let supplied_unlabelled_arguments = index_of_first_labelled_arg;
953+
let supplied_labelled_arguments = pattern_arguments
954+
.iter()
955+
.filter_map(|argument| argument.label.clone())
956+
.collect::<HashSet<_>>();
957+
let constructor_unlabelled_arguments =
958+
field_map.arity - field_map.fields.len() as u32;
959+
let labelled_arguments_supplied_as_unlabelled =
960+
supplied_unlabelled_arguments
961+
.saturating_sub(constructor_unlabelled_arguments as usize);
962+
963+
let mut missing_labels = field_map
964+
.fields
965+
.iter()
966+
// We take the labels in order of definition in the constructor...
967+
.sorted_by_key(|(_, position)| *position)
968+
.map(|(label, _)| label.clone())
969+
// ...and then remove the ones that were supplied as unlabelled
970+
// positional arguments...
971+
.skip(labelled_arguments_supplied_as_unlabelled)
972+
// ... lastly we still need to remove all those labels that
973+
// were explicitly supplied in the pattern.
974+
.filter(|label| !supplied_labelled_arguments.contains(label));
975+
976+
while pattern_arguments.len() < field_map.arity as usize {
977+
let new_call_arg = CallArg {
978+
value: Pattern::Discard {
979+
name: "_".into(),
980+
location: spread_location,
981+
type_: (),
982+
},
983+
location: spread_location,
984+
label: missing_labels.next(),
985+
implicit: Some(ImplicitCallArgOrigin::PatternFieldSpread),
986+
};
987+
988+
pattern_arguments.insert(index_of_first_labelled_arg, new_call_arg);
989+
}
919990
}
920991

921992
if let Err(error) = field_map.reorder(
@@ -926,13 +997,6 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
926997
incorrect_arity_error = true;
927998
self.problems.error(error);
928999
self.error_encountered = true;
929-
// Since we reported the arity error now, we can pretend
930-
// that `..` is now added, so other errors will not be affected.
931-
insert_spread_call_arguments(
932-
&mut pattern_arguments,
933-
field_map,
934-
name_location,
935-
);
9361000
}
9371001
}
9381002

@@ -968,6 +1032,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
9681032

9691033
let constructor_type = constructor.type_.clone();
9701034
let constructor_deprecation = constructor.deprecation.clone();
1035+
let constructor_field_map = constructor.field_map().cloned();
9711036
let pattern_constructor = match &constructor.variant {
9721037
ValueConstructorVariant::Record {
9731038
name,
@@ -1053,8 +1118,12 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
10531118
});
10541119
}
10551120

1056-
let pattern_arguments =
1057-
self.infer_pattern_call_arguments(pattern_arguments, arguments);
1121+
let pattern_arguments = self.infer_pattern_call_arguments(
1122+
pattern_arguments,
1123+
arguments,
1124+
constructor_field_map,
1125+
);
1126+
10581127
Pattern::Constructor {
10591128
location,
10601129
name_location,
@@ -1109,13 +1178,21 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
11091178
&mut self,
11101179
pattern_arguments: Vec<CallArg<UntypedPattern>>,
11111180
expected_types: &[Arc<Type>],
1181+
field_map: Option<FieldMap>,
11121182
) -> Vec<CallArg<TypedPattern>> {
11131183
pattern_arguments
11141184
.into_iter()
11151185
.enumerate()
11161186
.map(|(index, arg)| {
1187+
let mut index = index;
11171188
if !arg.is_implicit() && arg.uses_label_shorthand() {
11181189
self.track_feature_usage(FeatureKind::LabelShorthandSyntax, arg.location);
1190+
1191+
if let Some(field_map) = &field_map
1192+
&& let Some(label) = &arg.label
1193+
{
1194+
index = *field_map.fields.get(label).expect("Field already exists") as usize
1195+
}
11191196
}
11201197

11211198
let CallArg {
@@ -1320,80 +1397,3 @@ fn unify_constructor_variants(into: &mut Type, from: &Type) {
13201397
_ => {}
13211398
}
13221399
}
1323-
1324-
/// Inserts needed discard variables to Constructor::Pattern, when `spread` is used.
1325-
fn insert_spread_call_arguments(
1326-
pattern_arguments: &mut Vec<CallArg<Pattern<()>>>,
1327-
field_map: &FieldMap,
1328-
location: SrcSpan,
1329-
) {
1330-
// Insert discard variables to match the unspecified fields
1331-
// In order to support both positional and labelled arguments we have to insert
1332-
// them after all positional variables and before the labelled ones. This means
1333-
// we have calculate that index and then insert() the discards. It would be faster
1334-
// if we could put the discards anywhere which would let us use push().
1335-
// Potential future optimisation.
1336-
let index_of_first_labelled_arg = pattern_arguments
1337-
.iter()
1338-
.position(|argument| argument.label.is_some())
1339-
.unwrap_or(pattern_arguments.len());
1340-
1341-
// In Gleam we can pass in positional unlabelled args to a constructor
1342-
// even if the field was defined as labelled
1343-
//
1344-
// pub type Wibble {
1345-
// Wibble(Int, two: Int, three: Int, four: Int)
1346-
// }
1347-
// Wibble(1, 2, 3, 4)
1348-
//
1349-
// When using `..` to ignore some fields the compiler needs to add a
1350-
// placeholder implicit discard pattern for each one of the ignored
1351-
// arguments. To give those discards the proper missing label we need to
1352-
// know how many of the labelled fields were provided as unlabelled.
1353-
//
1354-
// That's why we want to keep track of the number of unlabelled argument
1355-
// that have been supplied to the pattern and all the labels that have
1356-
// been explicitly supplied.
1357-
//
1358-
// Wibble(a, b, four: c, ..)
1359-
// ┬─── ┬──────
1360-
// │ ╰ We supplied 1 labelled arg
1361-
// ╰ We supplied 2 unlabelled args
1362-
//
1363-
let supplied_unlabelled_arguments = index_of_first_labelled_arg;
1364-
let supplied_labelled_arguments = pattern_arguments
1365-
.iter()
1366-
.filter_map(|argument| argument.label.clone())
1367-
.collect::<HashSet<_>>();
1368-
let constructor_unlabelled_arguments = field_map.arity - field_map.fields.len() as u32;
1369-
let labelled_arguments_supplied_as_unlabelled =
1370-
supplied_unlabelled_arguments.saturating_sub(constructor_unlabelled_arguments as usize);
1371-
1372-
let mut missing_labels = field_map
1373-
.fields
1374-
.iter()
1375-
// We take the labels in order of definition in the constructor...
1376-
.sorted_by_key(|(_, position)| *position)
1377-
.map(|(label, _)| label.clone())
1378-
// ...and then remove the ones that were supplied as unlabelled
1379-
// positional arguments...
1380-
.skip(labelled_arguments_supplied_as_unlabelled)
1381-
// ... lastly we still need to remove all those labels that
1382-
// were explicitly supplied in the pattern.
1383-
.filter(|label| !supplied_labelled_arguments.contains(label));
1384-
1385-
while pattern_arguments.len() < field_map.arity as usize {
1386-
let new_call_arg = CallArg {
1387-
value: Pattern::Discard {
1388-
name: "_".into(),
1389-
location,
1390-
type_: (),
1391-
},
1392-
location,
1393-
label: missing_labels.next(),
1394-
implicit: Some(ImplicitCallArgOrigin::PatternFieldSpread),
1395-
};
1396-
1397-
pattern_arguments.insert(index_of_first_labelled_arg, new_call_arg);
1398-
}
1399-
}

‎compiler-core/src/type_/tests/custom_types.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,22 @@ fn handle_fish(fish: Fish) {
201201
"#
202202
);
203203
}
204+
205+
#[test]
206+
fn pattern_match_correct_pos_field() {
207+
assert_module_error!(
208+
r#"
209+
type Fish {
210+
Starfish()
211+
Jellyfish(String, Bool)
212+
}
213+
214+
fn handle_fish(fish: Fish) {
215+
case fish {
216+
Starfish() -> False
217+
Jellyfish(jiggly) -> jiggly
218+
}
219+
}
220+
"#
221+
);
222+
}

‎compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__custom_types__pattern_match_correct_pos_field.snap‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
source: compiler-core/src/type_/tests/custom_types.rs
3+
expression: "\ntype Fish {\n Starfish()\n Jellyfish(String, Bool)\n}\n\nfn handle_fish(fish: Fish) {\n case fish {\n Starfish() -> False\n Jellyfish(jiggly) -> jiggly\n }\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Fish {
8+
Starfish()
9+
Jellyfish(String, Bool)
10+
}
11+
12+
fn handle_fish(fish: Fish) {
13+
case fish {
14+
Starfish() -> False
15+
Jellyfish(jiggly) -> jiggly
16+
}
17+
}
18+
19+
20+
----- ERROR
21+
error: Incorrect arity
22+
┌─ /src/one/two.gleam:10:5
23+
│
24+
10 │ Jellyfish(jiggly) -> jiggly
25+
│ ^^^^^^^^^^^^^^^^^ Expected 2 arguments, got 1
26+
27+
28+
error: Type mismatch
29+
┌─ /src/one/two.gleam:10:5
30+
│
31+
10 │ Jellyfish(jiggly) -> jiggly
32+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
33+
34+
This case clause was found to return a different type than the previous
35+
one, but all case clauses must return the same type.
36+
37+
Expected type:
38+
39+
Bool
40+
41+
Found type:
42+
43+
String

0 commit comments

Comments
 (0)