Skip to content

Commit ec234fe

Browse files
author
Jesus Lapastora Núñez
committed
Make build_header return unit
Apart from removing some clones, this is to separate the visit process from the data so it is easier to extract phases.
1 parent eeacbb7 commit ec234fe

File tree

3 files changed

+31
-26
lines changed

3 files changed

+31
-26
lines changed

engine/baml-lib/ast/src/ast/baml_vis/graph.rs

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -229,15 +229,23 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
229229
.map(|h| h.hid)
230230
.collect();
231231

232-
let mut prev_exits = None;
233-
for hid in items {
234-
let (entry, exits) = self.build_header(hid, visited_scopes, parent_cluster);
235-
if let Some(prev) = prev_exits.take() {
236-
for e in prev {
237-
self.graph.edges.push(Edge { from: e, to: entry });
238-
}
239-
}
240-
prev_exits = Some(exits);
232+
// post-order: build headers inside scope. <- parent_cluster
233+
for &hid in &items {
234+
self.build_header(hid, visited_scopes, parent_cluster);
235+
}
236+
237+
// unordered: add edges from scope <- header_entry, header_exits
238+
for (hid, prev_hid) in items[1..].iter().zip(&items) {
239+
let entry = self.header_entry[hid];
240+
let prev_exits = &self.header_exits[prev_hid];
241+
// NOTE: `extend` on a loop. TL;DR: each exits vec is pretty small.
242+
// More thorough explanation at the end of `build_header`.
243+
self.graph.edges.extend(
244+
prev_exits
245+
.iter()
246+
.copied()
247+
.map(|e| Edge { from: e, to: entry }),
248+
);
241249
}
242250
}
243251

@@ -251,19 +259,14 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
251259
// TODO: understand why this exists!
252260
visited_scopes: &mut HashSet<ScopeId>,
253261
parent_cluster: Option<ClusterId>,
254-
) -> (NodeId, Vec<NodeId>) {
262+
) {
255263
// NOTE:
256264
// - `build_scope_sequence` is only called for `nested_children`, which we also have a
257265
// HashSet of.
258266

259267
// TODO: there should be no cache hit since there are no cycles!
260-
if let Some(entry) = self.header_entry.get(&hid).copied() {
261-
let exits = self
262-
.header_exits
263-
.get(&hid)
264-
.cloned()
265-
.unwrap_or_else(|| vec![entry]);
266-
return (entry, exits);
268+
if self.header_entry.contains_key(&hid) {
269+
return;
267270
}
268271
let header = self.by_hid[&hid];
269272
let md_children = self.md_children.get(&hid).map(Vec::as_slice).unwrap_or(&[]);
@@ -285,10 +288,13 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
285288
});
286289
self.header_entry.insert(hid, node_id);
287290
self.header_exits.insert(hid, vec![node_id]);
291+
292+
// unordered: render
293+
288294
if self.cfg.show_call_nodes {
289295
self.render_calls_for_header(hid, node_id, parent_cluster);
290296
}
291-
return (node_id, vec![node_id]);
297+
return;
292298
}
293299

294300
if header.label_kind == HeaderLabelKind::If {
@@ -390,7 +396,7 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
390396
branch_exits
391397
};
392398
self.header_exits.insert(hid, outward.clone());
393-
return (decision_id, outward);
399+
return;
394400
}
395401

396402
let single_nested_child_has_multiple_items = nested_children.len() == 1 && {
@@ -467,8 +473,8 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
467473
vec![node_id]
468474
};
469475

470-
self.header_exits.insert(hid, exits.clone());
471-
return (node_id, exits);
476+
self.header_exits.insert(hid, exits);
477+
return;
472478
}
473479

474480
// pre-order: assign cluster id cluster_id <- header, parent_cluster
@@ -504,7 +510,7 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
504510
// Left scan for choosing exits, although choosing fn is not expensive so it can be
505511
// executed twice.
506512

507-
let mut choose_exits_hid = |child_hid| {
513+
let choose_exits_hid = |child_hid| {
508514
// NOTE: since nested children Hids are marked, can we use pre?
509515
let prebuilt_scope_last_exits = if nested_children.contains(&child_hid) {
510516
let child_scope = self.by_hid[&child_hid].scope;
@@ -551,8 +557,7 @@ impl<'index, 'pre> GraphBuilder<'index, 'pre> {
551557
let entry = first_rep;
552558
self.header_entry.insert(hid, entry);
553559
let exits = self.header_exits[&prev_exits_hid].to_owned();
554-
self.header_exits.insert(hid, exits.clone());
555-
(entry, exits)
560+
self.header_exits.insert(hid, exits);
556561
}
557562

558563
/// If header is in [`HeaderIndex::header_calls`], inserts & links a call node to the

engine/baml-lib/baml/tests/validation_files/headers/ai_content_pipeline.mmd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ flowchart TD
4343
n7 --> n8
4444
n4 --> n7
4545
n6 --> n7
46-
n1 --> n2
4746
n9 --> n10
4847
n9 --> n11
48+
n1 --> n2
4949
n8 --> n9
5050
n10 --> n12
5151
n11 --> n12

engine/baml-lib/baml/tests/validation_files/headers/edge_cases.mmd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ flowchart TD
101101
n29 --> n30
102102
n28 --> n29
103103
n27 --> n28
104-
n33 --> n34
105104
n35 --> n36
106105
n36 --> n37
106+
n33 --> n34
107107
n34 --> n35
108108
n38 --> n39
109109
n38 --> n40

0 commit comments

Comments
 (0)