core/cmake: stop a serial dispatch rebuilding a std::deque it never fills, and say what -Wno-shadow-uncaptured-local really suppresses (fixes #660, refs #662)

[Yaraslaut]

Two tickets on disjoint trees. One closes, one does not — see the #662
section, which is the part worth reading first.

• core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660 — StrandExecutor rebuilds a model's Strand on every serial
  dispatch. Fixed: 2 fewer allocations and 608 fewer bytes per local call.
  fixes #660.
• -Wno-shadow-uncaptured-local suppresses a different set on each clang, so the WASM leg is the only one enforcing structured-binding shadowing #662 — -Wno-shadow-uncaptured-local is broader than its comment. The
  comment is corrected with the measurement; the issue stays open, because
  nothing here meets its own closing condition. refs #662.

Plus one labelled commit repointing a gate line hint that #660's diff moved.

---

#660 — commit 1

Re-measured first, as instructed

The 4 allocations / 752 bytes in the ticket came from a pre-#661 tree and #661
changed the dispatch path, so nothing was touched until the instrument #661
landed had been re-run. On 7a343e6f, x86-64 Linux, GCC 16.2.1 / libstdc++,
-O2, tests/bench/bench_dispatch_allocations.cpp:

local execute round-trips  : 200
heap allocations total     : 4159 (20.80 per call)
bytes allocated total      : 398072 (1994.3-1999.9 per call across runs)

Per-line attribution from a backtrace() added to the counting hook — the
method the bench's own header documents — resolved through dladdr (the binary
is PIE, so raw addresses give ??) and addr2line -f -i -C:

  #7    32 bytes  strand.hpp:104   _strands map node
  #8   152 bytes  strand.hpp:106   make_shared<Strand>
  #9    64 bytes  <stl_deque>      the Strand's std::queue deque map
  #10  512 bytes  <stl_deque>      the Strand's std::queue first buffer

760 of 1990 bytes, 38% — the ticket's figure reproduces, at 760 rather than
752 (the make_shared<Strand> block is 152 here, not 144).

What the re-measurement changed about the fix

576 of those 760 bytes are not the strand's lifetime at all. They are
libstdc++'s std::deque allocating a node map and a 512-byte first buffer
in its default constructor, for a queue that in this workload never holds more
than one task. That is morph#660's own direction 2, and it is the one that
needs no change to the lifetime rules.

So std::queue<std::function<void()>> becomes PendingQueue: the head task
lives inside the Strand, and a std::deque is constructed behind it only when
a second task is genuinely queued behind a running one. The locking is
untouched — same two scoped_locks, same order, same {_mapMtx, strand->mtx}
pair, erase still fires when empty() becomes true.

heap allocations total     : 3786 (18.93 per call)
bytes allocated total      : 279208 (1396.0 per call)

#7    32 bytes   _strands map node
#8   120 bytes   make_shared<Strand>

2 allocations and 608 bytes per local dispatch, 30% of the total.

Directions 1 and 3 are not taken and not smuggled in. Keeping the slot alive
would remove the last two allocations but trades this churn for a per-model
entry nothing reclaims — StrandExecutor has no deregistration hook, so the
erase is the only bound on the map. Filed as #670 with the leftover measured.

The safety claim, and how it was tested

The one claim this branch rests on: PendingQueue changes what the
Strand allocates and nothing about when it is created, erased, or locked —
so the {_mapMtx, strand->mtx} atomicity the comments at strand.hpp:81 and
:167 record survives untouched.

"The suite passes" is not evidence for that, so:

Full ctest under the clang-tsan preset — clang 22.1.8,
-DMORPH_BUILD_NET=ON -DMORPH_BUILD_OFFLINE_SQLITE=ON,
TSAN_OPTIONS=suppressions=cmake/tsan.supp, i.e. the Linux / clang-tsan leg's
own configuration. 1801/1801 pass. Not a blind run:
check_sanitizer_instrumentation.sh build/clang-tsan tsan reports
6 ctest binaries all carry __tsan_ symbols (0 allowlisted).

And that configuration was shown to be able to see this specific hazard.
Restoring the pre-fix two-step drain (flip running under strand->mtx, then
erase under _mapMtx) makes TSan report:

WARNING: ThreadSanitizer: data race (pid=490650)
  Write of size 4 at 0x72100001407c by thread T4:
    #0 LoadCountModel::execute(...) tests/test_concurrency_invariants.cpp:227:15
    ...
    #11 morph::exec::detail::StrandExecutor::scheduleNext(...)::'lambda'()::operator()() const strand.hpp:225:17
SUMMARY: ThreadSanitizer: data race ... in LoadCountModel::execute(...)

Two strand lambdas for one key running the model concurrently — exactly the
defect the combined lock exists to prevent. The fixed tree passes that same test
60/60.

Mutation-tested for coverage of the new code, because a container with an
untested overflow arm is a container that works by luck:

mutation	result
_overflow->push_back → push_front	3 cases fail, incl. the 50-task FIFO assertion (49 == 1)
refill-from-overflow arm disabled	8 of 12 [strand] cases fail

Full gcc-release suite: 1561 cases, 22971 assertions, all pass
(1 "failed as expected"). clang 22.1.8 -Weverything -Werror builds clean.
clang-tidy over a TU including strand.hpp, using the project's own Clang flag
set from compiler_options.cmake (not a bare invocation — findings end
[check,-warnings-as-errors]): zero findings in strand.hpp; the six it
does report are pre-existing lines in executor.hpp/logger.hpp that
clang-tidy-diff does not touch.

docs/spec/core/executor.md gains PendingQueue and the measured allocation
figures, replacing the paragraph that said only "the cost is allocation churn".

Not verified for #660

• The time cost. Nothing here claims a latency change; 608 bytes of
  short-lived churn is a malloc/free pair set, not necessarily visible.
• libc++, MSVC, WASM. The byte figures are libstdc++'s, and the eager
  std::deque allocation they turn on is an implementation choice. The saving
  may be smaller or absent elsewhere. Nothing regresses — the container is
  strictly smaller everywhere — but the number is one toolchain's.
• Kanban / ThreadSanitizer. Needs Qt + the ladder; not run locally.
  Linux / clang-tsan was, and it is the leg that covers strand.hpp's own
  tests.
• Coverage gates. check_branch_coverage.py needs a profile this lane did
  not produce; the allowlist repoint was verified against that script's own
  resolution rule instead.

---

#660 — commit 2, the gate repoint

PendingQueue inserts 67 lines above the allowlisted
if (iter != _strands.end() && iter->second == strand) {, so
branch_partial_allowlist.json's ST1 entry drifted from line 192 to 259. The
source text is unchanged and matches exactly one line, which is the "update
the hint, the disposition is fine" case resolve_allowlist_source_line() names.
Separate commit, no behaviour, no argument changed.

Re-ran that resolver's rule over every entry in all three allowlists
(branch_partial_allowlist.json, error_path_allowlist.json,
mutation_survivors.json): one drifted before, zero after. No backend.hpp
entry was touched, so the two deliberately-ambiguous citations did not come into
play.

---

#662 — commit 3, and why it does not close the issue

The fact, established first

clang++ -std=c++23 -fsyntax-only -Weverything, clang 22.1.8, one construct per
function:

construct	diagnostic	group
[](int value) {...}	shadows a local variable	-Wshadow-uncaptured-local
[] { int value = 2; ... }	shadows a local variable	-Wshadow-uncaptured-local
[value = 7] { ... }	shadows a local variable	-Wshadow-uncaptured-local
[first = first + 1] { ... }	shadows a structured binding	-Wshadow-uncaptured-local
{ int value = 2; } (no lambda)	shadows a local variable	-Wshadow
[value] { int value = inner; ... }	shadows a local variable	-Wshadow

This refines the ticket's framing. The split is not "structured binding
versus local variable" — rows 3 and 4 differ only in what is shadowed and land
in the same group, rows 5 and 6 are both locals and land in -Wshadow. The flag
routes on whether the shadowing declaration is inside a lambda with no
capture-default that did not capture the shadowed entity. So the suppression
means "any declaration inside an uncapturing lambda that shadows an uncaptured
enclosing local or structured binding" — four constructs, of which the comment
named one, and including the [x = std::move(x)] idiom used throughout this
codebase.

It also rules out the ticket's suggested remedy: "re-enable -Wshadow
explicitly after -Wno-shadow-uncaptured-local" cannot work, because -Wshadow
is already on via -Weverything and the groups are disjoint. clang has no
finer group, so narrowing to the named construct is not expressible at all.

Sizing the only real alternative

Suppression removed, -Werror swapped for -ferror-limit=0 so the build
completes rather than stopping at the first TU, clang 22.1.8, clang-debug with
NET/QT/FORMS_QML/OFFLINE_SQLITE/LOAD_TESTS/HMAC_EXAMPLES/LADDER/
BANK_EXAMPLE:

total emissions: 4856
--- by message ---
   4856 declaration shadows a local variable [-Wshadow-uncaptured-local]
--- distinct sites ---
41

41 distinct sites, 16 first-party files, 35 of them in headers (which is why
41 sites emit 4856 diagnostics). Eight each in core/backend.hpp and
core/remote.hpp, four each in core/bridge.hpp and core/completion.hpp. No
dependency affected. They are almost entirely the deliberate move-into-
continuation idiom, so the cleanup is 41 renames in the framework's most
lifetime-sensitive headers, for zero behaviour change — not a one-line diff.
Filed with the full breakdown as #669.

Note the second line of that output: zero shadows a structured binding.
#661 fixed the only four, so the class the WASM leg exclusively enforces
currently has no instances. The leg is a trap for the next one written, not a
live gate.

So: the comment is corrected, and nothing else

A suppression whose stated reason is narrower than its effect is a defect in
itself, so cmake/compiler_options.cmake:161 now records the six-row table, the
41-site measurement, the fact that the group cannot be narrowed, and the emsdk
divergence cited to the CI log it came from.

#662 is not closed. Its own condition is "a diagnostic in this class fails a
leg that a developer can run locally, demonstrated by mutating the code and
watching that leg go red". Nothing here changes which leg enforces what. It
stays open, blocked on #669, and the measurement is posted on the issue.

Not verified for #662

• Anything emsdk-side. No emsdk toolchain here, so the WASM claim is the
  ticket's own CI-log reading, labelled as such in the comment rather than
  restated as fact.
• Which clang release reclassified the structured-binding case. Only clang
  22.1.8 was available. This is the one part of "establish the fact" left
  unfinished.
• Whether any other diagnostic is split the same way. The probe covers six
  constructs, not the space.
• The 41-site count on any other toolchain. It is clang-22-specific by
  construction, and -Wno-shadow-uncaptured-local suppresses a different set on each clang, so the WASM leg is the only one enforcing structured-binding shadowing #662 itself shows the grouping moves between releases.

---

Issues filed

• test_strand_race.cpp does not detect the race it is the regression test for -- 10/10 passes under TSan against a build with the two-step drain-and-erase restored #668 (bug, area: core) — tests/test_strand_race.cpp does not detect
  the race it is the regression test for. Against the two-step-drain mutant it
  passed 10/10 under TSan, and the whole [strand] tag passed too; the only
  thing that caught the mutant was an unrelated Bridge test, intermittently
  (3rd of 60 repeats). Found because core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660 required proving the TSan
  configuration could see the hazard — the dedicated test is where I looked
  first, and it said nothing.
• Sizing the -Wshadow-uncaptured-local cleanup morph#662 needs: 41 distinct sites in 16 first-party files, 4856 diagnostics #669 (enhancement, area: ci) — the 41-site -Wshadow-uncaptured-local
  cleanup -Wno-shadow-uncaptured-local suppresses a different set on each clang, so the WASM leg is the only one enforcing structured-binding shadowing #662 is blocked on, with the full per-file breakdown.
• StrandExecutor still rebuilds the map node and the Strand on every serial dispatch -- 152 bytes, 2 allocations, and removing them needs a deregistration hook #670 (enhancement, area: core) — the 152 bytes / 2 allocations core: StrandExecutor rebuilds a model's Strand on every serial dispatch -- 4 allocations and 752 of the 1995 bytes per local call #660
  leaves behind, and why removing them needs a StrandExecutor deregistration
  hook rather than a container change.

No triage: label applied to any of them.

Review

Done inline rather than through /code-review or /simplify. The three things
worth a second reader's eye:

1. PendingQueue::pop()'s precondition. It is !empty(), and the single
   caller checks nothing — but it does not need to: scheduleNext's lambda only
   runs because post() pushed a task and flipped running, or because the
   drain block observed !pending.empty() and re-armed. Both decisions are made
   under strand->mtx, which the pop also holds. This is the same precondition
   std::queue::front() had before.
2. Occupancy is a flag, not operator bool on the callable. Deliberate: a
   posted empty std::function would otherwise be silently dropped and stall
   the strand. Today's code dispatches it and lets the bad_function_call land
   in the existing catch, which is the pre-existing behaviour, preserved.
3. _head = nullptr after the move. A moved-from std::function is valid
   but unspecified, so the idle slot is cleared explicitly rather than left
   holding whatever libstdc++ leaves — relevant because captured state
   (including shared_ptrs to models) would otherwise have an unspecified
   lifetime.

CI

Pushed and reported immediately, as instructed; CI was not waited on, polled
or re-run, so no check results are reported here — the picture is incomplete
by construction. Two flake shapes to discount if they appear: the ledger
scenario corpus failing with SQLite database is locked (#658), and an
ubuntu-toolchain-r PPA 503 killing legs at ~5 minutes during package
installation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

Checked and confirmed: fixes #660 / refs #662, #662 still OPEN, and the strand.hpp diff contains no lock, mutex, notify or wait code — the only lock-related lines are comments asserting the discipline is unchanged. That is consistent with the safety claim, though it is the weak form of the evidence; the strong form is below and it is the lane's, not mine.

The re-measurement changed the remedy, which is the point of re-measuring

The ticket said "StrandExecutor rebuilds a model's Strand every call" and implied the fix was to stop rebuilding. The measurement says 576 of the 760 bytes were never the strand's lifetime at all — they are libstdc++'s std::deque allocating a node map and a 512-byte buffer in its default constructor, for a queue that holds one task. So the fix is "shrink what is rebuilt" rather than "stop rebuilding", and it lands 2 allocations / 608 bytes, 30% of the round trip's bytes, without touching lifetime or locking at all.

That also means #670's residual (152 bytes, 2 allocations) is correctly separated: removing it needs a StrandExecutor deregistration hook, which is a different change with a different risk profile. Filing it rather than reaching for it is right.

The safety claim was tested, and — more importantly — the detector was tested first

Full ctest under the clang-tsan preset: 1801/1801 pass … and that configuration was proven able to see this hazard: restoring the pre-fix two-step drain makes TSan report a data race whose two stacks both run through scheduleNext's lambda into LoadCountModel::execute.

Proving the instrument can see the hazard before trusting its silence is the whole discipline this repository is built around, and it is the only reason the 1801/1801 means anything. A TSan run that passes because TSan was not instrumented, or because the workload never reaches the window, is the failure AGENTS.md leads with — and check_sanitizer_instrumentation.sh confirming all six binaries carry __tsan_ closes the first half of that.

#668 is the finding that matters most here

tests/test_strand_race.cpp asserts the right property — REQUIRE(maxInFlight.load() == 1), 134 lines, many threads hammering one key. And per the lane it passes 10/10 under TSan against the two-step-drain mutant, while an unrelated Bridge test caught the race intermittently, on the 3rd of 60 repeats.

So the named regression test for this repository's most-cited invariant does not detect the regression it exists for. That is the "control that reports success while measuring nothing" case, sitting on the concurrency contract — and it was found only because #660 required proving TSan could see the hazard at all. I have not reproduced it (no TSan run here), and it should be re-measured by whoever takes #668 before acting; but the shape of the evidence is right and the consequence if true is large.

#662 handled honestly rather than closed

Partially landed with refs, not fixes, because the comment defect is fixed and the enforcement gap is not — and nothing done here meets that issue's own closing condition. Two measured facts contradict the ticket's framing, and both are worth keeping:

1. Narrowing is not expressible. -Wshadow-uncaptured-local owns any declaration inside an uncapturing lambda shadowing an uncaptured local or structured binding — four constructs, of which the comment named one, including the [x = std::move(x)] idiom used throughout the tree. Clang has no finer group, so the ticket's suggested "re-enable -Wshadow after the suppression" cannot work.
2. Zero shadows a structured binding remain — core: give Bridge an executor for late registration replies, and re-measure what a local dispatch allocates (fixes #588, refs #572) #661 fixed the only four — so the WASM leg enforces nothing in this class today. It is a trap for the next one written, not a live gate. That reframes -Wno-shadow-uncaptured-local suppresses a different set on each clang, so the WASM leg is the only one enforcing structured-binding shadowing #662 from "a gate with accidental coverage" to "a gate with no coverage and an accidental past catch".

4856 diagnostics over 41 sites in 16 files is why the cleanup is #669's problem and not this PR's.

Not verified by me

The allocation figures, the TSan runs, the 41-site count and #668's reproduction — all need builds I did not run. Also unfinished by the lane, and worth carrying: which clang release reclassified the structured-binding case, which is the remaining half of #662's "establish the fact".

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Yaraslaut]

Runner: rebased onto 563502ab — rebase, on the gate limb

#665 merged, and its delta includes a gate this PR has never been judged by:

.github/workflows/drift-guard.yml       <-- new job
scripts/check_qobject_moc_pairing.py    <-- new gate
examples/bank/gui/**  examples/bank/tests/gui/**   (unrelated to this PR)

scripts/check_qobject_moc_pairing.py is the Q_OBJECT-header-pairing gate from #659, and it did not exist when this branch's CI last ran. That is the first limb of the staleness test — its CI was judged by rules that no longer exist — so rebase rather than merge, independently of the failures below.

Rebased cleanly, three commits replayed unchanged, pushed as be0c4a6f. The new gate passes on the rebased tree, along with the rest of the cheap set:

Q_OBJECT moc-pairing lint OK.
self-test OK: 9 fixture(s), including the #652 mutation of the real examples/common/CMakeLists.txt
ok: 15 structured citation(s) in scripts/mutation_survivors.json resolve to the line they name.
NOLINT directive lint OK: 177 NOLINTNEXTLINE directive(s), all annotating code.
bidi-control lint OK: 1246 text file(s) scanned, 12 codepoint(s) searched for, 0 raw occurrence(s).
All 58 rung-filter checks passed.
Prose lint OK: …

On the nine red legs this replaces

They were 28s–1m each, which is the package-installation signature, not a build or test one — the ubuntu-toolchain-r / Launchpad outage filed as #672. I could not read their logs to confirm individually, because that run still had a pending job and gh run view --log refuses while a run is in progress; the counts I could take from the readable ones were zero only because the log was unavailable, not because the signature was absent. Saying that plainly rather than reporting a confirmed cause.

What I did establish is that the outage has cleared: ppa.launchpadcontent.net returns HTTP 200 as of 16:07, and #665's re-run after that point went green and merged. So this fresh run should be judged on its merits.

The legs to watch are the ones that actually test this branch: Linux / clang-tsan for the strand change, and Linux / all optional features (clang) / (gcc) for the PendingQueue container swap.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner: 52 pass / 2 fail — both infrastructure, re-running

Linux / gcc-debug and Linux / gcc-release, both in the Install sccache step:

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now
##[error]Process completed with exit code 2

Confirmed on both legs (2 gzip/tar errors each). The installer was handed something that is not a tarball — almost certainly an error page from the download — and piped it to tar. No compilation ran, so this says nothing about the branch.

That matters here because the branch is a container change inside Strand, and GCC legs failing while every clang leg passes is exactly the shape a real portability problem would take. It is not one: nothing compiled.

Re-run requested at 17:55. Everything else is green — 52 of 54, including Linux / clang-tsan, which is the leg that actually judges #660's change.

Fourth distinct infrastructure failure today, after the PPA 503s, the Launchpad GPG 500s, and the database is locked flake. Added to #672 as another occurrence; this one is a different upstream from the other two, which strengthens rather than weakens that ticket's argument — the common factor is not one flaky host, it is that a setup-step failure is indistinguishable from a defect in the check list.

Note on my own monitoring, since it bears on this result

The first watcher I armed on this PR reported "settled" while every check was still pending: I started it seconds after a push, before the run's checks existed, so pending == 0 was true for the wrong reason. Re-armed with a total-count floor (total > 30 && pending == 0), and that is where the 52/2 above comes from — total checks: 54. A scan that examined nothing must not read as clean, which is the same rule check_bidi_controls.py and check_nolint_directives.sh already enforce on themselves.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW