tool: the three findings the reference lock held (#521, #447, #545)

[gafferongames]

The three fix-wave findings PR #543 left for a ruling because the reference
lock froze the path each one needed. #545 lifted the lock, so they land here:
one commit per finding, each carrying its reproduction, its fix, its nine-way
gate and the negative control that turns that gate red. The pages land in the
same PR, ahead of the code they describe.

bench/LOCK holds no prefixes and its standing gate applies. This PR moves the
C and C++ packet emitters, so the reproduction rows are re-pinned below from a
stated sitting.

finding	verdict	commits
G-05 / F-09: a [A..B] count born at 0, and a release build writing bytes no reader takes	tool fixed, all nine	3eab5c1, 4d30733, f146f95
F-15 / G-08: union tag enums carry no Count anywhere, and a name function only in C	tool fixed, all nine	fb66780, e9efc13
F-12: the else-side guard comment reads if X else	tool fixed	f3d87f8
the pages, which stated the opposite of what the tool now does	SPEC, USAGE, TUTORIAL	be935ae
the coverage anchor, the dead branch, the comments		e11326a

db9c8da follows the three: test/dart, test/java and test/rust pinned
the count's OLD contract, and the fix turns them red where it should. The rust
leg asserted that a freshly constructed ProbeSample is WIRE-ILLEGAL,
samples_count born 0 against [1, 8], which is the defect written down as a
test. It now checks the two halves that replace it.

The dart and java legs are a different case, and the earlier framing of them
here was wrong. They did not pin a defect. They pinned SPEC §5's stated
checked-twin contract exactly as §5 stated it: an out-of-range count trips the
writer's assert, live under --enable-asserts and -ea and gone without
them. Those legs were correct against the page. What this PR changes is the
page. §5 now carries a stated exception, the count, refused in every build in
all nine targets, and the legs move to match the new rule. Each now checks the
refusal directly, which also holds without --enable-asserts and without
-ea, where the old claim was skipped outright. test/elixir's raise is
unchanged: the BEAM has no compile-out assert, so its count contract was
already on in every build.

The merge commit is origin/main at af66617b merged in, not a rebase.

No wire golden moves and no protocol id changes. A birth value is not a
specified default, so the SPEC §3.1 projection never sees it. The tag enum's
Count and its name function are not in the projection either, and the guard
is a comment. testdata/golden/id.txt is untouched.

THE PAGES, for the cold read

Three files, 160 insertions and 29 deletions, no code in them. Read the page
diff on its own with git diff af66617b..HEAD -- docs/. What each section
says, section by section:

SPEC §4.2, zero initialization. The rule's opening list said "zeroed
buffers and counts" without qualification. It now says "zeroed counts outside
the one stated exception below", and the exception follows the array sentence:
a [A..B]T count with A above zero is BORN AT A, in every target, because zero
is outside that count's own wire range and an array takes no specified default
to name another birth value, so the bound itself is it. [..N]T is [0..N]T,
reaches zero, and is born at 0 under the ordinary rule. The elements are zeroed
either way, and the §5 zero form does not carry the birth value for the same
reason it does not carry a specified default.

SPEC §4.2, the exported extent. The declared-enum Count paragraph ended
"the generated <Union>Type tag enums carry Max alone", which is now false
for a packet union. It says a packet union's tag enum carries Count beside
Max in the same nine spellings, so a tag enum and a declared enum present one
surface to a reader, and that the member is there because the surface is
uniform rather than because a tag set's two numbers ever differ. A
table-closure union's tag shape carries Max alone today, linked to #601.

SPEC §4.6, shape checks. The excludes-zero bullet said "an array field's
range must reach zero" flatly. It now says ELEMENT range, and two indented
paragraphs follow it. The first states the count bound as the legal exception,
because the count is born at A and so is never outside its own range at rest.
The second is the rule the generated code cites by name: a count outside
[A, B] is refused by the WRITE in every build, in all nine targets, and the
paragraph says out loud that it is not a compile error but the other half of
the same rule, stated here because every emitted refusal carries the comment
"a count outside its wire range is refused in every build (SPEC §4.6)". The
mechanism is spelled (the count guards the element loop, the pack subtracts
the low bound, a count below A wraps), and the nine conventions are listed: C
0, C++ and C# false, Dart and Java -1, Go serialize.ErrValueOutOfRange,
Rust Err(serialize::Error::ValueOutOfRange), Elixir ArgumentError, and
JavaScript false from the stream writer with -1 from BOTH tiers of the flat
writer, production included.

SPEC §4.8, unions. Three changes. The reserved-names bullet adds Count,
refused over the exported spelling on a PACKET union, with the scoping said
plainly: the reservation reaches where the member exists, so the name stays
free on a table-closure union until #601. The tag-enum bullet now lists Count
(the declared variant count, None excluded) beside Max, and says a tag enum
carries the same four parts a declared enum carries. A new bullet states the
debug-name function: any wire value including out-of-set, returning the
variant's spelling or a fixed unknown marker, for logs and tooling only, on no
read or write path, called by no generated code, with a nine-row table giving
each target's count member and name spelling once. A second new bullet states
the table-closure split as present state and links #601 as the named follow-on.

SPEC §4.11, refused by name. A new bullet for Count as an arm name on a
packet union: the tag enum claims the member, so the arm would define it twice,
which C++ and C# refuse as a redefinition. Refused over the exported spelling,
so count goes too, diagnostic naming the tag enum. The table-closure name
stays legal, linked.

SPEC §5, trust model. The tier paragraph is untouched. A new paragraph
after it states the one exception: a count outside [A, B] is refused by the
write in every build in all nine, so C++, Dart and Java refuse from the write
rather than through the assert their other contracts ride, and JavaScript's
flat production tier refuses rather than trusting the caller. It says why (the
count is not a diagnostic, it guards the loop and the pack wraps below A) and
closes by saying every other write-side contract in the section keeps the tiers
above.

SPEC §6.1. Two one-line rows. The [Min..N]T storage row now reads "count
born at Min and validated to [Min, N]". The per-declaration union item now
lists the tag enum as None = 0, the variants, then Count and Max, with
its debug-name function.

USAGE. The Max paragraph gains a sentence saying the tag enum carries the
debug-name function too, in the same nine spellings a declared enum uses. The
Count paragraph gains the arm-name reservation on a packet union. The union
section's tag enum line and its C++ code block gain Count = 4. The Arrays
section, which teaches counts, gains two paragraphs: ranged_count born at 2
rather than 0 and why, and the every-build write refusal with its mechanism.

TUTORIAL part 4, the counted-array hazard. This is the section that taught
the reader to work around the defect, so it is the largest single change. The
storage block shows window_count = 2, the write block shows the range
refusal instead of serialize_assert, and the prose is rewritten as two
paragraphs, one per end of the rule: born at the declared minimum and why the
bound has to name it, then the every-build refusal with the wrap spelled out
under the write_bits line the reader can see. The sentence telling the reader
to "reach for [A..B] with A above zero only where your code sets the count
before the value is ever written" is deleted. It now says to reach for it
wherever the count genuinely has a floor, because the tool holds both ends.

TUTORIAL part 5, the union. The WeaponFireType block gains Count = 2.
The paragraph that read "The tag enum is not a full enum. It carries None and
Max, and it does not carry Count or a name function, so
EnumName( cmd.fire.type ) does not compile" now says the opposite, which is
what the tool does: it is a full enum, EnumName( cmd.fire.type ) compiles and
prints "Laser", the switch further down is there to reach the payload rather
than to name the tag, and count is a refused arm name for the same reason
none and max are.

G-05 / F-09: the count's birth, and its refusal

SPEC §4.6 refuses a scalar or element range that excludes zero, because zero
initialization is the rule and such a field would be born outside its own
range. A [A..B] count bound with A above zero fell between the rules: born at
0, outside its own wire range, and the writer's check was an assert in five of
the nine targets.

ir.Field.BornCount names the birth value: the declared minimum, the one
wire-legal count a fresh value can carry, since an array takes no specified
default to name another one. Every target's CONSTRUCTED form carries it the way
it carries a specified default. The §5 zero form does not, for the same reason
a specified default does not: it is the all-zero form by rule.

The write side refuses rather than asserts. C, C++, Dart, Java and JavaScript
held the count with an assert or a checked-build predicate, gone under
-DNDEBUG, without -ea, without --enable-asserts, and in the production
JavaScript writer. The count guards the element loop and the pack subtracts the
low bound, so a count below the minimum wraps and the write reports success on
bytes no reader accepts. Go, Rust and C# already refused unconditionally
through their error-returning runtimes, and Elixir through an always-on raise.

The reproduction, at the tip, under -DNDEBUG

#521 recorded this, compiled with -DNDEBUG:

window_count at rest = 0, declared wire range [2, 8]
write returned true
read returned false, window_count = 8

The same program at this branch's tip, same flags:

$ c++ -std=c++17 -DNDEBUG -I ../serialize -I g -o t t.cpp && ./t
window_count at rest = 2, declared wire range [2, 8]
write returned true
read returned true, window_count = 2
write of a count of 0 returned false

The corpus reproduction

PR #543 named generated/cpp/Wire.h:172 as the finding live in this
repository's own shipped corpus. Before:

generated/cpp/Wire.h:172:    int32_t samples_count = 0;      // declared wire range [1, 8]
generated/cpp/WireWire.h:249:    serialize_assert( int32_t( value.samples_count ) >= int32_t( 1 ) && ... );

After:

generated/cpp/Wire.h:172:    int32_t samples_count = 1;
generated/cpp/WireWire.h:249:    if ( int32_t( value.samples_count ) < int32_t( 1 ) || int32_t( value.samples_count ) > int32_t( 8 ) )
generated/cpp/WireWire.h:251:        return false; // a count outside its wire range is refused in every build (SPEC §4.6)

F-15 / G-08: the tag enum's Count and its debug name

A generated <Union>Type looks and acts like a declared enum, so a reader
logging which message arrived writes the enum's own name call against it. It
carried no Count in any target (#489 gave declared enums one and did not
reach tag enums) and a debug-name function in exactly one: C's
enum_name_weapon_fire_type. Each target now emits both, in its own spelling:

target	count	debug name
c	#define WEAPON_FIRE_TYPE_COUNT 2	enum_name_weapon_fire_type (already had it)
cpp	Count = 2,	EnumName( WeaponFireType ), overloaded
cs	Count = 2,	EnumNameWeaponFireType(ulong)
dart	static const int count = 2;	enumNameWeaponFireType(int)
elixir	def count, do: 2	enum_name_weapon_fire_type/1
go	WeaponFireTypeCount	EnumNameWeaponFireType(uint64)
java	public static final byte count = 2;	enumNameWeaponFireType(long)
js	Count: 2,	EnumNameWeaponFireType(value)
rust	pub const COUNT	enum_name_weapon_fire_type

Adding Count reserves the name: an arm whose exported spelling is Count
would define the member twice, which C++ and C# refuse as a redefinition, so
e9efc13 refuses it at the source the way Max has been refused since the tag
enum carried an extent. The reservation is scoped to where the member exists: a
TABLE-CLOSURE union's tag shape is emitted beside the tables and carries Max
alone, so the name stays free there, and tables/messages/Messages.schema's
EditBody uses it. Both sides are gated. Giving the table-closure tag shape
its own Count and extending the reservation to reach it is
#601, filed here and
linked from SPEC §4.2, §4.8 and §4.11.

F-12: the guard comment

The comment spelled the branch if on_radar, nesting with / and negating
with a trailing else, so the else side read if on_radar else. The
reflection descriptors always carried the right spelling, built by each table
backend's guardWalk. The comment now spells it the same way:

    // !at_rest — wire branch; storage holds both sides, a read zeroes the
    // !at_rest && active — wire branch; ...
    // !at_rest && active && has_target — wire branch; ...
    // !at_rest && active && !has_target — wire branch; ...
    // at_rest — wire branch; ...

Field.Guard reaches comments only, in every backend, so nothing on a wire
moves.

The coverage anchor, quoted red then green

e11326a fixes a gate that could not fail.
TestTagEnumDebugNameCoversNoneVariantsAndOutOfSet searched the WHOLE
generated output for None, Laser, Missile and "???". All four appear
elsewhere in the same file: Laser and Missile name the arm types, None is
the tag enum's own member, and "???" is the DECLARED enum's name function
default. So the gate passed with the feature it was gating deleted.

Deleting the C++ tag enum's name function from internal/codegen/cpp/cpp.go,
with the test as it stood:

=== RUN   TestTagEnumDebugNameCoversNoneVariantsAndOutOfSet
--- PASS: TestTagEnumDebugNameCoversNoneVariantsAndOutOfSet (0.00s)

The gate now slices the function's body: from the opening line each target's
claim already carries, to the out-of-set arm, "???" being the last name every
target returns. The same deletion, with the anchored test:

--- FAIL: TestTagEnumDebugNameCoversNoneVariantsAndOutOfSet (0.00s)
    tagenumsurface_test.go:217: cpp: the tag enum has no name function, "inline const char * EnumName( WeaponFireType value )" not emitted

And restored:

--- PASS: TestUnionTagEnumExportsCountAndItsDebugName (0.00s)
--- PASS: TestAPacketUnionArmNamedCountIsRefusedByName (0.00s)
--- PASS: TestATableClosureUnionArmNamedCountIsAccepted (0.00s)
--- PASS: TestTagEnumDebugNameCoversNoneVariantsAndOutOfSet (0.00s)

The nine spellings moved into a named struct, so the count, the extent and the
name function's opening line live in one map and both gates read the same
source.

The same commit deletes emitWriteCount's bits == 0 branch in
internal/codegen/cpp/functions.go. §4.6 refuses [Min..N]T with Min at or
above N, so a count range is never degenerate and the zero-bit path a scalar
range needs had no case to serve. Every generated/ tree regenerates
byte-identical across the delete, which is the proof it was unreachable.

The negative controls, quoted red

(1) the birth value. ir.Field.BornCount returns 0 for a counted array:

--- FAIL: TestCountedArrayIsBornAtItsDeclaredMinimum (0.00s)
    countedbirth_test.go:127: c: a [2..8] count is not born at 2, "value.window_count = 2;" is absent from the constructed form
    countedbirth_test.go:127: cpp: a [2..8] count is not born at 2, "int32_t window_count = 2;" is absent from the constructed form
    countedbirth_test.go:127: cs: a [2..8] count is not born at 2, "public int WindowCount = 2;" is absent from the constructed form
    countedbirth_test.go:127: dart: a [2..8] count is not born at 2, "int windowCount = 2;" is absent from the constructed form
    countedbirth_test.go:127: elixir: a [2..8] count is not born at 2, "defstruct window: List.duplicate(0, 2)" is absent from the constructed form
    countedbirth_test.go:127: go: a [2..8] count is not born at 2, "value.WindowCount = 2" is absent from the constructed form
    countedbirth_test.go:127: java: a [2..8] count is not born at 2, "public int windowCount = 2;" is absent from the constructed form
    countedbirth_test.go:127: js: a [2..8] count is not born at 2, "this.WindowCount = 2;" is absent from the constructed form
    countedbirth_test.go:127: rust: a [2..8] count is not born at 2, "value.window_count = 2;" is absent from the constructed form

(2) the release-build refusal. The C++ count write put back on
emitWriteRangedFold32, the assert form:

--- FAIL: TestCountOutsideItsWireRangeIsRefusedInEveryBuild (0.00s)
    countedbirth_test.go:146: cpp: the count's range refusal is absent, "if ( int32_t( value.window_count ) < int32_t( 2 ) || int32_t( value.window_count ) > int32_t( 8 ) )" not emitted
    countedbirth_test.go:146: cpp: the count's range refusal is absent, "return false;" not emitted
    countedbirth_test.go:157: cpp: the count is held by serialize_assert, which the build can remove: serialize_assert( int32_t( value.window_count ) >= int32_t( 2 ) && int32_t( value.window_count ) <= int32_t( 8 ) );

The two claims are separate tests, so each control turns exactly its own red.

(3) the tag enum's Count. The C++ tag enum's Count line removed:

--- FAIL: TestUnionTagEnumExportsCountAndItsDebugName (0.01s)
    tagenumsurface_test.go:116: cpp: the tag enum's surface is short, "    Count = 2, // the declared variant count (SPEC §4.2)" not emitted

The gate's probe unit declares an enum of THREE variants beside a union of two,
found while running this control: with both at two, the declared enum's own
Count line satisfied the tag enum's claim and the control stayed green.

(4) the guard spelling. The old if X / if X else composition put back
in internal/check:

--- FAIL: TestGuardCommentsSpellTheConditionTheWayDescriptorsDo (0.01s)
    guardspelling_test.go:83: cpp: the stale guard spelling survives: "if !at_rest"
    guardspelling_test.go:83: cpp: the stale guard spelling survives: " else —"
    guardspelling_test.go:83: cpp: the stale guard spelling survives: " / if "
    guardspelling_test.go:91: cpp: no branch storage comment spells the guard "!at_rest && active"
    guardspelling_test.go:91: cpp: no branch storage comment spells the guard "!at_rest && active && has_target"
    guardspelling_test.go:91: cpp: no branch storage comment spells the guard "!at_rest && active && !has_target"

...and the same eight lines for cs, dart, elixir, go, java, js and rust.

(5) the debug-name coverage anchor. Quoted above.

The reproduction rows, and the owner law

bench/LOCK's standing gate: this PR moves the C and C++ packet emitters, so
the reproduction rows are re-pinned here with the sitting stated.

The sitting. A LAPTOP SITTING, not a certification run: MacBook Air, Apple
M2, 8 cores, macOS 26.6.2, Apple clang 21.0.0, Release
(-O3 -DNDEBUG -DSERIALIZE_RELEASE, -ffp-contract=off), no pinning (macOS
has no taskset), serialize at 8b765b7, corpus_id 6b213fbfa1a03a99 on every
row, seven runs per row, pinning: none and noise: unlabelled in every CSV
header. The machine was otherwise idle apart from this session. THREE
interleaved rounds per side for C++ and TWO for C, before and after
alternating, bench/run.sh --only cpp and --only c. Best (max) rate per the
standard's §2.2 headline statistic, and the per-round values below because the
headline alone reads wrong here. The whole sitting spans five minutes, 05:48 to
05:53 UTC, and the alternation did not fully cancel a machine drift inside it,
which the per-round section takes seriously rather than around. A quiet-box
sitting on the space server precedes the next release, with the owner's
per-run blessing.

before is origin/main at b5960d2b and after is db9c8da1, which are
the commits the runner stamped into every CSV. The branch has since merged
origin/main at af66617b, and the page and test commits that follow touch no
emitter, so the rows stand for the emitter change they were taken for.

leg	row	before (best)	after (best)	delta
cpp	bench_mixed write	7,543,961 msg/s	7,781,125 msg/s	+3.1%
cpp	bench_mixed round_trip	3,630,039 msg/s	3,693,003 msg/s	+1.7%
cpp	bitpacker write	60,643	60,691	+0.1%
cpp	bitpacker read	93,574	93,483	-0.1%
c	bench_mixed write	7,345,285 msg/s	7,301,575 msg/s	-0.6%
c	bench_mixed round_trip	3,655,846 msg/s	3,641,366 msg/s	-0.4%
c	bitpacker write	60,734	60,569	-0.3%
c	bitpacker read	57,483	57,463	-0.03%

What the numbers show, per round

The earlier reading here took the bitpacker's best-to-best movement (0.1% in
C++, 0.3% in C) as a noise band and dropped every bench_mixed delta inside
twice it. That is the wrong use of the control. bitpacker's source is
byte-identical across this diff, so a best-to-best comparison of it measures
what translation-unit layout did to an unchanged loop. It is not a
run-to-run variance estimate, and it is not a band any other row's delta can
be checked against.

The PER-ROUND values are, and they say something the summary rows hide. Best
rate per round, in the order the rounds ran, with the runner's own timestamp:

leg	row	05:48	05:49	05:50	05:52	05:53
cpp	bench_mixed write	after 7,689,113	after 7,781,125 / before 7,543,961	before 7,522,051	before 7,358,554	after 7,589,283
cpp	bench_mixed round_trip	after 3,693,003	after 3,685,886 / before 3,628,121	before 3,630,039	before 3,550,906	after 3,608,228
cpp	bitpacker write	after 60,691	after 59,075 / before 60,643	before 60,618	before 59,328	after 59,322
cpp	bitpacker read	after 93,483	after 91,420 / before 93,574	before 93,539	before 91,334	after 91,321

leg	row	05:50:53 before	05:51:26 after	05:52:10 after	05:52:32 before
c	bench_mixed write	7,345,285	7,297,060	7,301,575	7,173,524
c	bench_mixed round_trip	3,655,846	3,641,366	3,558,336	3,570,115
c	bitpacker write	60,734	60,569	59,177	59,133
c	bitpacker read	57,483	57,463	56,099	56,091

The machine drifted about 2.5% downward across the sitting, and the
bitpacker rows are what show it. On a byte-identical loop, C++ bitpacker
write reads 60,643 at 05:49 and 59,328 at 05:52 on the SAME binary, and
bitpacker read goes 93,574 to 91,334 over the same minutes. The C side does
the same thing: bitpacker write 60,734 at 05:50:53 down to 59,133 at 05:52:32.
That drift is larger than any delta in the summary table, it hits both sides,
and it is why a best-of-all-rounds comparison is not by itself a reading.
Comparing rounds inside the same minutes of the sitting is.

The C++ rows are faster after, both ways. Time-matched, every after round
beats every before round on both bench_mixed rows. In the early window,
after writes 7,689,113 and 7,781,125 against before's 7,543,961 and 7,522,051.
In the late window, after writes 7,589,283 against before's 7,358,554.
Round_trip does the same in both windows. The raw sets separate outright on
write (the slowest after round beats the fastest before round) and overlap on
round_trip only because the 2.5% drift is bigger than the effect. This moves
the WRONG WAY for a cost, since the diff adds two range refusals to
WriteBenchMixed. Layout is the likely cause, which is the same quantity the
bitpacker row measures, and it is not a win this PR claims.

The C write row, at -0.6% best to best, is not proven either way. Two
rounds a side, and they interleave rather than pair: before 7,345,285,
after 7,297,060, after 7,301,575, before 7,173,524, in that order. The
bitpacker beside them fell 2.6% over the same 100 seconds on an unchanged
loop, which is four times the delta being read. The two after rounds sit
exactly where that drift puts them, between the two before rounds. One of
those after rounds also carried the noisiest spread in the sitting, 8.5%,
with a low of 6,678,666. Two samples a side, unpinned, on a laptop, inside a
drift larger than the effect: this sitting cannot separate a real 0.6% write
cost from the machine, and nothing here claims the C write path is unchanged.
It is the row where the diff could plausibly cost something, since it is the
row that gained work, so it is the row that gets settled at the quiet-box
sitting on the space server before release, with the owner's per-run
blessing, rather than argued from a laptop.

The C round_trip row is the same case and gets the same treatment: before
3,655,846, after 3,641,366, after 3,558,336, before 3,570,115, which is drift
with the sides shuffled into it.

What the emitters' change reaches in the bench corpus, and what it does not

The whole diff under generated/bench/{c,cpp} is five files and 39 added
lines. By class, and there is no sixth:

1. MixedEventType gains Count = 3 and, in C++, an EnumName overload. C
   gains #define MIXED_EVENT_TYPE_COUNT 3 beside the
   enum_name_mixed_event_type it already had. Neither is called by any
   generated code: a constant and a function with no caller, off the read
   path and off the write path.
2. entities_count is born at 1, its declared minimum of [1, 8], a member
   initializer in C++ and one line in new_bench_mixed() in C. Construction,
   not a path.
3. Two write-path range refusals per WriteBenchMixed, on entities_count
   and stats_count, replacing serialize_assert calls that -DNDEBUG
   removed. This is real work the release build did not do before, and it
   is not a diagnostic: it is the fix. It is what the write rows measure, and
   it is the reason the C write row goes to the quiet box.
4. Guard comments in Bench.h and RealWorld.h change spelling. Comments.
5. Nothing else. The READ path is untouched in both legs, byte for byte.

No bench-corpus unit's bytes move. bytes_per_op is 438 in every row on
both sides, corpus_id is 6b213fbfa1a03a99 on both sides, and no wire golden
is in this diff. The runners prove it themselves before they will produce a
number: every pinned instance is byte-compared against its wire golden and
round-tripped (write, read, re-write, memcmp), and a runner whose bytes differ
from the corpus refuses to bench at all. Both legs printed OK on every run.

What #366's matrix says

Nothing changes. #366's rows are tables-layer features, the fixed and variable
classes, the text form, reflection, the block form, the cook, the build
version, and none of these three is one. All three land in all nine targets at
once, so no cell moves in either direction.