Adopt the failable-call standard, and queue its workspace audit

[MikeGrier]

Records a workspace-wide standard and queues the work it implies. Docs only -- no code changes.

The standard

A call that can fail has its failure handled. No per-site analysis, no exemption for calls that "cannot fail in practice."

It is deliberately flat, because the analysis it replaces is the thing that goes wrong. Judging per site whether a particular call is worth checking means the exempted calls are not the genuinely infallible ones -- they are the ones nobody looked at hard enough. "Trivial enough not to check" is a fact about the reader's attention, not a property of the call. The cost is a compare and an untaken branch, which is worth paying outright for a rule that needs no thought, and overwhelmingly worth it the first time it catches something.

The measurement behind it

From windows-platform-probes (#83): three timing loops discarded every BOOL, while the SubmitIoRing loop twelve lines below them carried a careful note explaining why discarding a status is precisely the hazard the crate exists to avoid. The argument was correct and had been applied only to the call that looked expensive enough to deserve it.

Run against a deliberately invalid handle so every event call fails:

timing	unchecked, every call failing	true
set_event_already_signalled	212.7	205
set_reset_event	422.9	531
wait_zero_signalled	231.5	280

None of those looks wrong -- the first is within 4%. A failing syscall still costs a measurable transition, so the report reads as evidence while describing operations that never happened. That is the core of it: a discarded status does not produce an obviously bad result, it produces a plausible one, which is exactly why it survives review.

Checks cost nothing detectable — 204-206, 528-534, 280.3-280.7 across three runs with them in place, which is run-to-run spread rather than a shift.

Queued work

• M22.1 — classify and fix the failable sites. Measured scope: 121 bare unsafe { Call(...) }; statements across 11 crates, but most are not violations — SubmitThreadpoolWork, SetThreadpoolWait, GetSystemInfo, SetLastError and the WaitForThreadpool*Callbacks family return void, and SetErrorMode returns the previous mode rather than a status. The failable set is roughly 28, dominated by CloseHandle at 24 sites. The item records that the regex matched single-line statements only, so 121 is a floor, not a total.
• M22.2 — the type-embedding half: a checked owning handle type, so the rule is discharged by construction rather than by 24 written-out checks. Flags two questions that decide whether the type is usable at all — what a failing close does in Drop given that panicking during unwind aborts, and whether teardown paths that legitimately expect a close to fail exist here (windows-threadpool-sys's caller-supplied close routines are where they would be).

Worth noting: std's OwnedHandle is not a discharge on its own. It closes on drop but discards CloseHandle's BOOL.

Why root, and why a separate PR

The standard is workspace-wide, so burying it in one crate's design notes would orphan it. And per the repo's own rule that design notes are not a work queue, the audit is queued as checklist items rather than left as prose nobody is obliged to act on.

Kept separate from #83 so that peel stays a windows-platform-probes change.

[Copilot]

🟢 Approval recommended

The changes are documentation-only and consistently record/queue the standard without introducing code or behavioral risk.

Pull request overview

Records a workspace-wide documentation standard that every failable call must have its failure handled, and queues a workspace audit + type-based remediation work to apply the standard to existing code.

Changes:

• Add a new decision section to DESIGN-NOTES.md defining the “failable call must be handled” rule, including motivating measurements and scope boundaries.
• Add a new M22 checklist section that queues (1) a workspace audit/fix pass and (2) a checked owning handle type to discharge most sites by construction.

File summaries

File	Description
DESIGN-NOTES.md	Adds the new “failable call” decision, rationale/measurement summary, and points to the queued checklist work.
CHECKLIST.md	Queues M22.1 (audit/fix) and M22.2 (checked owning handle type) to apply the new standard across the workspace.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The new checklist text contains an unverifiable/stale file reference and a POSIX-only RUSTFLAGS=... example that should be corrected for Windows/PowerShell usage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[MikeGrier]

Merged origin/main (PR #83 landed) and addressed the review in 4809f48. The branch is back to a clean two-file diff against main.

The column header

Taken. "true" didn't say what it was true of. Both columns are now named for the run they came from: reported when unchecked and every call fails against reported when the calls succeed.

Merging main invalidated most of the numbers, so I re-measured rather than left them

This is the part worth flagging, because the entry's whole value is that a reader can trust its figures. Every measurement in this PR describes code that PR #83 just changed:

claim	as written	at dc2b463
bare unsafe { Call(...) }; statements	121	119
...in windows-platform-probes	25	23
SetEvent discards	4	2
unused_results warnings on that crate	112	104

And one was an overstatement independent of the merge: "two thirds are ordinary Rust" is really 62 of 104, about sixty percent — with the remaining 42 naming a raw Win32 call, and not even all of those are violations, since several of those calls return void.

The failable set is now a table of six calls totalling 30 sites, each confirmed against its windows-sys signature rather than assumed. I checked the four I'd been carrying on inference: CancelIoEx, RevertToSelf, SetCurrentDirectoryW and FindCloseChangeNotification all return BOOL; CloseIoRing returns an HRESULT.

The correction I'd most want kept

The entry now says outright that every number in it is stale on arrival and the lint should be re-run instead. Those figures moved twice in one afternoon purely because an unrelated PR merged. Writing them as an inventory to tick off would guarantee the next reader works against a tree that has moved underneath them; writing them as a description of the work's shape, with the command to regenerate them, does not.

That is the same lesson the #83 flake record produced an hour ago — a document written to save someone re-deriving facts is only worth as much as its facts, and prose about code gets no automatic verification the way code does.

No conflicts in the merge; the branch remains docs-only.

[Copilot]

🟡 Changes recommended

The checklist’s suggested RUSTFLAGS spelling (-W unused_results) is likely incorrect for rustc’s command-line lint naming and could cause the documented audit procedure to fail when followed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[MikeGrier]

Declined as stated, but it pointed at something real, so 36ba6c3 records that instead.

The command works as written

Tested directly on 1.98.0 rather than reasoned about:

spelling	exit	warns	rejected
-W unused_results	0	yes	no
-W unused-results	0	yes	no

rustc normalises _ and - in lint names on the command line, so neither form is rejected. There is also standing evidence in this PR: every unused_results figure in the item — the 112, then the re-measured 104 — was produced by running exactly that RUSTFLAGS command, through cargo, on this branch. The instruction is verified by use, not just by argument.

But the premise behind the finding is real

Here is the actual output, and it explains where the intuition came from:

warning: unused result of type `i32`
 --> lintprobe.rs:2:13
  |
2 | fn main() { f(); }
  |             ^^^^
  |
  = note: requested on the command line with `-W unused-results`

rustc accepts the underscore form and then echoes the hyphenated form back. So a reader who follows the instruction and sees -W unused-results in their own output will reasonably wonder whether they mistyped it — which is presumably exactly what happened here.

That is a genuine trap in the instructions even though the instructions are correct, so I added a parenthetical saying both spellings work and that the echo reads like a correction but is not one. A finding that is wrong about the code but right about how the code will be read is still worth acting on; it just gets a different fix than the one suggested.

No change to the command itself.

[Copilot]

🟢 Approval recommended

The change is documentation-only and consistently records the standard plus an actionable checklist plan without introducing code or behavioral risk.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite