implement Select and HasAttr

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 => {
+            Op::JumpIfSelectFailed => {
-                let path_len = self.read_u16();
+                let offset = self.read_i32();
-                ("HasAttr", format!("path_len={}", path_len))
+                let target = (current_pc as isize + 1 + 4 + offset as isize) as usize;
+                (
+                    "JumpIfSelectFailed",
+                    format!("-> {:04x} offset={}", target, offset),
+                )
             }
 
             Op::MakeList => {

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<RawIrRef<'_>>]) {
         self.emit_expr(lhs);
-        for attr in rhs.iter() {
+
-            if let Attr::Dynamic(expr, _) = *attr {
+        let mut dynamic_patches = Vec::new();
-                self.emit_expr(expr);
+        let [attrs @ .., last] = rhs else {
-            }
+            panic!("attrpath is empty");
-        }
+        };
-        self.emit_op(Op::HasAttr);
+        for attr in attrs {
-        self.emit_u16(rhs.len() as u16);
-        for attr in rhs.iter() {
             match *attr {
-                Attr::Str(sym, _) => {
+                Attr::Str(sym, span) => {
-                    self.emit_u8(AttrKeyType::Static as u8);
+                    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(_, _) => {
+                Attr::Dynamic(key_expr, span) => {
-                    self.emit_u8(AttrKeyType::Dynamic as u8);
+                    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(

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,

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),
+            ctx.get_string(lhs),
-            VmContextExt::get_string(ctx, rhs),
+            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),
+            ctx.get_string(lhs),
-            VmContextExt::get_string(ctx, rhs),
+            ctx.get_string(rhs),
         ) {
             return Ok(a == 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 {
+            None => return self.select_skip(key, ctx, reader),
-                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")));
-                }
-            },
         }
         Step::Continue(())
     }
@@ -93,40 +82,205 @@ impl<'gc> crate::Vm<'gc> {
 
         let (attrset, key_val) = self.try_force::<(Gc<AttrSet>, StrictValue)>(reader, mc)?;
 
-        let key_sid = if let Some(sid) = key_val.as_inline::<crate::StringId>() {
+        let key_sid = match ctx.get_string_id(key_val) {
-            sid
+            Ok(id) => id,
-        } else if let Some(ns) = key_val.as_gc::<NixString>() {
+            Err(got) => return self.finish_type_err(NixType::String, got),
-            ctx.intern_string(ns.as_str())
-        } else {
-            return self.finish_err(Error::eval_error("dynamic select key must be a string"));
         };
 
         match attrset.lookup(key_sid) {
             Some(v) => {
                 self.push(v);
             }
-            None => {
+            None => return self.select_skip(key_sid, ctx, reader),
-                let name = ctx.resolve_string(key_sid);
+        }
-                return self.finish_err(Error::eval_error(format!("attribute '{name}' missing")));
+        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::<StrictValue>(reader, mc)?;
+
+        match current.as_gc::<AttrSet>().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::<AttrSet>().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 {
+    pub(crate) fn op_has_attr_static(
-        let _n = reader.read_u16() as usize;
+        &mut self,
-        todo!("HasAttr");
+        reader: &mut BytecodeReader<'_>,
+        mc: &gc_arena::Mutation<'gc>,
+    ) -> Step {
+        let key = reader.read_string_id();
+        let current = self.try_force::<StrictValue>(reader, mc)?;
+
+        self.push(Value::new_inline(
+            current
+                .as_gc::<AttrSet>()
+                .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::<AttrSet>()
+                .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)]

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<StringId, NixType>;
     fn convert_value(&self, val: Value) -> fix_common::Value;
 }
 
@@ -86,6 +87,16 @@ impl<T: VmContext> VmContextExt for T {
         }
     }
 
+    fn get_string_id<'a, 'gc: 'a>(&'a mut self, val: StrictValue<'gc>) -> std::result::Result<StringId, NixType> {
+        if let Some(sid) = val.as_inline::<StringId>() {
+            Ok(sid)
+        } else if let Some(s) = val.as_gc::<NixString>().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::<i32>() {
@@ -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(),