Skip to content

Commit 8353933

Browse files
authored
flow-control: Add an example for an uncovered pattern. (#8370)
1 parent 6c60160 commit 8353933

File tree

4 files changed

+115
-41
lines changed

4 files changed

+115
-41
lines changed

crates/cairo-lang-lowering/src/lower/flow_control/create_graph.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,15 @@ pub fn create_graph_expr_if<'db>(
7575
.iter()
7676
.map(|pattern| Some(get_pattern(ctx, *pattern)))
7777
.collect_vec(),
78-
build_node_callback: &mut |graph, pattern_indices| {
78+
build_node_callback: &mut |graph, pattern_indices, path| {
7979
if let Some(index_and_bindings) = pattern_indices.first() {
8080
cache.get_or_compute(
81-
&mut |graph, index_and_bindings: IndexAndBindings| {
81+
&mut |graph, index_and_bindings: IndexAndBindings, _path| {
8282
index_and_bindings.wrap_node(graph, current_node)
8383
},
8484
graph,
8585
index_and_bindings,
86+
path,
8687
)
8788
} else {
8889
false_branch
@@ -141,24 +142,25 @@ pub fn create_graph_expr_match<'db>(
141142
.iter()
142143
.map(|(pattern, _)| Some(get_pattern(ctx, *pattern)))
143144
.collect_vec(),
144-
build_node_callback: &mut |graph, pattern_indices| {
145+
build_node_callback: &mut |graph, pattern_indices, path| {
145146
// Get the first arm that matches.
146147
let Some(index_and_bindings) = pattern_indices.first() else {
147148
// If no arm is available, report a non-exhaustive match error.
148149
let kind = LoweringDiagnosticKind::MatchError(MatchError {
149150
kind: MatchKind::Match,
150-
error: MatchDiagnostic::NonExhaustiveMatchValue,
151+
error: MatchDiagnostic::MissingMatchArm(path),
151152
});
152153
return graph.report_with_missing_node(expr.stable_ptr.untyped(), kind);
153154
};
154155

155156
cache.get_or_compute(
156-
&mut |graph, index_and_bindings: IndexAndBindings| {
157+
&mut |graph, index_and_bindings: IndexAndBindings, _path| {
157158
let index = index_and_bindings.index();
158159
index_and_bindings.wrap_node(graph, pattern_and_nodes[index].1)
159160
},
160161
graph,
161162
index_and_bindings,
163+
path,
162164
)
163165
},
164166
location: matched_expr_location,

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/cache.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,16 @@ impl<Input: std::hash::Hash + Eq + Clone> Cache<Input> {
1515
/// Returns the previous result, otherwise.
1616
pub fn get_or_compute<'db>(
1717
&mut self,
18-
callback: &mut dyn FnMut(&mut FlowControlGraphBuilder<'db>, Input) -> NodeId,
18+
callback: &mut dyn FnMut(&mut FlowControlGraphBuilder<'db>, Input, String) -> NodeId,
1919
graph: &mut FlowControlGraphBuilder<'db>,
2020
input: Input,
21+
path: String,
2122
) -> NodeId {
2223
if let Some(node_id) = self.cache.get(&input) {
2324
return *node_id;
2425
}
2526

26-
let node_id = callback(graph, input.clone());
27+
let node_id = callback(graph, input.clone(), path);
2728
assert!(!self.cache.contains_key(&input));
2829
self.cache.insert(input, node_id);
2930
node_id

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs

Lines changed: 82 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use cairo_lang_defs::ids::NamedLanguageElementId;
12
use cairo_lang_diagnostics::Maybe;
23
use cairo_lang_semantic::corelib::validate_literal;
34
use cairo_lang_semantic::db::SemanticGroup;
@@ -13,6 +14,7 @@ use cairo_lang_utils::iterators::zip_eq3;
1314
use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
1415
use itertools::Itertools;
1516
use num_bigint::BigInt;
17+
use salsa::Database;
1618

1719
use super::super::graph::{
1820
Deconstruct, EnumMatch, FlowControlGraphBuilder, FlowControlNode, FlowControlVar, NodeId,
@@ -48,8 +50,10 @@ use crate::lower::lower_match::numeric_match_optimization_threshold;
4850
/// returns `0` if `y=C` and `1` otherwise.
4951
/// Finally, the inner pattern-matching function (for `x`) will construct a [EnumMatch] node
5052
/// that leads to the two nodes returned by the callback.
53+
///
54+
/// The `path: String` parameter is an example for a pattern leading to this callback.
5155
type BuildNodeCallback<'db, 'a> =
52-
&'a mut dyn FnMut(&mut FlowControlGraphBuilder<'db>, FilteredPatterns) -> NodeId;
56+
&'a mut dyn FnMut(&mut FlowControlGraphBuilder<'db>, FilteredPatterns, String) -> NodeId;
5357

5458
/// A thin wrapper around [semantic::Pattern], where `None` represents the `_` pattern.
5559
type PatternOption<'a, 'db> = Option<&'a semantic::Pattern<'db>>;
@@ -95,14 +99,16 @@ pub fn create_node_for_patterns<'db>(
9599
let mut cache = Cache::default();
96100

97101
// Wrap `build_node_callback` to add the bindings to the patterns and cache the result.
98-
let mut build_node_callback =
99-
|graph: &mut FlowControlGraphBuilder<'db>, pattern_indices: FilteredPatterns| {
100-
cache.get_or_compute(
101-
build_node_callback,
102-
graph,
103-
pattern_indices.add_bindings(bindings.clone()),
104-
)
105-
};
102+
let mut build_node_callback = |graph: &mut FlowControlGraphBuilder<'db>,
103+
pattern_indices: FilteredPatterns,
104+
path: String| {
105+
cache.get_or_compute(
106+
build_node_callback,
107+
graph,
108+
pattern_indices.add_bindings(bindings.clone()),
109+
path,
110+
)
111+
};
106112

107113
let var_ty = graph.var_ty(input_var);
108114
let (n_snapshots, long_ty) = peel_snapshots(ctx.db, var_ty);
@@ -130,7 +136,7 @@ pub fn create_node_for_patterns<'db>(
130136
else {
131137
// If all the patterns are catch-all, we do not need to look into `input_var`.
132138
// Call the callback with all patterns accepted.
133-
return build_node_callback(graph, FilteredPatterns::all(patterns.len()));
139+
return build_node_callback(graph, FilteredPatterns::all(patterns.len()), "_".into());
134140
};
135141

136142
// Check if this is a numeric type that should use value matching.
@@ -225,8 +231,12 @@ fn create_node_for_enum<'db>(
225231
ctx,
226232
graph,
227233
patterns: &inner_patterns,
228-
build_node_callback: &mut |graph, pattern_indices_inner| {
229-
build_node_callback(graph, pattern_indices_inner.lift(&pattern_indices))
234+
build_node_callback: &mut |graph, pattern_indices_inner, path| {
235+
build_node_callback(
236+
graph,
237+
pattern_indices_inner.lift(&pattern_indices),
238+
format!("{}({path})", concrete_variant.id.name(ctx.db)),
239+
)
230240
},
231241
location,
232242
},
@@ -269,7 +279,15 @@ fn create_node_for_tuple<'db>(
269279
.collect_vec();
270280

271281
let node = create_node_for_tuple_inner(
272-
CreateNodeParams { ctx, graph, patterns, build_node_callback, location },
282+
CreateNodeParams {
283+
ctx,
284+
graph,
285+
patterns,
286+
build_node_callback: &mut |graph, pattern_indices, path| {
287+
build_node_callback(graph, pattern_indices, format!("({path})"))
288+
},
289+
location,
290+
},
273291
&inner_vars,
274292
types,
275293
0,
@@ -305,7 +323,16 @@ fn create_node_for_struct<'db>(
305323
.collect_vec();
306324

307325
let node = create_node_for_tuple_inner(
308-
CreateNodeParams { ctx, graph, patterns, build_node_callback, location },
326+
CreateNodeParams {
327+
ctx,
328+
graph,
329+
patterns,
330+
build_node_callback: &mut |graph, pattern_indices, path| {
331+
let struct_name = concrete_struct_id.struct_id(ctx.db).name(ctx.db);
332+
build_node_callback(graph, pattern_indices, format!("{struct_name}{{{path}}}"))
333+
},
334+
location,
335+
},
309336
&inner_vars,
310337
&types,
311338
0,
@@ -334,7 +361,7 @@ fn create_node_for_tuple_inner<'db>(
334361
let CreateNodeParams { ctx, graph, patterns, build_node_callback, location } = params;
335362

336363
if item_idx == types.len() {
337-
return build_node_callback(graph, FilteredPatterns::all(patterns.len()));
364+
return build_node_callback(graph, FilteredPatterns::all(patterns.len()), "".into());
338365
}
339366

340367
// If the type is a struct, get the current member.
@@ -399,15 +426,19 @@ fn create_node_for_tuple_inner<'db>(
399426
ctx,
400427
graph,
401428
patterns: &patterns_ref,
402-
build_node_callback: &mut |graph, pattern_indices| {
429+
build_node_callback: &mut |graph, pattern_indices, path_head| {
403430
// Call `create_node_for_tuple_inner` recursively to handle the rest of the tuple.
404431
create_node_for_tuple_inner(
405432
CreateNodeParams {
406433
ctx,
407434
graph,
408435
patterns: &pattern_indices.indices().map(|idx| patterns[idx]).collect_vec(),
409-
build_node_callback: &mut |graph, pattern_indices_inner| {
410-
build_node_callback(graph, pattern_indices_inner.lift(&pattern_indices))
436+
build_node_callback: &mut |graph, pattern_indices_inner, path_tail| {
437+
build_node_callback(
438+
graph,
439+
pattern_indices_inner.lift(&pattern_indices),
440+
add_item_to_path(ctx.db, &path_head, &path_tail, current_member),
441+
)
411442
},
412443
location,
413444
},
@@ -423,6 +454,34 @@ fn create_node_for_tuple_inner<'db>(
423454
)
424455
}
425456

457+
/// Concatenates the given `item` to the given `path_tail`.
458+
///
459+
/// Adds the member name if the current item is a struct, and adds a comma if necessary.
460+
///
461+
/// `current_member` is `None` for tuples, and `Some` for structs.
462+
///
463+
/// For example, for tuples (`current_member` is `None`):
464+
/// if `item` is `A` and `path_tail` is `B, C`, the result is `A, B, C`.
465+
/// For structs (`current_member` is `Some`):
466+
/// if `item` is `A` and `path_tail` is `b: B, c: C`, the result will be of the form:
467+
/// `a: A, b: B, c: C`.
468+
fn add_item_to_path<'db>(
469+
db: &dyn Database,
470+
item: &String,
471+
path_tail: &String,
472+
current_member: Option<&semantic::Member<'db>>,
473+
) -> String {
474+
// If it's a struct, add the member name.
475+
let item_str = if let Some(current_member) = current_member {
476+
format!("{}: {}", current_member.id.name(db), item)
477+
} else {
478+
item.clone()
479+
};
480+
481+
// If it's not the only item, add a comma.
482+
if path_tail.is_empty() { item_str } else { format!("{item_str}, {path_tail}") }
483+
}
484+
426485
/// Handles the u256 literal as if it was a pattern of the form `u256 { low: ..., high: ... }`.
427486
///
428487
/// According to `item_idx`, returns the low or the high part of the value as a u128 literal
@@ -528,7 +587,7 @@ fn create_node_for_value<'db>(
528587
};
529588

530589
// First, construct a node that handles the otherwise patterns.
531-
let mut current_node = build_node_callback(graph, otherwise_filter.clone());
590+
let mut current_node = build_node_callback(graph, otherwise_filter.clone(), "_".into());
532591

533592
// Go over the literals (in reverse order), and construct a chain of [BooleanIf] nodes that
534593
// handle each literal.
@@ -537,7 +596,7 @@ fn create_node_for_value<'db>(
537596
if *literal < value_match_size_bigint {
538597
continue;
539598
}
540-
let node_if_literal = build_node_callback(graph, filter.clone());
599+
let node_if_literal = build_node_callback(graph, filter.clone(), literal.to_string());
541600

542601
// Don't add an [EqualsLiteral] node if both branches lead to the same node.
543602
if node_if_literal == current_node {
@@ -567,7 +626,9 @@ fn create_node_for_value<'db>(
567626
let in_range_var = graph.new_var(bounded_int_ty, graph.var_location(input_var));
568627

569628
let nodes = (0..value_match_size)
570-
.map(|i| build_node_callback(graph, literals_map[&BigInt::from(i)].0.clone()))
629+
.map(|i| {
630+
build_node_callback(graph, literals_map[&BigInt::from(i)].0.clone(), i.to_string())
631+
})
571632
.collect();
572633
let value_match_node = graph
573634
.add_node(FlowControlNode::ValueMatch(ValueMatch { matched_var: in_range_var, nodes }));

crates/cairo-lang-lowering/src/lower/flow_control/test_data/match

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,35 +1384,45 @@ End:
13841384
test_create_graph(expect_diagnostics: true)
13851385

13861386
//! > function_code
1387-
fn foo(x: Option<Option<felt252>>) -> felt252 {
1387+
fn foo(x: MyStruct) -> felt252 {
13881388
match x {
1389-
Some(Some(_)) => 1,
1390-
None => 2,
1389+
MyStruct { a: Some(Some(_)), .. } => 1,
1390+
MyStruct { b: (None, _, _), .. } => 1,
13911391
}
13921392
}
13931393

1394+
//! > module_code
1395+
struct MyStruct {
1396+
a: Option<Option<felt252>>,
1397+
b: (Option<felt252>, felt252, felt252),
1398+
}
1399+
13941400
//! > graph
1395-
Root: 5
1396-
5 EvaluateExpr { expr: ExprId(0), var_id: v0, next: NodeId(4) }
1397-
4 EnumMatch { matched_var: v0, variants: (NodeId(3), v1), (NodeId(1), v4)}
1398-
3 EnumMatch { matched_var: v1, variants: (NodeId(0), v2), (NodeId(2), v3)}
1399-
2 Missing
1401+
Root: 9
1402+
9 EvaluateExpr { expr: ExprId(0), var_id: v0, next: NodeId(8) }
1403+
8 Deconstruct { input: v0, outputs: [v1, v2], next: NodeId(7) }
1404+
7 EnumMatch { matched_var: v1, variants: (NodeId(6), v3), (NodeId(5), v16)}
1405+
6 EnumMatch { matched_var: v3, variants: (NodeId(2), v4), (NodeId(5), v10)}
1406+
5 Deconstruct { input: v2, outputs: [v11, v12, v13], next: NodeId(4) }
1407+
4 EnumMatch { matched_var: v11, variants: (NodeId(3), v14), (NodeId(1), v15)}
1408+
3 Missing
1409+
2 Deconstruct { input: v2, outputs: [v5, v6, v7], next: NodeId(0) }
14001410
1 ArmExpr { expr: ExprId(2) }
14011411
0 ArmExpr { expr: ExprId(1) }
14021412

14031413
//! > semantic_diagnostics
14041414

14051415
//! > lowering_diagnostics
1406-
error: Match is non exhaustive - add a wildcard pattern (`_`).
1407-
--> lib.cairo:2:5-5:5
1416+
error: Missing match arm: `MyStruct{a: Some(None(_)), b: (Some(_), _, _)}` not covered.
1417+
--> lib.cairo:6:5-9:5
14081418
match x {
14091419
_____^
14101420
| ...
14111421
| }
14121422
|_____^
14131423

14141424
//! > lowered
1415-
Parameters: v0: core::option::Option::<core::option::Option::<core::felt252>>
1425+
Parameters: v0: test::MyStruct
14161426

14171427
//! > ==========================================================================
14181428

@@ -1532,7 +1542,7 @@ Root: 1
15321542
//! > semantic_diagnostics
15331543

15341544
//! > lowering_diagnostics
1535-
error: Match is non exhaustive - add a wildcard pattern (`_`).
1545+
error: Missing match arm: `_` not covered.
15361546
--> lib.cairo:2:5
15371547
match x {}
15381548
^^^^^^^^^^
@@ -1594,7 +1604,7 @@ Root: 1
15941604
//! > semantic_diagnostics
15951605

15961606
//! > lowering_diagnostics
1597-
error: Match is non exhaustive - add a wildcard pattern (`_`).
1607+
error: Missing match arm: `_` not covered.
15981608
--> lib.cairo:5:5
15991609
match x {}
16001610
^^^^^^^^^^

0 commit comments

Comments
 (0)