diff --git a/libs/sdk/README.md b/libs/sdk/README.md index c7e3d34d..ce36da1b 100644 --- a/libs/sdk/README.md +++ b/libs/sdk/README.md @@ -97,7 +97,7 @@ automatically. | `st_set_insert(old, v, new)`, `st_set_delete(old, v, new)` | `new` is `old` with `v` added / removed | | `st_array_update(old, i, v, new)` | `new` is `old` with index `i` set to `v` | -Two limits are worth knowing before reaching for these: +One limit is worth knowing before reaching for these: - The transition statements (`st_*_insert` / `_update` / `_delete`) constrain a relation between two container values. They do not compute the new @@ -105,10 +105,6 @@ Two limits are worth knowing before reaching for these: witness from an `unsafe` block. Until the SDK can build container values (see Missing features), the reachable use is relating two containers a script already holds. -- A field read of an object's whole-dict form is fine on inputs and mutates, - but reading a field of an *output* you just built (`out.durability`) is not - supported: the emitter renders it as `initials.out.durability`, a double - anchor podlang has no syntax for, and the module fails to compile. ## Type checking @@ -238,7 +234,6 @@ as opaque entropy, not for byte-exact comparison with the L1 hash. - [x] manifest support - [ ] error pretty print - [x] forbid Object::set after the object has been used in other operations -- [ ] read a field of an output created in the same action (`out.field`) # Test example diff --git a/libs/sdk/src/fmt_podlang.rs b/libs/sdk/src/fmt_podlang.rs index 2dfa7a36..b0622dc3 100644 --- a/libs/sdk/src/fmt_podlang.rs +++ b/libs/sdk/src/fmt_podlang.rs @@ -59,6 +59,13 @@ pub(crate) const fn output_max_ts(base_ts: usize, is_output: bool) -> usize { if is_output { base_ts + 1 } else { base_ts } } +/// Inverse of the ts `output_max_ts` reserves: where an Output's +/// script-final form sits, which is what the `initials` record holds and +/// what TxInsert takes as the object's initial state. +pub(crate) const fn initials_ts(max_ts: usize) -> usize { + max_ts.saturating_sub(1) +} + /// An action's chain max_ts must be at least this for the SDK to pack /// intermediate chain states into a `Chain` record. Below the /// threshold, the per-step scalar wildcards (`chain1`, `chain2`, ...) @@ -246,12 +253,12 @@ fn fmt_record_decls(loader: &Loader, w: &mut dyn fmt::Write) -> fmt::Result { )?; } if let Some(initials) = &meta.initials_entries { - writeln!( - w, - "record {} = ({})", - schema_name_initials(&meta.name), - render(initials), - )?; + let names = initials + .iter() + .map(|e| e.varname.as_str()) + .collect::>() + .join(", "); + writeln!(w, "record {} = ({names})", schema_name_initials(&meta.name),)?; } } Ok(()) @@ -422,29 +429,34 @@ fn fmt_action(action: &ActionContext, loader: &Loader, w: &mut dyn fmt::Write) - // sides that need a wildcard; collapsed sides drop the clause. for o in &meta.object_refs { let max_ts = meta.max_ts(&o.varname); - if meta - .in_entry(&o.varname) - .is_some_and(|(_, e)| e.needs_wildcard) - { - writeln!( - w, - " ArrayContains(io, {}::in_{}, {})", - schema_name_io(&action.name), - o.varname, - fmt_var_at(&o.varname, 0, max_ts), - )?; - } - if meta - .out_entry(&o.varname) - .is_some_and(|(_, e)| e.needs_wildcard) - { - writeln!( - w, - " ArrayContains(io, {}::out_{}, {})", - schema_name_io(&action.name), - o.varname, - fmt_var_at(&o.varname, max_ts, max_ts), - )?; + let io_schema = schema_name_io(&action.name); + for (entry, record, entry_name, ts) in [ + ( + meta.in_entry(&o.varname), + "io", + format!("{io_schema}::in_{}", o.varname), + 0, + ), + ( + meta.out_entry(&o.varname), + "io", + format!("{io_schema}::out_{}", o.varname), + max_ts, + ), + ( + meta.initials_entry(&o.varname), + "initials", + format!("{}::{}", schema_name_initials(&action.name), o.varname), + initials_ts(max_ts), + ), + ] { + if entry.is_some_and(|(_, e)| e.needs_wildcard) { + writeln!( + w, + " ArrayContains({record}, {entry_name}, {})", + fmt_var_at(&o.varname, ts, max_ts), + )?; + } } } // Pin each referenced sub-action alias to its sub's first out diff --git a/libs/sdk/src/lib.rs b/libs/sdk/src/lib.rs index 729d1e59..c87c1dd6 100644 --- a/libs/sdk/src/lib.rs +++ b/libs/sdk/src/lib.rs @@ -475,6 +475,33 @@ impl ActionContext { state.dict_read = true; } } + /// Per-var max ts, including the extra ts an Output reserves for + /// the identity-stamp bump (`fmt_podlang::output_max_ts`). This is + /// the single source for in/out/initials slot positions, consumed + /// by `compute_wildcard_needs`, `ActionMeta`, and `fmt_podlang`. + fn max_ts_per_var(&self) -> HashMap { + let output_vars: HashSet = self + .insts + .iter() + .filter_map(|inst| match inst { + Inst::Object { + io: ObjectIO::Output, + obj, + .. + } => Some(obj.borrow().var_name().to_string()), + _ => None, + }) + .collect(); + self.var_state + .iter() + .map(|(k, v)| { + ( + k.clone(), + fmt_podlang::output_max_ts(v.ts, output_vars.contains(k)), + ) + }) + .collect() + } /// Reject a `set` that cannot hold. `set` writes its initializer into /// the object's dict without advancing the object's ts, so any /// earlier operation that pinned the dict's exact contents would then @@ -537,33 +564,6 @@ impl ActionContext { } Ok(()) } - /// Per-var max ts, including the extra ts an Output reserves for - /// the identity-stamp bump (`fmt_podlang::output_max_ts`). This is - /// the single source for in/out/initials slot positions, consumed - /// by `compute_wildcard_needs`, `ActionMeta`, and `fmt_podlang`. - fn max_ts_per_var(&self) -> HashMap { - let output_vars: HashSet = self - .insts - .iter() - .filter_map(|inst| match inst { - Inst::Object { - io: ObjectIO::Output, - obj, - .. - } => Some(obj.borrow().var_name().to_string()), - _ => None, - }) - .collect(); - self.var_state - .iter() - .map(|(k, v)| { - ( - k.clone(), - fmt_podlang::output_max_ts(v.ts, output_vars.contains(k)), - ) - }) - .collect() - } fn assert_unsafe(&self, unsafe_block: bool) -> RuntimeResult<()> { if self.unsafe_block != unsafe_block { if self.unsafe_block { @@ -783,7 +783,10 @@ impl ActionHandle { // Resolve an Output's pre-identity dict to its slot in the // `Initials` record. let initials_anchor = |obj_name: &str| -> Option { - let slot = meta.initials_slot(obj_name)?; + let (slot, entry) = meta.initials_entry(obj_name)?; + if entry.needs_wildcard { + return None; + } Some((initials_array.as_ref()?, slot as i64).into()) }; @@ -833,30 +836,33 @@ impl ActionHandle { .clone(), _ => post_dict.clone(), }; - if let Some((idx, e)) = meta.in_entry(&varname) - && e.needs_wildcard - { - let st = exe_ctx - .bld - .builder - .priv_op(Operation::array_contains( - Value::from(io_array.clone()), - idx as i64, - Value::from(pre_dict.clone()), - )) - .unwrap(); - array_contains_sts.push(st); - } - if let Some((idx, e)) = meta.out_entry(&varname) - && e.needs_wildcard - { + // The script-final form, which for an Output is the + // dict before TxInsert stamps identity onto it. + let initials_dict = obj.borrow().to_dict(); + // Same forms and order as `fmt_action`'s clauses; the + // two sets have to line up statement for statement. + for (entry, record, dict) in [ + (meta.in_entry(&varname), Some(&io_array), &pre_dict), + (meta.out_entry(&varname), Some(&io_array), &post_dict), + ( + meta.initials_entry(&varname), + initials_array.as_ref(), + &initials_dict, + ), + ] { + let (Some((idx, e)), Some(record)) = (entry, record) else { + continue; + }; + if !e.needs_wildcard { + continue; + } let st = exe_ctx .bld .builder .priv_op(Operation::array_contains( - Value::from(io_array.clone()), + Value::from(record.clone()), idx as i64, - Value::from(post_dict.clone()), + Value::from(dict.clone()), )) .unwrap(); array_contains_sts.push(st); @@ -1900,7 +1906,7 @@ pub struct ActionMeta { /// when there is no such record: an Intro that consumes an output's /// pre-identity dict whole forces that dict to stay a literal /// wildcard, since Intro args can't be anchored. - pub(crate) initials_entries: Option>, + pub(crate) initials_entries: Option>, /// `var_state["chain"].ts` for this action — i.e. the count of /// txlib events recorded by this action (Object insts + sub-action /// calls). Drives whether the chain is packed into `Chain`. @@ -1938,13 +1944,17 @@ impl ActionMeta { self.total_outputs.iter() } - /// Slot for `varname` in the `Initials` record, if such a - /// record exists for this action. - pub(crate) fn initials_slot(&self, varname: &str) -> Option { + /// Find this Output's entry in the `Initials` record, with + /// its slot. `needs_wildcard` is set when the body reads a field of + /// the object's script-final form, which cannot render as an + /// anchored `initials.` because `initials..` would + /// anchor twice. + pub(crate) fn initials_entry(&self, varname: &str) -> Option<(usize, &EntryShape)> { self.initials_entries .as_ref()? .iter() - .position(|name| name == varname) + .enumerate() + .find(|(_, e)| e.varname == varname) } /// Find this Object's in-side entry. Returns its slot in the @@ -1994,10 +2004,14 @@ impl ActionMeta { { return Some(fmt_podlang::Collapse::IO(fmt_podlang::Side::Out)); } - // If we have an "initials" record for this var, and we are at - // the penultimate ts, then the var must be collapsed into - // initials, for passing to TxInsert. - if ts + 1 == max_ts && self.initials_slot(varname).is_some() { + // An Output's script-final form collapses into the initials + // record for passing to TxInsert, unless the body reads one of + // its fields, which needs the form to stay a wildcard. + if ts == fmt_podlang::initials_ts(max_ts) + && self + .initials_entry(varname) + .is_some_and(|(_, e)| !e.needs_wildcard) + { return Some(fmt_podlang::Collapse::Initials); } None @@ -2048,29 +2062,32 @@ impl ActionMeta { _ => {} } } - let (needs_in, needs_out) = compute_wildcard_needs(ctx); + let needs = compute_wildcard_needs(ctx); for r in &meta.object_refs { if r.io.consumes() { meta.in_entries.push(EntryShape { varname: r.varname.clone(), - needs_wildcard: needs_in.contains(&r.varname), + needs_wildcard: needs.in_side.contains(&r.varname), }); } if r.io.produces() { meta.out_entries.push(EntryShape { varname: r.varname.clone(), - needs_wildcard: needs_out.contains(&r.varname), + needs_wildcard: needs.out_side.contains(&r.varname), }); } } // Give the action an initials record iff it has Output objects. - let output_names: Vec = meta + let outputs: Vec = meta .object_refs .iter() .filter(|r| r.io == ObjectIO::Output) - .map(|r| r.varname.clone()) + .map(|r| EntryShape { + varname: r.varname.clone(), + needs_wildcard: needs.initials.contains(&r.varname), + }) .collect(); - meta.initials_entries = (!output_names.is_empty()).then_some(output_names); + meta.initials_entries = (!outputs.is_empty()).then_some(outputs); Ok(meta) } } @@ -2110,16 +2127,24 @@ pub(crate) fn body_referenced_vars(insts: &[Inst]) -> HashSet { referenced } -/// An Object's in/out form normally collapses into the io record and -/// needs no wildcard of its own. A sub-field body reference (`var.key`) -/// defeats that, keeping the form as an explicit wildcard instead: +/// Which of an Object's rendered forms a body field reference forces +/// open as an explicit wildcard. +#[derive(Default)] +struct WildcardNeeds { + in_side: HashSet, + out_side: HashSet, + initials: HashSet, +} + +/// An Object's forms normally collapse into a record entry and need no +/// wildcard of their own. A sub-field body reference (`var.key`) defeats +/// that, keeping the form as an explicit wildcard instead: /// double-anchoring a record entry isn't supported, so the dict must /// stay a wildcard for `.` to render. This walks the body and -/// returns the Objects forced open on each side: `needs_in` and -/// `needs_out`. Whole-dict refs never force a wildcard; a collapsed -/// dict arg is lifted to its record entry at execution via -/// ReplaceValueWithEntry. -fn compute_wildcard_needs(ctx: &ActionContext) -> (HashSet, HashSet) { +/// returns the Objects forced open per form. Whole-dict refs never force +/// a wildcard; a collapsed dict arg is lifted to its record entry at +/// execution via ReplaceValueWithEntry. +fn compute_wildcard_needs(ctx: &ActionContext) -> WildcardNeeds { let mut object_io: HashMap = HashMap::new(); for inst in &ctx.insts { if let Inst::Object { io, obj, .. } = inst { @@ -2128,13 +2153,9 @@ fn compute_wildcard_needs(ctx: &ActionContext) -> (HashSet, HashSet = object_io.keys().map(|v| (v.clone(), 0)).collect(); let max_ts = ctx.max_ts_per_var(); - let mut needs_in: HashSet = HashSet::new(); - let mut needs_out: HashSet = HashSet::new(); + let mut needs = WildcardNeeds::default(); - let check = |arg: &Ref, - cur: &HashMap, - needs_in: &mut HashSet, - needs_out: &mut HashSet| { + let check = |arg: &Ref, cur: &HashMap, needs: &mut WildcardNeeds| { let arg = arg.borrow(); let VarOrValue::Var(var) = &*arg else { return; @@ -2149,11 +2170,15 @@ fn compute_wildcard_needs(ctx: &ActionContext) -> (HashSet, HashSet (HashSet, HashSet {} Inst::Update { obj, value, .. } => { - check(value, ¤t_ts, &mut needs_in, &mut needs_out); + check(value, ¤t_ts, &mut needs); if let Some(ts) = current_ts.get_mut(obj) { *ts += 1; } } Inst::Set { kvs, .. } => { for (_k, v) in kvs { - check(v, ¤t_ts, &mut needs_in, &mut needs_out); + check(v, ¤t_ts, &mut needs); } } Inst::Statement { args, .. } | Inst::Intro { args, .. } => { for arg in args { - check(arg, ¤t_ts, &mut needs_in, &mut needs_out); + check(arg, ¤t_ts, &mut needs); } } } } - (needs_in, needs_out) + needs } /// Collected metadata that declares a Class diff --git a/libs/sdk/src/tests.rs b/libs/sdk/src/tests.rs index 080ff810..84abc3f8 100644 --- a/libs/sdk/src/tests.rs +++ b/libs/sdk/src/tests.rs @@ -1142,15 +1142,12 @@ fn test_statement_surface_round_trips() { action.st_dict_delete(crate_in, "k", crate_out); } - // Entry args on both sides come from inputs: a keyed read of an - // output's script-final form renders as `initials..`, - // a double anchor podlang has no syntax for. fn SetTransitions(action) { - var crate_a = action.input("Crate"); - var crate_b = action.input("Crate"); - action.st_set_insert(crate_a.tags, 1, crate_b.tags); - action.st_set_delete(crate_a.tags, 1, crate_b.tags); - action.st_array_update(crate_a.items, 0, 1, crate_b.items); + var crate_in = action.input("Crate"); + var crate_out = action.output("Crate"); + action.st_set_insert(crate_in.tags, 1, crate_out.tags); + action.st_set_delete(crate_in.tags, 1, crate_out.tags); + action.st_array_update(crate_in.items, 0, 1, crate_out.items); } "#; let sdk = Sdk::default(); @@ -1187,13 +1184,54 @@ fn test_statement_surface_round_trips() { "ArrayContains(crate_in.items, 0, 1)", r#"ContainerDelete(io.in_crate_in, "k", initials.crate_out)"#, r#"DictInsert(io.in_crate_in, "k", 1, initials.crate_out)"#, - "SetInsert(crate_a.tags, 1, crate_b.tags)", - "SetDelete(crate_a.tags, 1, crate_b.tags)", - "ArrayUpdate(crate_a.items, 0, 1, crate_b.items)", + "SetInsert(crate_in.tags, 1, crate_out0.tags)", + "SetDelete(crate_in.tags, 1, crate_out0.tags)", + "ArrayUpdate(crate_in.items, 0, 1, crate_out0.items)", ], ); } +/// Reading a field of an output built in the same action. That form is +/// normally the anchored `initials.` TxInsert consumes, which cannot +/// also carry a field access, so the field read has to force it open as a +/// wildcard pinned to the initials record. +#[allow(clippy::cloned_ref_to_slice_refs)] +#[test] +fn test_read_field_of_own_output() { + let _ = env_logger::builder().is_test(true).try_init(); + let craft_src = r#" + fn CraftPick(action) { + var pick = action.output("Pick"); + pick.set([["durability", 100]]); + action.st_gt(pick.durability, 0); + } +"#; + let sdk = Sdk::default(); + let module = sdk + .load_module_from_src_actions(craft_src, &["CraftPick"]) + .unwrap(); + assert_renders( + &module, + &[ + "ArrayContains(initials, CraftPickInitials::pick, pick0)", + r#"DictContains(pick0, "durability", 100)"#, + "Gt(pick0.durability, 0)", + "tx::TxInsert(chain0, chain, pick0, io.out_pick, @self_predicate(IsPick))", + ], + ); + + let mut state = TestState::default(); + let executor = module.executor(true, grounding_witness(&state, &[])); + let res = executor.action("CraftPick", vec![]).unwrap(); + let pick_tx = res.tx.clone(); + let [pick] = res.objs(); + apply_tx(&mut state, &pick_tx); + assert_eq!( + pick.obj.get(&StrKey::from("durability")).unwrap().unwrap(), + Value::from(100) + ); +} + /// `set` writes into the object's dict in place without advancing its /// ts, so it is only sound on an output nothing has pinned yet. Repeated /// sets stay consistent (see `test_cross_read_into_set`): containment