Close the handle-order class: delete <=>, fix a search-while-appending

[typeless]

Three findings from an adversarial review of the enumeration-order PRs (#174, #176, #181, #186).

A search-while-appending precondition violation (major)

compose_nested_project_subtree (src/cli/context.cpp:463) binary-searched available inside a loop that push_backs onto it. After the first append the range is unsorted, so binary_search's precondition is violated — a directory already present can be reported absent and appended again, and the final std::sort had no unique to remove it.

A duplicate there means sort_dirs_by_depth parses that Tupfile twice in one run, registering its rules twice — which now dies at the duplicate-key rejection with an error naming a project that is perfectly valid. Which projects hit it is decided by interning-order accidents: exactly what this PR series set out to remove.

The search now covers only the sorted prefix, and the sort dedups, so the invariant no longer rests on the rel_ids within one call happening to be distinct.

The reviewer audited every other insert-while-searching site (cmd_build.cpp:1324, cmd_show.cpp:577, var_tracking.cpp:22, builder.cpp:1695) — all insert at the lower_bound position and preserve sortedness. This was the only one.

#186 left the class half-closed (minor, but it is the point of #186)

Deleting <, >, <=, >= on StringId forces every comparison to say whether it means handle order or name order. Two routes stayed open. Both compile against the real header today:

auto x = (a <=> b) < 0;                                  // compiles
struct Node { StringId name; int x;                      // compiles: member-wise
  auto operator<=>(Node const&) const = default; };      //   built-in enum <=>

The second is the dangerous one — a defaulted spaceship on any struct holding a StringId sorts by handle without a single character at the call site saying so. Deleting operator<=> rejects both; == is untouched. Verified by compiling probes before and after.

Neither route is used anywhere in src/, include/ or test/ today, so this closes the class before it grows a member rather than fixing a live bug.

Exported variables reached the child in handle order (minor)

prepare_job_launch emitted a job's exported variables in interning-handle order. create_env_block (process-win32.cpp:102) writes that sequence into the environment block verbatim, and Win32 expects it sorted alphabetically. Now emitted by name.

Not claimed: the review reported observing handle order in a child's env output. That evidence is confounded — commands run under sh, which re-exports its own environment in its own order, so the block order putup passes is not observable that way. I checked, and could not reproduce a name-order result through a shell after the fix. The Win32 block requirement stands on its own and is the reason this changed.

Also from the review, not acted on here

• #174/#176 comments still attribute interning order to readdir; that has been false since Return directory entries in name order (fixes #179) #181 made read_directory return sorted entries. The sorts remain necessary — interning order is still arbitrary — but the stated reason is stale. Left for a docs pass.
• cmd_show.cpp:602 ("Top instructions") and output.cpp:39 ("Removed empty dir") both tie-break in handle order in user-facing output. Cosmetic; not reproduced.
• PathId (include/pup/core/path_id.hpp) is the same kind of handle enum with all relational operators intact. No ordered use exists today, so the same deletion would be free — worth doing, but it is a separate change.

Verification

make test: 142392 assertions, 634 cases, 32 e2e shards. make tidy, make iwyu clean.

The reviewer also confirmed by mutation that the tests added by #174, #181 and #186 genuinely bite: reverting each production sort in turn makes the corresponding test fail, then restoring it makes them pass. And that POSIX and Win32 read_directory agree on ordering (both sort UTF-8 bytes after conversion), and that #174's glob order matches tup's, whose unique(dir, name) index has default BINARY collation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA

[github-actions]

PR metrics

Performance (gcc example, Linux)

Workload	Instructions	CPU time	Page faults	D1 miss	LL miss	Wall	Peak RSS
parse	1076 M (+0.1%)	0.51 s	14.2 k	0.8%	0.1%	0.512 s	31.2 MB
dry-run	1881 M (+0.1%)	0.61 s	16.6 k	0.9%	0.1%	0.622 s	40.2 MB (+0.1MB)

Deterministic signals: instructions (cachegrind-simulated instruction reads — exact across runs, no PMU needed), page faults, peak RSS, and the cachegrind D1/LL miss rates. CPU time is user+sys from time(1).

Internal statistics (gcc example, up-to-date dry run)

Metric	Value
Tupfiles parsed	24
Commands	3545
Commands scheduled	0
Files checked	5818
Files changed	0
Files in index	6087
Graph edges	367913
Index size (bytes)	7378282
Implicit deps	330624
Hash computations	0
Hashes skipped (stat cache)	5818
Stat calls	5869
Parse time (ms)	501.2 (+2.3%)
Total time (ms)	620.2 (+2.3%)
Runner CPU	AMD EPYC 9V74 80-Core Processor

Counters from putup -n --stat on the fully-built gcc example (up-to-date dry run): deterministic work measures — a jump in commands scheduled, hash computations, or stat calls is a real behavior change, not noise. Timings are the minimum over repeated runs, compared only against a baseline from the same CPU model; the counters are the regression signal.

Binary size (Linux)

Binary	.text	.data	.bss	File
putup	517.6 KB (+0.6%)	1.6 KB	98.7 KB	615.2 KB (+0.9%)

Test coverage (lines)

Overall	Median file	Min file	Max file
86.7%	95%	0.0% src/platform/path-posix.cpp	100.0% include/pup/core/arena.hpp

103 files · 15163/17494 lines covered

Deltas vs main@a47fe41f5.

Updated for e7788db