[M1-SYS-03] Fix macOS/Windows CI: volatile compound-assign error; platform-dependent lastMs assertion - #28
Merged
Merged
Conversation
…tform-dependent lastMs assertion The windows-msvc, macos-intel, and macos-arm64 jobs are red on the M1-SYS-03 merge (run 82). Two independent, platform-sensitive defects, both in tests/laige-sim/system_timing_tests.cpp: macOS Intel (macos-14, AppleClang 15) — build error: system_timing_tests.cpp:125: error: compound assignment to object of volatile-qualified type is deprecated [-Werror,-Wdeprecated-volatile] The P1152R2 volatile deprecation (compound assignment to a volatile-qualified type) is diagnosed by AppleClang 15 and fatal under the engine policy's -Werror, while the other P0 job toolchains do not diagnose it (Clang 18 on ubuntu-24.04, AppleClang 16 on macos-15, MSVC 2022) — so only macos-intel failed to build. The burn-loop sink is now updated with an ordinary assignment (gBurnSink = gBurnSink + 1;), the pattern already established in budget_harness_tests.cpp's Spin() helper. The volatile read-modify-write remains, so the loop still defeats elimination. Windows (windows-2022) + macOS arm64 (macos-15) — test failure: SystemTiming.HealthyTicksLogNothingAndTrackStats: EXPECT_GT(st.value().lastMs, 0.0) — actual 0 vs 0 A noop system runs in tens of nanoseconds; on platforms whose steady_clock tick is coarser than that (the start and end reads land on the same tick) the measured run time is exactly 0.0 ms — a legitimate sub-resolution reading (wall-clock resolution is platform-sensitive; ARCH-009), not a failure state. The assertion was process-state-dependent: the unfiltered laige-sim_tests entry failed on Windows while the filtered system_timing entry passed, and vice versa on arm64 — the same flake class as the pre-M1-ECS-05 churn cost checks fixed in #20. The lower bound becomes EXPECT_GE(st.value().lastMs, 0.0); the tracked-state property is already pinned by the runs counter, and the upper bound (< 1.0 ms, under the 1 ms budget) is unchanged. Docs (DOC-006/007): system.h (SystemTimingStats::lastMs and SystemTimingRecord::lastMs) and docs/api/system_timing.md now state that a run shorter than the platform's steady_clock tick measures as exactly 0.0 ms and must not be treated as "not measured". laige-api.json: regenerated (manifest entries carry declaration line numbers; the system.h comment additions shifted four declarations). The api-manifest job's check passes against the regenerated manifest. Verification (local, g++ 16.2 / clang 22.1, Debug trees): - full ctest suite: 43/43 green on both toolchains - include-lint: OK - laige-api-scanner --check: up to date The Windows and macOS fixes are verified by this PR's ci:windows job, and (after the label swaps to ci:macos) the two macOS jobs.
offdev
added a commit
that referenced
this pull request
Sep 15, 2026
…SecondRunFailsWithoutLogging (#33) * [M1-HEAD-01] Fix macOS CI: platform-dependent zero-warn assertion in SecondRunFailsWithoutLogging The macos-arm64 and macos-intel jobs (merge matrix) are red on the M1-HEAD-01 merge: [ FAILED ] EngineRun.SecondRunFailsWithoutLogging engine_tests.cpp:411: Expected equality of these values: sink->entries.size() (Which is: 1) vs 0u The test asserted zero Warn-or-above entries over the WHOLE capture window - the first wall-clock-paced run_headless(1, 1) at 60 Hz (one 16.7 ms tick) plus the stopped-state second run. On a loaded runner a frame can run longer than one tick of simulation time, and the loop then warns loop/tick_dropped - the documented M1-LOOP-01 overload behavior (the BoundedRun tests in the same suite deliberately do not assert the drop count for this reason). The macOS runners' wall-clock tail (measured in the same merge run: dropped=3 in BoundedRun, dropped=2 in DoubleShutdownAfterRun) makes the warn near-certain there, while the faster Linux runners happened to stay under the 16.7 ms frame - a wall-clock-dependent assertion that does not travel across P0 platforms (the #20/#26/#28 CI-fix precedent). The test's actual property is only about the SECOND run: a stopped- state failure is a pure failure, no log (the GameLoop's moved-out precedent). The assertion is now scoped to that run - the sink entry count is recorded after the first run, and the second run must add none. The first run's legitimate tick_dropped warn (its own accounting, rate-limited per LOG-004) no longer taints the stopped-state check. No engine/loop code change: the second run still returns InvalidArgument before any log call (engine.cpp run_headless, stopped-state branch). Verified locally on the gcc and clang trees (47/47 ctest, the EngineRun suite green). Verified by this PR's ci:macos job (macos-arm64 + macos-intel). * Retrigger CI: ci:macos label applied to the PR The first pull_request event fired when the branch was pushed, before the PR existed and before the ci:macos label was attached, so that run's macOS jobs were skipped by the label selector. This empty commit re-fires the pull_request event with the label in place (the concurrency group cancels the label-less run).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The windows-msvc, macos-intel, and macos-arm64 jobs are red on the M1-SYS-03 merge (run 82). Two independent, platform-sensitive defects, both in
tests/laige-sim/system_timing_tests.cpp:1. macOS Intel (macos-14, AppleClang 15) — build error
The P1152R2 volatile deprecation (compound assignment to a volatile-qualified type) is diagnosed by AppleClang 15 and fatal under the engine policy's
-Werror, while the other P0 job toolchains do not diagnose it (Clang 18 on ubuntu-24.04, AppleClang 16 on macos-15, MSVC 2022) — so only macos-intel failed to build. The burn-loop sink is now updated with an ordinary assignment (gBurnSink = gBurnSink + 1;), the pattern already established inbudget_harness_tests.cpp'sSpin(). The volatile read-modify-write remains, so the loop still defeats elimination.2. Windows + macOS arm64 — flaky
lastMsassertionA noop system runs in tens of nanoseconds; on platforms whose
steady_clocktick is coarser than that (the start and end reads land on the same tick) the measured run time is exactly0.0ms — a legitimate sub-resolution reading (wall-clock resolution is platform-sensitive; ARCH-009), not a failure state. The assertion was process-state-dependent: the unfilteredlaige-sim_testsentry failed on Windows while the filteredsystem_timingentry passed, and vice versa on arm64 — the same flake class as the pre-M1-ECS-05 churn cost checks fixed in #20. The lower bound becomesEXPECT_GE(st.value().lastMs, 0.0); the tracked-state property is already pinned by therunscounter, and the upper bound (< 1.0ms, under the 1 ms budget) is unchanged.Docs + manifest
system.h(SystemTimingStats::lastMs,SystemTimingRecord::lastMs) anddocs/api/system_timing.mdnow state that a run shorter than the platform's steady_clock tick measures as exactly 0.0 ms and must not be treated as "not measured" (DOC-006/007).laige-api.jsonregenerated: manifest entries carry declaration line numbers, and thesystem.hcomment additions shifted four declarations (LAIGE_SYSTEM,SystemTimingStats::{lastMs,warns,errors}).Verification
Local (g++ 16.2 / clang 22.1, Debug trees): full ctest suite 43/43 on both toolchains;
include-lintOK;laige-api-scanner --checkup to date.CI:
workflow_dispatch(run https://github.com/offdev/laige-cpp/actions/runs/34886215741): all 10 jobs green, including the three previously red jobs — Windows x64 (MSVC 2022), macOS Intel (AppleClang), and macOS arm64 (AppleClang).ci:windowslabel stays on the PR so any further push to this branch re-verifies the Windows P0 job; the merge to master re-runs the full matrix viaci.ymlregardless of labels.