Skip to content

Commit e3a4f08

Browse files
timfennisclaude
andcommitted
docs(vm,analyser): drop external repo references and simplify comments πŸ“
Addresses review feedback on PR #146 from @timfennis: * scrub all mentions of an external AoC repo (`benches/programs/vec_hot_loop.ndc`, three comments in `ndc_vm/src/vm.rs`) β€” those references don't belong here * simplify the OpAssignment "both `op=` and `op`" comment in `ndc_analyser/src/analyser.rs` β€” drop the jargon, keep the rationale * tighten the `analyse_call` doc comment β€” say what it does, skip the side-table mechanics that the caller already documents No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 14379d5 commit e3a4f08

3 files changed

Lines changed: 15 additions & 26 deletions

File tree

β€Žbenches/programs/vec_hot_loop.ndcβ€Ž

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
// Vec dispatch hot loop. Mirrors the per-iteration vec calls that drove
2-
// the AoC 2025/08 regression in PR #141: a tight loop over millions of
3-
// `Tuple<Int, Int> + Tuple<Int, Int>` calls. The CallVec fast path
4-
// should bring this close to the master baseline.
1+
// Vec dispatch hot loop: a tight loop over many
2+
// `Tuple<Int, Int> + Tuple<Int, Int>` calls.
53
let n = 200_000;
64
let acc = (0, 0);
75
for i in 0..n {

β€Žndc_analyser/src/analyser.rsβ€Ž

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,8 @@ impl Analyser {
202202
let right_type = self.analyse_or_any(r_value);
203203
let arg_types = vec![left_type, right_type];
204204

205-
// Both `op=` and `op` desugar from operator syntax, so vec
206-
// dispatch is available for either path. Resolving `op` gives
207-
// us the result type to widen the lvalue with β€” that's how
208-
// `a += (3, 4)` on `Tuple<Int, Int>` widens correctly instead
209-
// of trying to widen with the scalar `+(Int,Int) -> Int`.
205+
// Resolve both `op=` and `op` so we can widen the lvalue
206+
// by the result type of whichever one actually fires.
210207
let ResolvedCall {
211208
binding: assign_binding,
212209
..
@@ -472,11 +469,8 @@ impl Analyser {
472469
}
473470
}
474471

475-
/// Analyse a call expression β€” either `Call` (regular) or `OperatorCall`.
476-
/// Resolves the function binding (with vec dispatch eligible iff
477-
/// `kind == Operator`) and returns the inferred result type. The binding
478-
/// is written back into the function-identifier node and the return type
479-
/// recorded on the call's [`NodeId`] via the surrounding `analyse` wrapper.
472+
/// Resolves a call (regular or operator-form) and returns its result type.
473+
/// Only operator-form calls are eligible for vec dispatch.
480474
fn analyse_call(
481475
&mut self,
482476
function: &mut ExpressionLocation,

β€Žndc_vm/src/vm.rsβ€Ž

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -818,10 +818,9 @@ impl Vm {
818818
}
819819

820820
/// Probes `OpCode::Call`'s callee for vec dispatch eligibility without
821-
/// allocating: returns `(vec_candidates_vars, axis_len)` borrowed off the
822-
/// stack when vec applies. `dispatch_vec_call_dynamic` resolves the
823-
/// candidates lazily β€” avoids the `Vec<Function>` allocation per call
824-
/// that the eager-resolve version cost on AoC-style hot loops.
821+
/// allocating: returns the `OverloadSet` Rc and broadcast axis length
822+
/// when vec applies. `dispatch_vec_call_dynamic` resolves the candidates
823+
/// lazily from there, avoiding a per-call `Vec<Function>` allocation.
825824
fn try_vec_dispatch(&self, args: usize) -> Option<(Rc<Object>, usize)> {
826825
let Value::Object(obj) = &self.stack[self.stack.len() - args - 1] else {
827826
return None;
@@ -882,11 +881,10 @@ impl Vm {
882881
})
883882
}
884883

885-
/// Vec dispatch when `vec_candidates` lives behind a shared `Object::OverloadSet`
886-
/// Rc β€” the runtime-narrowing path for `Binding::Dynamic` operator calls.
887-
/// Resolves the candidate vars to `Function`s lazily inside the broadcast
888-
/// loop with a last-match cache, so homogeneous tuples (the common case,
889-
/// including the `AoC` 2025/08 hot loop) pay one resolve per outer call
884+
/// Vec dispatch for `Binding::Dynamic` operator calls β€” the runtime
885+
/// picks the matching scalar overload per element position. Resolves
886+
/// candidates lazily from `vec_candidates` with a last-match cache, so
887+
/// homogeneous tuples (the common case) pay one resolve per outer call
890888
/// instead of N. No upfront `Vec<Function>` allocation.
891889
fn dispatch_vec_call_dynamic(
892890
&mut self,
@@ -999,9 +997,8 @@ impl Vm {
999997
let mut elem_args: Vec<Value> = Vec::with_capacity(args);
1000998
let mut results: Vec<Value> = Vec::with_capacity(axis_len);
1001999
// Cache the last scalar that matched. Homogeneous tuples β€” the common
1002-
// case at runtime, including the AoC 2025/08 hot loop β€” all want the
1003-
// same scalar at every position, so probing it first short-circuits
1004-
// the candidate walk for positions 1..N.
1000+
// case β€” all want the same scalar at every position, so probing it
1001+
// first short-circuits the candidate walk for positions 1..N.
10051002
let mut last_match: Option<usize> = None;
10061003

10071004
for i in 0..axis_len {

0 commit comments

Comments
Β (0)