Skip to content

Commit bb6c58f

Browse files
GearsDatapackslpil
authored andcommitted
Fix pattern match scoping of bit arrays
1 parent 4dd16c3 commit bb6c58f

File tree

3 files changed

+129
-26
lines changed

3 files changed

+129
-26
lines changed

compiler-core/src/type_/environment.rs

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,16 @@ impl Environment<'_> {
162162
// Process scope
163163
let result = process_scope(self, problems);
164164

165-
self.close_scope(initial, result.is_ok(), problems);
165+
// We only check for unused entities if the scope was successfully
166+
// processed. If it was not then any seemingly unused entities may have
167+
// been used beyond the point where the error occurred, so we don't want
168+
// to incorrectly warn about them.
169+
let usage_tracking = if result.is_ok() {
170+
UsageTracking::TrackUnused
171+
} else {
172+
UsageTracking::IgnoreUnused
173+
};
174+
self.close_scope(initial, usage_tracking, problems);
166175

167176
// Return result of typing the scope
168177
result
@@ -177,19 +186,15 @@ impl Environment<'_> {
177186
pub fn close_scope(
178187
&mut self,
179188
data: ScopeResetData,
180-
was_successful: bool,
189+
usage_tracking: UsageTracking,
181190
problems: &mut Problems,
182191
) {
183192
let unused = self
184193
.local_variable_usages
185194
.pop()
186195
.expect("There was no top entity scope.");
187196

188-
// We only check for unused entities if the scope was successfully
189-
// processed. If it was not then any seemingly unused entities may have
190-
// been used beyond the point where the error occurred, so we don't want
191-
// to incorrectly warn about them.
192-
if was_successful {
197+
if let UsageTracking::TrackUnused = usage_tracking {
193198
self.handle_unused_variables(unused, problems);
194199
}
195200
self.scope = data.local_values;
@@ -794,6 +799,12 @@ pub enum Imported {
794799
Value(EcoString),
795800
}
796801

802+
#[derive(Debug, Clone, Copy)]
803+
pub enum UsageTracking {
804+
TrackUnused,
805+
IgnoreUnused,
806+
}
807+
797808
/// Unify two types that should be the same.
798809
/// Any unbound type variables will be linked to the other type as they are the same.
799810
///

compiler-core/src/type_/expression.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,14 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
378378
// Process the scope
379379
let (result, was_successful) = process_scope(self);
380380

381+
let usage_tracking = if was_successful {
382+
UsageTracking::TrackUnused
383+
} else {
384+
UsageTracking::IgnoreUnused
385+
};
381386
// Close scope, discarding any scope local state
382387
self.environment
383-
.close_scope(environment_reset_data, was_successful, self.problems);
388+
.close_scope(environment_reset_data, usage_tracking, self.problems);
384389
self.hydrator.close_scope(hydrator_reset_data);
385390
result
386391
}
@@ -1762,11 +1767,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
17621767
self.problems,
17631768
);
17641769

1765-
let value_variable_name = match value {
1766-
TypedExpr::Var { ref name, .. } => Some(name.clone()),
1767-
_ => None,
1768-
};
1769-
let pattern = pattern_typer.unify(pattern, type_.clone(), value_variable_name);
1770+
let pattern = pattern_typer.infer_single_pattern(pattern, &value);
17701771

17711772
let minimum_required_version = pattern_typer.minimum_required_version;
17721773
if minimum_required_version > self.minimum_required_version {

compiler-core/src/type_/pattern.rs

Lines changed: 104 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,23 @@ pub struct PatternTyper<'a, 'b> {
3333
pub minimum_required_version: Version,
3434

3535
pub error_encountered: bool,
36+
37+
variables: Vec<Variable>,
38+
pattern_position: PatternPosition,
39+
}
40+
41+
#[derive(Debug)]
42+
struct Variable {
43+
name: EcoString,
44+
location: SrcSpan,
45+
origin: VariableOrigin,
46+
type_: Arc<Type>,
47+
}
48+
49+
#[derive(Debug, Clone, Copy)]
50+
enum PatternPosition {
51+
BitArray,
52+
NotBitArray,
3653
}
3754

3855
enum PatternMode {
@@ -59,27 +76,27 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
5976
minimum_required_version: Version::new(0, 1, 0),
6077
problems,
6178
error_encountered: false,
79+
variables: Vec::new(),
80+
pattern_position: PatternPosition::NotBitArray,
6281
}
6382
}
6483

6584
fn insert_variable(
6685
&mut self,
67-
name: &str,
86+
name: &EcoString,
6887
type_: Arc<Type>,
6988
location: SrcSpan,
7089
origin: VariableOrigin,
7190
) {
72-
self.check_name_case(location, &EcoString::from(name), Named::Variable);
91+
self.check_name_case(location, &name, Named::Variable);
7392

7493
match &mut self.mode {
7594
PatternMode::Initial => {
7695
// Register usage for the unused variable detection
77-
self.environment
78-
.init_usage(name.into(), origin.clone(), location, self.problems);
7996
// Ensure there are no duplicate variable names in the pattern
8097
if self.initial_pattern_vars.contains(name) {
8198
self.error(convert_unify_error(
82-
UnifyError::DuplicateVarInPattern { name: name.into() },
99+
UnifyError::DuplicateVarInPattern { name: name.clone() },
83100
location,
84101
));
85102
return;
@@ -90,18 +107,41 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
90107
// Record that this variable originated in this pattern so any
91108
// following alternative patterns can be checked to ensure they
92109
// have the same variables.
93-
let _ = self.initial_pattern_vars.insert(name.into());
110+
let _ = self.initial_pattern_vars.insert(name.clone());
111+
94112
// And now insert the variable for use in the code that comes
95113
// after the pattern.
96-
self.environment
97-
.insert_local_variable(name.into(), location, origin, type_);
114+
115+
self.variables.push(Variable {
116+
name: name.clone(),
117+
location,
118+
origin: origin.clone(),
119+
type_: type_.clone(),
120+
});
121+
match self.pattern_position {
122+
PatternPosition::BitArray => {
123+
self.environment.init_usage(
124+
name.clone(),
125+
origin.clone(),
126+
location,
127+
self.problems,
128+
);
129+
self.environment.insert_local_variable(
130+
name.clone(),
131+
location,
132+
origin,
133+
type_,
134+
);
135+
}
136+
PatternPosition::NotBitArray => {}
137+
};
98138
}
99139

100140
PatternMode::Alternative(assigned) => {
101141
match self.environment.scope.get_mut(name) {
102142
// This variable was defined in the Initial multi-pattern
103143
Some(initial) if self.initial_pattern_vars.contains(name) => {
104-
assigned.push(name.into());
144+
assigned.push(name.clone());
105145
let initial_type = initial.type_.clone();
106146
match unify(initial_type, type_.clone()) {
107147
Ok(()) => {}
@@ -115,7 +155,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
115155

116156
// This variable was not defined in the Initial multi-pattern
117157
_ => self.error(convert_unify_error(
118-
UnifyError::ExtraVarInAlternativePattern { name: name.into() },
158+
UnifyError::ExtraVarInAlternativePattern { name: name.clone() },
119159
location,
120160
)),
121161
}
@@ -254,14 +294,61 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
254294
let pattern = self.unify(pattern, subject.type_(), subject_variable);
255295
typed_multi.push(pattern);
256296
}
297+
298+
self.register_variables();
299+
257300
typed_multi
258301
}
259302

303+
pub fn infer_single_pattern(
304+
&mut self,
305+
pattern: UntypedPattern,
306+
subject: &TypedExpr,
307+
) -> TypedPattern {
308+
let subject_variable = match subject {
309+
TypedExpr::Var {
310+
constructor:
311+
ValueConstructor {
312+
// Records should not be considered local variables
313+
// See: https://github.com/gleam-lang/gleam/issues/3861
314+
variant: ValueConstructorVariant::Record { .. },
315+
..
316+
},
317+
..
318+
} => None,
319+
TypedExpr::Var { name, .. } => Some(name.clone()),
320+
_ => None,
321+
};
322+
323+
let typed_pattern = self.unify(pattern, subject.type_(), subject_variable);
324+
self.register_variables();
325+
typed_pattern
326+
}
327+
328+
fn register_variables(&mut self) {
329+
for variable in std::mem::take(&mut self.variables) {
330+
let Variable {
331+
name,
332+
location,
333+
origin,
334+
type_,
335+
} = variable;
336+
337+
self.environment
338+
.init_usage(name.clone(), origin.clone(), location, self.problems);
339+
self.environment
340+
.insert_local_variable(name, location, origin, type_);
341+
}
342+
}
343+
260344
fn infer_pattern_bit_array(
261345
&mut self,
262346
mut segments: Vec<UntypedPatternBitArraySegment>,
263347
location: SrcSpan,
264348
) -> TypedPattern {
349+
let scope_reset_data = self.environment.open_new_scope();
350+
self.pattern_position = PatternPosition::BitArray;
351+
265352
let last_segment = segments.pop();
266353

267354
let mut typed_segments: Vec<_> = segments
@@ -274,6 +361,10 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
274361
typed_segments.push(typed_last_segment)
275362
}
276363

364+
self.pattern_position = PatternPosition::NotBitArray;
365+
self.environment
366+
.close_scope(scope_reset_data, UsageTracking::IgnoreUnused, self.problems);
367+
277368
TypedPattern::BitArray {
278369
location,
279370
segments: typed_segments,
@@ -418,7 +509,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
418509
/// inferred type of the subject in order to determine what variables to insert
419510
/// into the environment (or to detect a type error).
420511
///
421-
pub fn unify(
512+
fn unify(
422513
&mut self,
423514
pattern: UntypedPattern,
424515
type_: Arc<Type>,
@@ -515,7 +606,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
515606
// The left hand side may assign a variable, which is the prefix of the string
516607
if let Some((left, left_location)) = &left_side_assignment {
517608
self.insert_variable(
518-
left.as_ref(),
609+
left,
519610
string(),
520611
*left_location,
521612
VariableOrigin::AssignmentPattern,
@@ -526,7 +617,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
526617
match &right_side_assignment {
527618
AssignName::Variable(right) => {
528619
self.insert_variable(
529-
right.as_ref(),
620+
right,
530621
string(),
531622
right_location,
532623
VariableOrigin::Variable(right.clone()),

0 commit comments

Comments
 (0)