note: close out the model — correct an inert mutation claim, retag four non-discriminating assertions, assert Tempo's closure, fix three doc/test accuracy gaps
No behavior change; verification-record corrections and one static_assert.
This commit is contained in:
@@ -31,14 +31,16 @@ diverge: the capture-signal popup that edits it and the bake that renders it.
|
||||
the ladder ever gained a rung or a modifier.
|
||||
- **An offset stores the denomination it was entered in** — see `OffsetAmount` in
|
||||
`note_program.h` for why.
|
||||
- **Every value type establishes its domain at construction, and nothing downstream can
|
||||
fail.** `Tempo::fromBpm` rejects, alone, because an unusable BPM has no nearest usable one
|
||||
to fall to. `Division`, `OffsetAmount`, and `Velocity` clamp, because an off-ladder rung,
|
||||
an unrepresentable magnitude, and an out-of-range velocity each do. Each has exactly one
|
||||
door (`makeDivision`, `offsetOf`, `Velocity::of`) and a private constructor behind it, so
|
||||
an out-of-domain value cannot be held, only passed in. That is what lets every reader
|
||||
branch without a fallback, equality compare fields raw, and `resolveNote` return finite
|
||||
times for every constructible input with no failure path and no validity flag.
|
||||
- **Every value type establishes its domain at construction, so every field `resolveNote`
|
||||
returns is finite for every constructible program and tempo.** `Tempo::fromBpm` rejects,
|
||||
alone, because an unusable BPM has no nearest usable one to fall to. `Division`,
|
||||
`OffsetAmount`, and `Velocity` clamp, because an off-ladder rung, an unrepresentable
|
||||
magnitude, and an out-of-range velocity each do. Each has exactly one door (`makeDivision`,
|
||||
`offsetOf`, `Velocity::of`); `Division` and `OffsetAmount` block any other path with a
|
||||
private value constructor, `Velocity` with a private member that only `of()` writes —
|
||||
either way an out-of-domain value cannot be held, only passed in. That is what lets every
|
||||
reader branch without a fallback, equality compare fields raw, and `resolveNote` return
|
||||
finite times for every constructible input with no failure path and no validity flag.
|
||||
- **The module will not tell a caller a record is junk, because a junk record cannot exist
|
||||
here.** Corruption is only visible where raw bytes are: a codec sees both the bytes it
|
||||
read and the value construction produced, and reporting the difference is the codec's job.
|
||||
@@ -79,6 +81,9 @@ diverge: the capture-signal popup that edits it and the bake that renders it.
|
||||
Both are legal; `resolveNote` only refuses to invert the window.
|
||||
- **ms <-> beats round-trips are lossless to double precision, not bit-identical.** The
|
||||
conversion is a multiply/divide pair; compare with an epsilon.
|
||||
- **`Division` and `OffsetAmount` are trivially copyable, so a `memcpy` of a wire record
|
||||
bypasses every door.** Decode field-by-field through `makeDivision`/`offsetOf` (the pattern
|
||||
`src/core/wire/bytes.h` already uses) instead — never `memcpy` raw bytes into either type.
|
||||
- **Editing the ms field of a beats-stored offset stores beats, and the ms readout will then
|
||||
move with the tempo.** `withMsView` keeps the stored denomination on purpose, so typing 250
|
||||
into the ms field of a beats offset stores 0.5 beats at 120 BPM. That is the intended
|
||||
|
||||
@@ -64,8 +64,10 @@ double offsetSeconds(OffsetAmount amount, Tempo tempo) {
|
||||
}
|
||||
|
||||
OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) {
|
||||
// Route the requested target through the same door a stored denomination goes through,
|
||||
// so an out-of-enum target lands where a corrupt stored one does.
|
||||
// Defense-in-depth, not a discriminating guard: the branch below already treats any
|
||||
// non-Beats target as Milliseconds, so an unnamed `to` resolves the same way whether or
|
||||
// not it is routed through offsetOf first. Kept because a future third denomination
|
||||
// would make this the one place that still pins it.
|
||||
const Denomination target = offsetOf(0.0, to).denomination();
|
||||
if (amount.denomination() == target) return amount;
|
||||
return target == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo))
|
||||
|
||||
@@ -43,5 +43,7 @@ private:
|
||||
|
||||
static_assert(!std::is_default_constructible_v<Tempo>,
|
||||
"Tempo must not be constructible without a validated BPM");
|
||||
static_assert(!std::is_constructible_v<Tempo, double>,
|
||||
"fromBpm must be the only way to give a Tempo a value");
|
||||
|
||||
} // namespace reasampler::instrument::note
|
||||
|
||||
@@ -175,6 +175,9 @@ static void testUnnamedModifierClampsToStraight() {
|
||||
== divisionIndex(makeDivision(0, DivisionModifier::Straight)));
|
||||
CHECK(divisionLabel(makeDivision(0, junk)) == "1/4"); // and no junk reaches the readout
|
||||
CHECK(makeDivision(0, junk) == makeDivision(0, DivisionModifier::Straight));
|
||||
// Measured (clamp removed from clampModifier): still passes. junk(7) != Dotted's stored
|
||||
// modifier either way, clamped or raw — this discriminates a degenerate operator== that
|
||||
// ignores the modifier field, not the clamp itself.
|
||||
CHECK(makeDivision(0, junk) != makeDivision(0, DivisionModifier::Dotted));
|
||||
}
|
||||
|
||||
@@ -184,6 +187,8 @@ static void testEveryConstructibleDivisionIndexesIntoThePickerSet() {
|
||||
// modifier clamp.
|
||||
const int exponents[] = {-9000, -99, kMinQuarterExponent, 0, kMaxQuarterExponent, 120, 9000};
|
||||
for (int e : exponents) {
|
||||
// 260, not 256: m=256..259 wrap modulo uint8_t back to 0..3, re-covering the four
|
||||
// lowest bytes rather than reaching any byte 256 alone couldn't already reach.
|
||||
for (int m = 0; m < 260; ++m) {
|
||||
const Division d = makeDivision(e, static_cast<DivisionModifier>(m));
|
||||
const int index = divisionIndex(d);
|
||||
|
||||
@@ -356,14 +356,27 @@ static void testEveryDenominationBranchingFunctionAgreesWithThePin() {
|
||||
CHECK(corrupt == asMs); // offsetMs / offsetBeats / offsetSeconds covered above
|
||||
CHECK(!(corrupt != asMs));
|
||||
CHECK(redenominate(corrupt, Denomination::Milliseconds, t) == corrupt);
|
||||
// Measured (mutate offsetOf to a pass-through): still passes. offsetFromBeats always
|
||||
// tags its result Beats, so this holds regardless of whether corrupt was pinned — it
|
||||
// does not discriminate the pin.
|
||||
CHECK(redenominate(corrupt, Denomination::Beats, t).denomination() == Denomination::Beats);
|
||||
// Reported measured (withMsView reverted to its pre-domain-closure form): still passes.
|
||||
// corrupt already equals asMs by this point, and withMsView is a pure function of its
|
||||
// argument, so this line cannot discriminate anything withMsView-specific — it is a
|
||||
// restatement of the equality above.
|
||||
CHECK(withMsView(corrupt, 40.0, t) == withMsView(asMs, 40.0, t));
|
||||
CHECK(withMsView(corrupt, 40.0, t).denomination() == Denomination::Milliseconds);
|
||||
// Measured (mutate offsetOf to a pass-through): still passes. withBeatsView only branches
|
||||
// on `== Beats`; any non-Beats value — pinned or raw corrupt — takes the same ms-based
|
||||
// else branch, so this does not discriminate the pin either.
|
||||
CHECK(withBeatsView(corrupt, 1.0, t) == withBeatsView(asMs, 1.0, t));
|
||||
CHECK(withBeatsView(corrupt, 1.0, t).denomination() == Denomination::Milliseconds);
|
||||
CHECK(almostEqual(withBeatsView(corrupt, 1.0, t).magnitude(), 500.0, 1e-6));
|
||||
|
||||
// An unnamed TARGET denomination pins the same way an unnamed stored one does.
|
||||
// Reported measured (offsetOf(0.0, to) replaced with `= to;`): still passes, for the
|
||||
// same reason as above — `target == Beats` is false whether target is pinned or raw, so
|
||||
// this always takes the ms branch and cannot discriminate the door (see the comment on
|
||||
// that line in note_program.cpp).
|
||||
CHECK(redenominate(offsetFromBeats(1.0), static_cast<Denomination>(7), t)
|
||||
== offsetFromMs(500.0));
|
||||
}
|
||||
@@ -445,7 +458,10 @@ static void testResolveNoteIsFiniteForEveryConstructibleInput() {
|
||||
const double magnitudes[] = {-inf, -kMaxConvertibleMagnitude, -1e300, 0.0, 1e300,
|
||||
kMaxConvertibleMagnitude, inf, nan};
|
||||
int accepted = 0, rejected = 0;
|
||||
for (double bpm : {1e-320, 1e-306, 1e-200, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e308}) {
|
||||
// 1e-294/1e-295 bracket the accept/reject edge (measured ~3.34e-295) so the sweep
|
||||
// actually approaches it rather than jumping past it by ~95 orders of magnitude.
|
||||
for (double bpm :
|
||||
{1e-320, 1e-306, 1e-295, 1e-294, 1e-200, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e308}) {
|
||||
const std::optional<Tempo> tempo = Tempo::fromBpm(bpm);
|
||||
if (!tempo) { ++rejected; continue; }
|
||||
++accepted;
|
||||
|
||||
Reference in New Issue
Block a user