diff --git a/fix-codegen/src/disassembler.rs b/fix-codegen/src/disassembler.rs index e16601a..e183dc8 100644 --- a/fix-codegen/src/disassembler.rs +++ b/fix-codegen/src/disassembler.rs @@ -320,6 +320,27 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { let span_id = self.read_u32(); ("SelectDynamic", format!("span={}", span_id)) } + Op::HasAttrPathStatic => { + let span_id = self.read_u32(); + let key_id = self.read_u32(); + ( + "HasAttrPathStatic", + format!("key={} span={}", self.ctx.resolve_string(key_id), span_id), + ) + } + Op::HasAttrPathDynamic => { + let span_id = self.read_u32(); + ("HasAttrPathDynamic", format!("span={}", span_id)) + } + Op::HasAttrStatic => { + let key_id = self.read_u32(); + ( + "HasAttrStatic", + format!("key={}", self.ctx.resolve_string(key_id)), + ) + } + Op::HasAttrDynamic => ("HasAttrDynamic", String::new()), + Op::HasAttrResolve => ("HasAttrResolve", String::new()), Op::JumpIfSelectSucceeded => { let offset = self.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; @@ -328,9 +349,13 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { format!("-> {:04x} offset={}", target, offset), ) } - Op::HasAttr => { - let path_len = self.read_u16(); - ("HasAttr", format!("path_len={}", path_len)) + Op::JumpIfSelectFailed => { + let offset = self.read_i32(); + let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; + ( + "JumpIfSelectFailed", + format!("-> {:04x} offset={}", target, offset), + ) } Op::MakeList => { diff --git a/fix-codegen/src/lib.rs b/fix-codegen/src/lib.rs index 9c02054..a820834 100644 --- a/fix-codegen/src/lib.rs +++ b/fix-codegen/src/lib.rs @@ -47,8 +47,13 @@ pub enum Op { MakeEmptyAttrs, SelectStatic, SelectDynamic, + HasAttrPathStatic, + HasAttrPathDynamic, + HasAttrStatic, + HasAttrDynamic, + HasAttrResolve, JumpIfSelectSucceeded, - HasAttr, + JumpIfSelectFailed, MakeList, MakeEmptyList, @@ -822,6 +827,7 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { ) { self.emit_expr(expr); + let mut dynamic_patches = Vec::new(); for attr in attrpath.iter() { match *attr { Attr::Str(sym, _) => { @@ -831,6 +837,8 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { self.emit_str_id(sym); } Attr::Dynamic(key_expr, _) => { + self.emit_op(Op::JumpIfSelectFailed); + dynamic_patches.push(self.emit_i32_placeholder()); self.emit_expr(key_expr); let span_id = self.ctx.register_span(span); self.emit_op(Op::SelectDynamic); @@ -840,35 +848,65 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { } if let Some(default) = default { + let before: i32 = self.ctx.get_code().len().try_into().unwrap(); + for patch in dynamic_patches { + self.patch_jump_target(patch); + } self.emit_op(Op::JumpIfSelectSucceeded); let placeholder = self.emit_i32_placeholder(); - let before: i32 = self.ctx.get_code().len().try_into().unwrap(); self.emit_expr(default); let after: i32 = self.ctx.get_code().len().try_into().unwrap(); - self.patch_i32(placeholder, after - before); + // Offset is relative to after the placeholder, so subtract the + // size of JumpIfSelectSucceeded (1) + placeholder (4). + self.patch_i32(placeholder, after - before - 5); + } else { + for patch in dynamic_patches { + self.patch_jump_target(patch); + } } } fn emit_has_attr(&mut self, lhs: RawIrRef<'_>, rhs: &[Attr>]) { self.emit_expr(lhs); - for attr in rhs.iter() { - if let Attr::Dynamic(expr, _) = *attr { - self.emit_expr(expr); - } - } - self.emit_op(Op::HasAttr); - self.emit_u16(rhs.len() as u16); - for attr in rhs.iter() { + + let mut dynamic_patches = Vec::new(); + let [attrs @ .., last] = rhs else { + panic!("attrpath is empty"); + }; + for attr in attrs { match *attr { - Attr::Str(sym, _) => { - self.emit_u8(AttrKeyType::Static as u8); + Attr::Str(sym, span) => { + let span_id = self.ctx.register_span(span); + self.emit_op(Op::HasAttrPathStatic); + self.emit_u32(span_id); self.emit_str_id(sym); } - Attr::Dynamic(_, _) => { - self.emit_u8(AttrKeyType::Dynamic as u8); + Attr::Dynamic(key_expr, span) => { + self.emit_op(Op::JumpIfSelectFailed); + dynamic_patches.push(self.emit_i32_placeholder()); + self.emit_expr(key_expr); + let span_id = self.ctx.register_span(span); + self.emit_op(Op::HasAttrPathDynamic); + self.emit_u32(span_id); } } } + match *last { + Attr::Str(sym, _) => { + self.emit_op(Op::HasAttrStatic); + self.emit_str_id(sym); + } + Attr::Dynamic(key_expr, _) => { + self.emit_op(Op::JumpIfSelectFailed); + dynamic_patches.push(self.emit_i32_placeholder()); + self.emit_expr(key_expr); + self.emit_op(Op::HasAttrDynamic); + } + } + for patch in dynamic_patches { + self.patch_jump_target(patch); + } + self.emit_op(Op::HasAttrResolve); } fn emit_with( diff --git a/fix-vm/src/dispatch_tailcall.rs b/fix-vm/src/dispatch_tailcall.rs index 8efe6c5..0f48746 100644 --- a/fix-vm/src/dispatch_tailcall.rs +++ b/fix-vm/src/dispatch_tailcall.rs @@ -148,8 +148,13 @@ tail_fn!(op_make_attrs, (ctx, reader, mc)); tail_fn!(op_make_empty_attrs, ()); tail_fn!(op_select_static, (ctx, reader, mc)); tail_fn!(op_select_dynamic, (ctx, reader, mc)); +tail_fn!(op_has_attr_path_static, (ctx, reader, mc)); +tail_fn!(op_has_attr_path_dynamic, (ctx, reader, mc)); +tail_fn!(op_jump_if_select_failed, (reader)); tail_fn!(op_jump_if_select_succeeded, (reader)); -tail_fn!(op_has_attr, (reader)); +tail_fn!(op_has_attr_static, (reader, mc)); +tail_fn!(op_has_attr_dynamic, (ctx, reader, mc)); +tail_fn!(op_has_attr_resolve, ()); tail_fn!(op_make_list, (ctx, reader, mc)); tail_fn!(op_make_empty_list, ()); @@ -235,8 +240,13 @@ table! { MakeEmptyAttrs => op_make_empty_attrs, SelectStatic => op_select_static, SelectDynamic => op_select_dynamic, + HasAttrPathStatic => op_has_attr_path_static, + HasAttrPathDynamic => op_has_attr_path_dynamic, + HasAttrStatic => op_has_attr_static, + HasAttrDynamic => op_has_attr_dynamic, + HasAttrResolve => op_has_attr_resolve, JumpIfSelectSucceeded => op_jump_if_select_succeeded, - HasAttr => op_has_attr, + JumpIfSelectFailed => op_jump_if_select_failed, MakeList => op_make_list, MakeEmptyList => op_make_empty_list, diff --git a/fix-vm/src/instructions/arithmetic.rs b/fix-vm/src/instructions/arithmetic.rs index c289e46..258aad8 100644 --- a/fix-vm/src/instructions/arithmetic.rs +++ b/fix-vm/src/instructions/arithmetic.rs @@ -15,8 +15,8 @@ impl<'gc> crate::Vm<'gc> { ) -> Step { let (lhs, rhs) = self.try_force::<(StrictValue, StrictValue)>(reader, mc)?; if let (Some(ls), Some(rs)) = ( - VmContextExt::get_string(ctx, lhs), - VmContextExt::get_string(ctx, rhs), + ctx.get_string(lhs), + ctx.get_string(rhs), ) { let ns = Gc::new(mc, crate::NixString::new(format!("{ls}{rs}"))); self.push(Value::new_gc(ns)); @@ -236,8 +236,8 @@ impl<'gc> crate::Vm<'gc> { return Ok(true); } if let (Some(a), Some(b)) = ( - VmContextExt::get_string(ctx, lhs), - VmContextExt::get_string(ctx, rhs), + ctx.get_string(lhs), + ctx.get_string(rhs), ) { return Ok(a == b); } diff --git a/fix-vm/src/instructions/collections.rs b/fix-vm/src/instructions/collections.rs index 92d9d1b..0661cfc 100644 --- a/fix-vm/src/instructions/collections.rs +++ b/fix-vm/src/instructions/collections.rs @@ -1,9 +1,12 @@ +use fix_common::StringId; use fix_error::Error; use gc_arena::Gc; use smallvec::SmallVec; +use crate::value::NixType; use crate::{ - AttrKeyData, AttrSet, BytecodeReader, List, NixString, OperandData, Step, StrictValue, Value, + AttrKeyData, AttrSet, BytecodeReader, List, OperandData, Step, StrictValue, Value, + VmContextExt, }; impl<'gc> crate::Vm<'gc> { @@ -63,21 +66,7 @@ impl<'gc> crate::Vm<'gc> { Some(v) => { self.push(v); } - None => loop { - let byte = reader.bytecode()[reader.pc()]; - if byte == fix_codegen::Op::SelectStatic as u8 { - reader.set_pc(reader.pc() + 1 + 4 + 4); - } else if byte == fix_codegen::Op::SelectDynamic as u8 { - reader.set_pc(reader.pc() + 1 + 4); - } else if byte == fix_codegen::Op::JumpIfSelectSucceeded as u8 { - reader.set_pc(reader.pc() + 1 + 4); - break; - } else { - let name = ctx.resolve_string(key); - return self - .finish_err(Error::eval_error(format!("attribute '{name}' missing"))); - } - }, + None => return self.select_skip(key, ctx, reader), } Step::Continue(()) } @@ -93,40 +82,205 @@ impl<'gc> crate::Vm<'gc> { let (attrset, key_val) = self.try_force::<(Gc, StrictValue)>(reader, mc)?; - let key_sid = if let Some(sid) = key_val.as_inline::() { - sid - } else if let Some(ns) = key_val.as_gc::() { - ctx.intern_string(ns.as_str()) - } else { - return self.finish_err(Error::eval_error("dynamic select key must be a string")); + let key_sid = match ctx.get_string_id(key_val) { + Ok(id) => id, + Err(got) => return self.finish_type_err(NixType::String, got), }; match attrset.lookup(key_sid) { Some(v) => { self.push(v); } - None => { - let name = ctx.resolve_string(key_sid); - return self.finish_err(Error::eval_error(format!("attribute '{name}' missing"))); + None => return self.select_skip(key_sid, ctx, reader), + } + Step::Continue(()) + } + + /// Skip the rest of a **Select** attrpath after a missing attribute. + /// Only recognises Select opcodes and jumps; encountering any other + /// opcode means we've reached the end of the select sequence and + /// should report the missing-attribute error. + fn select_skip( + &mut self, + key: StringId, + ctx: &mut impl crate::VmContext, + reader: &mut BytecodeReader<'_>, + ) -> Step { + use fix_codegen::Op::*; + loop { + match reader.read_op() { + SelectStatic => { + reader.set_pc(reader.pc() + 4 + 4); + } + SelectDynamic => { + reader.set_pc(reader.pc() + 4); + } + JumpIfSelectSucceeded => { + reader.set_pc(reader.pc() + 4); + break Step::Continue(()); + } + JumpIfSelectFailed => { + let offset = reader.read_i32(); + reader.set_pc(((reader.pc() as isize) + (offset as isize)) as usize); + } + _ => { + let name = ctx.resolve_string(key); + return self + .finish_err(Error::eval_error(format!("attribute '{name}' missing"))); + } } } + } + + /// Skip the rest of a **HasAttr** attrpath after an intermediate + /// lookup failed. Only recognises HasAttr opcodes and jumps. + fn has_attr_skip(&mut self, reader: &mut BytecodeReader<'_>) -> Step { + use fix_codegen::Op::*; + loop { + match reader.read_op() { + HasAttrPathStatic => { + reader.set_pc(reader.pc() + 4 + 4); + } + HasAttrPathDynamic => { + reader.set_pc(reader.pc() + 4); + } + HasAttrStatic => { + reader.set_pc(reader.pc() + 4); + break Step::Continue(()); + } + JumpIfSelectFailed => { + let offset = reader.read_i32(); + reader.set_pc(((reader.pc() as isize) + (offset as isize)) as usize); + } + HasAttrDynamic => { + break Step::Continue(()); + } + HasAttrResolve => { + reader.set_pc(reader.pc() - 1); + break Step::Continue(()); + } + other => { + unreachable!( + "unexpected opcode {:?} in has_attr_skip", + other as u8 + ) + } + } + } + } + + #[inline(always)] + pub(crate) fn op_has_attr_path_static( + &mut self, + _ctx: &mut impl crate::VmContext, + reader: &mut BytecodeReader<'_>, + mc: &gc_arena::Mutation<'gc>, + ) -> Step { + let _span_id = reader.read_u32(); + let key = reader.read_string_id(); + + let current = self.try_force::(reader, mc)?; + + match current.as_gc::().and_then(|attrs| attrs.lookup(key)) { + Some(v) => { + self.push(v); + } + None => return self.has_attr_skip(reader), + } Step::Continue(()) } #[inline(always)] - pub(crate) fn op_jump_if_select_succeeded( + pub(crate) fn op_has_attr_path_dynamic( &mut self, + ctx: &mut impl crate::VmContext, reader: &mut BytecodeReader<'_>, + mc: &gc_arena::Mutation<'gc>, ) -> Step { + let _span_id = reader.read_u32(); + + let (current, key_val) = self.try_force::<(StrictValue, StrictValue)>(reader, mc)?; + + let key_sid = match ctx.get_string_id(key_val) { + Ok(id) => id, + Err(got) => return self.finish_type_err(NixType::String, got), + }; + + match current.as_gc::().and_then(|attrs| attrs.lookup(key_sid)) { + Some(v) => { + self.push(v); + } + None => return self.has_attr_skip(reader), + } + Step::Continue(()) + } + + #[inline(always)] + pub(crate) fn op_jump_if_select_failed(&mut self, reader: &mut BytecodeReader<'_>) -> Step { + // No-op + let _offset = reader.read_i32(); + Step::Continue(()) + } + + #[inline(always)] + pub(crate) fn op_jump_if_select_succeeded(&mut self, reader: &mut BytecodeReader<'_>) -> Step { let offset = reader.read_i32(); reader.set_pc(((reader.pc() as isize) + (offset as isize)) as usize); Step::Continue(()) } #[inline(always)] - pub(crate) fn op_has_attr(&mut self, reader: &mut BytecodeReader<'_>) -> Step { - let _n = reader.read_u16() as usize; - todo!("HasAttr"); + pub(crate) fn op_has_attr_static( + &mut self, + reader: &mut BytecodeReader<'_>, + mc: &gc_arena::Mutation<'gc>, + ) -> Step { + let key = reader.read_string_id(); + let current = self.try_force::(reader, mc)?; + + self.push(Value::new_inline( + current + .as_gc::() + .and_then(|attrs| attrs.lookup(key)) + .is_some(), + )); + // Skip HasAttrResolve + reader.set_pc(reader.pc() + 1); + + Step::Continue(()) + } + + #[inline(always)] + pub(crate) fn op_has_attr_dynamic( + &mut self, + ctx: &mut impl crate::VmContext, + reader: &mut BytecodeReader<'_>, + mc: &gc_arena::Mutation<'gc>, + ) -> Step { + let (current, dyn_key) = self.try_force::<(StrictValue, StrictValue)>(reader, mc)?; + + let key_sid = match ctx.get_string_id(dyn_key) { + Ok(id) => id, + Err(got) => return self.finish_type_err(NixType::String, got), + }; + + self.push(Value::new_inline( + current + .as_gc::() + .and_then(|attrs| attrs.lookup(key_sid)) + .is_some(), + )); + // Skip HasAttrResolve + reader.set_pc(reader.pc() + 1); + + Step::Continue(()) + } + + #[inline(always)] + pub(crate) fn op_has_attr_resolve(&mut self) -> Step { + // If we reach here, has_attr check has failed, push false (AttrSet is already popped) + self.push(Value::new_inline(false)); + Step::Continue(()) } #[inline(always)] diff --git a/fix-vm/src/lib.rs b/fix-vm/src/lib.rs index 3c5bae7..f0367b3 100644 --- a/fix-vm/src/lib.rs +++ b/fix-vm/src/lib.rs @@ -74,6 +74,7 @@ pub trait VmContext { pub(crate) trait VmContextExt: VmContext { fn get_string<'a, 'gc: 'a>(&'a self, val: StrictValue<'gc>) -> Option<&'a str>; + fn get_string_id<'a, 'gc: 'a>(&'a mut self, val: StrictValue<'gc>) -> std::result::Result; fn convert_value(&self, val: Value) -> fix_common::Value; } @@ -86,6 +87,16 @@ impl VmContextExt for T { } } + fn get_string_id<'a, 'gc: 'a>(&'a mut self, val: StrictValue<'gc>) -> std::result::Result { + if let Some(sid) = val.as_inline::() { + Ok(sid) + } else if let Some(s) = val.as_gc::().map(|ns| ns.as_ref().as_str()) { + Ok(self.intern_string(s)) + } else { + Err(val.ty()) + } + } + fn convert_value(&self, val: Value) -> fix_common::Value { use fix_common::Value; if let Some(i) = val.as_inline::() { @@ -531,8 +542,13 @@ impl<'gc> Vm<'gc> { MakeEmptyAttrs => self.op_make_empty_attrs(), SelectStatic => self.op_select_static(ctx, &mut reader, mc), SelectDynamic => self.op_select_dynamic(ctx, &mut reader, mc), + HasAttrPathStatic => self.op_has_attr_path_static(ctx, &mut reader, mc), + HasAttrPathDynamic => self.op_has_attr_path_dynamic(ctx, &mut reader, mc), + HasAttrStatic => self.op_has_attr_static(&mut reader, mc), + HasAttrDynamic => self.op_has_attr_dynamic(ctx, &mut reader, mc), + HasAttrResolve => self.op_has_attr_resolve(), + JumpIfSelectFailed => self.op_jump_if_select_failed(&mut reader), JumpIfSelectSucceeded => self.op_jump_if_select_succeeded(&mut reader), - HasAttr => self.op_has_attr(&mut reader), MakeList => self.op_make_list(ctx, &mut reader, mc), MakeEmptyList => self.op_make_empty_list(),