Skip to content

Commit b24d428

Browse files
Correctly implement NotifyBlock instruction (#2678)
Remove the hack used to implement `Instruction::NotifyBlock` and properly store metadata somewhere else and index into it.
1 parent 1b6efb3 commit b24d428

File tree

11 files changed

+126
-50
lines changed

11 files changed

+126
-50
lines changed

engine/Cargo.lock

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

engine/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ default-members = [
5050

5151
[workspace.dependencies]
5252
anyhow = "1.0"
53-
arraystring = "0.3.0"
5453
askama = { version = "0.14.0", features = ["code-in-doc"] }
5554
axum = "0.7.5"
5655
baml-cli = { path = "cli" }

engine/baml-compiler/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ license-file.workspace = true
88

99
[dependencies]
1010
anyhow = { workspace = true }
11-
arraystring.workspace = true
1211
baml-types = { path = "../baml-lib/baml-types" }
1312
baml-vm = { path = "../baml-vm" }
1413
internal-baml-ast = { path = "../baml-lib/ast" }

engine/baml-compiler/src/codegen.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use std::collections::{HashMap, HashSet};
44

5-
use arraystring::{typenum::U255, ArrayString};
65
use baml_types::{ir_type::TypeIR, BamlMap, BamlMediaType, BamlValueWithMeta, TypeValue};
76
use baml_vm::{
87
BamlVmProgram, BinOp, Bytecode, Class, CmpOp, Enum, Function, FunctionKind, GlobalIndex,
@@ -152,6 +151,7 @@ fn compile_thir_to_bytecode(
152151
kind: FunctionKind::Llm,
153152
locals_in_scope: vec![func.parameters.iter().map(|p| p.name.clone()).collect()],
154153
span: func.span.clone(),
154+
block_notifications: Vec::new(),
155155
});
156156

157157
let object_index = objects.insert(bytecode_llm_function);
@@ -235,6 +235,7 @@ fn compile_thir_to_bytecode(
235235
kind: FunctionKind::Native(func),
236236
locals_in_scope: vec![], // TODO.
237237
span: Span::fake_builtin_baml(),
238+
block_notifications: Vec::new(),
238239
});
239240

240241
let object_index = objects.insert(native_function);
@@ -247,6 +248,7 @@ fn compile_thir_to_bytecode(
247248
kind: FunctionKind::Future,
248249
locals_in_scope: vec![],
249250
span: Span::fake_builtin_baml(),
251+
block_notifications: Vec::new(),
250252
}))));
251253

252254
let mut resolved_class_names = HashMap::new();
@@ -431,6 +433,9 @@ struct HirCompiler<'g> {
431433
/// `AllocInstance` instructions that have a placeholder, which must be resolved when location
432434
/// of the class object is resolved.
433435
class_alloc_patch_list: &'g mut Vec<AllocInstancePatch>,
436+
437+
/// Block notifications for the current function being compiled.
438+
block_notifications: Vec<baml_vm::bytecode::BlockNotification>,
434439
}
435440

436441
#[derive(Debug)]
@@ -470,6 +475,7 @@ impl<'g> HirCompiler<'g> {
470475
scopes: Vec::new(),
471476
current_source_line: 0,
472477
locals_in_scope: Vec::new(),
478+
block_notifications: Vec::new(),
473479
}
474480
}
475481

@@ -508,6 +514,7 @@ impl<'g> HirCompiler<'g> {
508514
})),
509515

510516
span: func.span.clone(),
517+
block_notifications: self.block_notifications.clone(),
511518
})
512519
}
513520

@@ -1585,18 +1592,24 @@ impl<'g> HirCompiler<'g> {
15851592
self.emit(Instruction::LoadConst(const_index));
15861593
}
15871594

1588-
fn emit_annotated_block(&mut self, v: &str) {
1589-
self.emit_string_literal(v);
1595+
fn emit_annotated_block(&mut self, annotation: &str) {
1596+
// Create the notification metadata
1597+
let notification = baml_vm::bytecode::BlockNotification {
1598+
function_name: String::new(), // Will be populated at runtime from Function::name
1599+
block_name: annotation.to_string(),
1600+
level: self.scopes.len(), // Current scope depth (1-based)
1601+
block_type: baml_vm::bytecode::BlockNotificationType::Statement,
1602+
is_enter: true,
1603+
};
15901604

1591-
self.emit(Instruction::NotifyBlock(
1592-
baml_vm::bytecode::BlockNotification {
1593-
function_name: ArrayString::<U255>::from_str_truncate(v),
1594-
block_name: ArrayString::<U255>::from_str_truncate(v),
1595-
level: 1,
1596-
block_type: baml_vm::bytecode::BlockNotificationType::Statement,
1597-
is_enter: true,
1598-
},
1599-
));
1605+
// Add to the function's notification list
1606+
let notification_index = self.block_notifications.len();
1607+
self.block_notifications.push(notification);
1608+
1609+
// Emit instruction with just the index
1610+
self.emit(Instruction::NotifyBlock(notification_index));
1611+
1612+
// TODO: Emit exit notification when leaving the block
16001613
}
16011614

16021615
/// Emits a single instruction and returns the index of the instruction.

engine/baml-vm/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ description = "BAML Virtual Machine"
77
license-file.workspace = true
88

99
[dependencies]
10-
arraystring.workspace = true
1110
colored.workspace = true
1211
tokio = { workspace = true }
1312
baml-types = { path = "../baml-lib/baml-types" }

engine/baml-vm/src/bytecode.rs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
//! Instruction set and bytecode representation.
22
3-
use arraystring::{typenum::U255, ArrayString};
4-
53
use crate::{types::Value, GlobalIndex, ObjectIndex};
64

75
/// Individual bytecode instruction.
@@ -242,15 +240,17 @@ pub enum Instruction {
242240

243241
/// Notifies about entering or exiting a block.
244242
///
245-
/// Format: `NOTIFY_BLOCK function_name block_name`
246-
NotifyBlock(BlockNotification),
243+
/// Format: `NOTIFY_BLOCK block_index` where `block_index` is the index
244+
/// into the current function's block_notifications array.
245+
NotifyBlock(usize),
247246
}
248247

249-
#[derive(Clone, Copy, Debug, PartialEq)]
248+
/// Block notification metadata stored in the Function struct.
249+
/// The function_name field is populated at runtime from the Function containing this notification.
250+
#[derive(Clone, Debug, PartialEq)]
250251
pub struct BlockNotification {
251-
// This is a hack cause i don't wanna deal with implementing Copy by allocating this on the heap + doing an object pointer.
252-
pub function_name: ArrayString<U255>,
253-
pub block_name: ArrayString<U255>,
252+
pub function_name: String, // Populated at runtime from Function::name
253+
pub block_name: String,
254254
pub level: usize,
255255
pub block_type: BlockNotificationType,
256256
pub is_enter: bool,
@@ -368,18 +368,8 @@ impl std::fmt::Display for Instruction {
368368
Instruction::Assert => f.write_str("ASSERT"),
369369
Instruction::AllocMap(n) => write!(f, "ALLOC_MAP {n}"),
370370
Instruction::Watch(i) => write!(f, "WATCH {i}"),
371-
Instruction::NotifyBlock(notification) => {
372-
write!(
373-
f,
374-
"{}_BLOCK {function_name}.{block_name}",
375-
if notification.is_enter {
376-
"ENTER"
377-
} else {
378-
"EXIT"
379-
},
380-
function_name = &notification.function_name,
381-
block_name = &notification.block_name,
382-
)
371+
Instruction::NotifyBlock(block_index) => {
372+
write!(f, "NOTIFY_BLOCK {block_index}")
383373
}
384374
Instruction::Notify(i) => write!(f, "NOTIFY {i}"),
385375
}

engine/baml-vm/src/debug.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,12 @@ pub fn display_instruction(
5757
let instruction = &function.bytecode.instructions[instruction_ptr as usize];
5858

5959
let metadata = match instruction {
60-
Instruction::NotifyBlock(notification) => {
61-
format!("({})", &notification.block_name)
60+
Instruction::NotifyBlock(block_index) => {
61+
if let Some(notification) = function.block_notifications.get(*block_index) {
62+
format!("({})", &notification.block_name)
63+
} else {
64+
format!("(invalid block index: {})", block_index)
65+
}
6266
}
6367
Instruction::LoadConst(index) => format!(
6468
"({})",

engine/baml-vm/src/types.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ pub struct Function {
5353

5454
/// Span of the function as computed by the parser.
5555
pub span: internal_baml_diagnostics::Span,
56+
57+
/// Block notifications for this function.
58+
///
59+
/// Stores metadata about annotated blocks (//# annotations) in this function.
60+
/// Instructions reference these by index.
61+
pub block_notifications: Vec<crate::bytecode::BlockNotification>,
5662
}
5763

5864
impl std::fmt::Display for Function {

engine/baml-vm/src/vm.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,22 @@ impl Vm {
639639
// }
640640

641641
match function.bytecode.instructions[instruction_ptr as usize] {
642-
Instruction::NotifyBlock(notification) => {
643-
return Ok(VmExecState::Notify(WatchNotification::Block(notification)));
642+
Instruction::NotifyBlock(block_index) => {
643+
// Get the notification from the function's storage
644+
let notification = &function.block_notifications[block_index];
645+
646+
// Create a copy with the function name populated
647+
let full_notification = crate::bytecode::BlockNotification {
648+
function_name: function.name.clone(),
649+
block_name: notification.block_name.clone(),
650+
level: notification.level,
651+
block_type: notification.block_type,
652+
is_enter: notification.is_enter,
653+
};
654+
655+
return Ok(VmExecState::Notify(WatchNotification::Block(
656+
full_notification,
657+
)));
644658
}
645659
Instruction::LoadConst(index) => {
646660
let value = &function.bytecode.constants[index];

engine/baml-vm/tests/common.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ pub fn assert_vm_executes_bytecode_with_inspection(
462462
vec![names]
463463
},
464464
span: internal_baml_diagnostics::Span::fake(),
465+
block_notifications: Vec::new(),
465466
};
466467

467468
let objects = vec![VmObject::Function(function)];

0 commit comments

Comments
 (0)