refactor(aura-core): split Scalar into a tag-free Cell + ScalarKind
Motivation
----------
`Scalar` was a tagged enum (I64/F64/Bool/Ts), so every scalar value
physically carried its own kind tag. But the kind is already known from
the schema/port/column the value flows through (C7: the type is a
property of the column, not of the value — the hot path is already
columnar `Column<T>`, and `AnyColumn::get` *reconstructs* the tag from
the column on the way out). The per-value tag was therefore redundant
with the kind the surrounding context already holds.
That redundancy had three costs:
* It baked an implicit `match` (a branch) into every function that read
a Scalar payload — even where the caller statically knew the type.
The tag could never be exploited away.
* Size: a tagged enum is tag + payload = 16 bytes (f64/i64 alignment),
twice the 8 bytes the value needs. A `Column<Scalar>` would be double
the memory and half the cache utilisation.
* It is the shared root of several downstream papercuts we keep hitting
— the lossy f64 manifest field, the `unreachable!` panic on a
non-numeric param, the serde-tag question — all symptoms of "the type
is baked into the value".
Change
------
Introduce `Cell`: a type-erased 64-bit word (`struct Cell(u64)`) that is
not readable without external type context. It is constructed per base
type (`from_i64/from_f64/from_bool/from_ts`) and read only by naming the
type at the call site (`i64()/f64()/bool()/ts()`) — each a branch-free
bit-cast. The hot path resolves the kind once at the boundary (from the
schema) and then reads natively, with no per-value branch. `Cell` knows
nothing of `Scalar` or `ScalarKind`; the dependency is strictly one-way,
and it lives in its own `cell.rs` (more is planned on top of it).
`Scalar` becomes `struct { kind: ScalarKind, cell: Cell }` — the
self-describing form for the dynamic boundaries (builder binding,
serialization, rendering), built on top of `Cell`. Its `as_*` accessors
now `debug_assert` the kind and return the native value (free in
release); calling the wrong accessor is a caller bug, not a checked
`Option`. The variant constructors `Scalar::I64(..)` become associated
fns `Scalar::i64(..)`.
`PartialEq` is hand-written (not derived) to preserve the former enum's
value semantics: kinds must match, then native payloads compare, so f64
keeps IEEE-754 behaviour (`NaN != NaN`, `+0.0 == -0.0`) and a kind
mismatch is never equal even when the raw words coincide. A fixture
(`scalar_eq_is_value_not_bitwise`) pins exactly the cases where bit- and
value-equality diverge, so it can't silently regress. `Cell`'s own
`Eq`/`Hash` stay bitwise — correct for a raw word.
The change is behaviour-preserving: Scalar's observable behaviour is
identical to the pre-Cell enum (the value-equality fixture proves it);
only the internal representation changed. The ~440 call sites across the
workspace are a mechanical constructor rename plus ~12 destructuring
sites (match-arms / `let`-patterns) rewritten to `kind()` + `as_*`.
Verified: cargo build --workspace --all-targets, cargo clippy --workspace
--all-targets -- -D warnings, cargo test --workspace — all green.
This commit is contained in:
@@ -435,45 +435,45 @@ mod tests {
|
||||
#[test]
|
||||
fn bind_removes_slot_and_reconstructs_positionally() {
|
||||
// chained bind of b then a (reverse slot order); c stays open
|
||||
let bound = probe3().bind("b", Scalar::I64(8)).bind("a", Scalar::I64(7));
|
||||
let bound = probe3().bind("b", Scalar::i64(8)).bind("a", Scalar::i64(7));
|
||||
assert_eq!(
|
||||
bound.params().iter().map(|p| p.name.as_str()).collect::<Vec<_>>(),
|
||||
["c"], // only c remains open
|
||||
);
|
||||
let built = bound.build(&[Scalar::F64(9.0)]); // inject c
|
||||
let built = bound.build(&[Scalar::f64(9.0)]); // inject c
|
||||
assert_eq!(
|
||||
built.label(),
|
||||
format!("{:?}", vec![Scalar::I64(7), Scalar::I64(8), Scalar::F64(9.0)]),
|
||||
format!("{:?}", vec![Scalar::i64(7), Scalar::i64(8), Scalar::f64(9.0)]),
|
||||
);
|
||||
|
||||
// partial: bind only b; a and c stay open and keep their positions
|
||||
let built2 = probe3()
|
||||
.bind("b", Scalar::I64(8))
|
||||
.build(&[Scalar::I64(7), Scalar::F64(9.0)]); // a, c injected in order
|
||||
.bind("b", Scalar::i64(8))
|
||||
.build(&[Scalar::i64(7), Scalar::f64(9.0)]); // a, c injected in order
|
||||
assert_eq!(
|
||||
built2.label(),
|
||||
format!("{:?}", vec![Scalar::I64(7), Scalar::I64(8), Scalar::F64(9.0)]),
|
||||
format!("{:?}", vec![Scalar::i64(7), Scalar::i64(8), Scalar::f64(9.0)]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bind_records_bound_param_at_original_position() {
|
||||
// middle-of-three: binding `b` records its ORIGINAL slot position (1).
|
||||
let mid = probe3().bind("b", Scalar::I64(8));
|
||||
let mid = probe3().bind("b", Scalar::i64(8));
|
||||
assert_eq!(
|
||||
mid.bound_params(),
|
||||
[BoundParam { pos: 1, name: "b".into(), kind: ScalarKind::I64, value: Scalar::I64(8) }]
|
||||
[BoundParam { pos: 1, name: "b".into(), kind: ScalarKind::I64, value: Scalar::i64(8) }]
|
||||
.as_slice(),
|
||||
);
|
||||
|
||||
// chained reverse-order bind (b then a): each entry carries its ORIGINAL
|
||||
// position regardless of the shrinking array — positions {1, 0} in call order.
|
||||
let pair = probe3().bind("b", Scalar::I64(8)).bind("a", Scalar::I64(7));
|
||||
let pair = probe3().bind("b", Scalar::i64(8)).bind("a", Scalar::i64(7));
|
||||
assert_eq!(
|
||||
pair.bound_params(),
|
||||
[
|
||||
BoundParam { pos: 1, name: "b".into(), kind: ScalarKind::I64, value: Scalar::I64(8) },
|
||||
BoundParam { pos: 0, name: "a".into(), kind: ScalarKind::I64, value: Scalar::I64(7) },
|
||||
BoundParam { pos: 1, name: "b".into(), kind: ScalarKind::I64, value: Scalar::i64(8) },
|
||||
BoundParam { pos: 0, name: "a".into(), kind: ScalarKind::I64, value: Scalar::i64(7) },
|
||||
]
|
||||
.as_slice(),
|
||||
);
|
||||
@@ -494,7 +494,7 @@ mod tests {
|
||||
},
|
||||
|_| Box::new(Bare),
|
||||
);
|
||||
let _ = b.bind("width", Scalar::I64(2)); // "width" is not a declared param
|
||||
let _ = b.bind("width", Scalar::i64(2)); // "width" is not a declared param
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -512,7 +512,7 @@ mod tests {
|
||||
},
|
||||
|_| Box::new(Bare),
|
||||
);
|
||||
let _ = b.bind("dup", Scalar::I64(1)); // two slots named "dup"
|
||||
let _ = b.bind("dup", Scalar::i64(1)); // two slots named "dup"
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -527,7 +527,7 @@ mod tests {
|
||||
},
|
||||
|_| Box::new(Bare),
|
||||
);
|
||||
let _ = b.bind("length", Scalar::F64(2.0)); // F64 value for an I64 slot
|
||||
let _ = b.bind("length", Scalar::f64(2.0)); // F64 value for an I64 slot
|
||||
}
|
||||
}
|
||||
|
||||
@@ -542,12 +542,12 @@ mod zip_params_tests {
|
||||
ParamSpec { name: "fast".into(), kind: ScalarKind::I64 },
|
||||
ParamSpec { name: "scale".into(), kind: ScalarKind::F64 },
|
||||
];
|
||||
let point = vec![Scalar::I64(2), Scalar::F64(0.5)];
|
||||
let point = vec![Scalar::i64(2), Scalar::f64(0.5)];
|
||||
assert_eq!(
|
||||
zip_params(&space, &point),
|
||||
vec![
|
||||
("fast".to_string(), Scalar::I64(2)),
|
||||
("scale".to_string(), Scalar::F64(0.5)),
|
||||
("fast".to_string(), Scalar::i64(2)),
|
||||
("scale".to_string(), Scalar::f64(0.5)),
|
||||
],
|
||||
);
|
||||
}
|
||||
@@ -558,7 +558,7 @@ mod zip_params_tests {
|
||||
ParamSpec { name: "a".into(), kind: ScalarKind::I64 },
|
||||
ParamSpec { name: "b".into(), kind: ScalarKind::I64 },
|
||||
];
|
||||
let point = vec![Scalar::I64(7), Scalar::I64(9)];
|
||||
let point = vec![Scalar::i64(7), Scalar::i64(9)];
|
||||
let hand: Vec<(String, Scalar)> =
|
||||
space.iter().zip(&point).map(|(ps, v)| (ps.name.clone(), *v)).collect();
|
||||
assert_eq!(zip_params(&space, &point), hand);
|
||||
|
||||
Reference in New Issue
Block a user