[M1-SYS-02] Fix Windows CI: MSVC C4267/C4127 fatal under /WX - #26
Merged
Merged
Conversation
The windows-msvc job (merge matrix, ci-pull.yml ci:windows) is red on every master merge since M1-ECS-05: two warning classes are fatal under the engine policy's /W4 /WX (NFR-8.10, CORE-010): - C4267 (size_t -> uint32_t) in iter_order_tests.cpp's per-round Fisher-Yates shuffle: the loop variable is initialized from live.size(), which is 64-bit on Windows x64. live holds at most kScenarioEntities (16, a uint32_t constant) elements, so an explicit narrowing cast to uint32_t is lossless and the loop is unchanged. - C4127 (conditional expression is constant) in the new World::resolveIoEntry template (M1-SYS-01): the access of the Io<T, Access> tag is a compile-time constant, and the plain if tripped the warning in every instantiating translation unit (system_registry_tests.cpp, scheduler_tests.cpp — the two registerSystem call sites). The branch is now if constexpr, as the compiler's own diagnostic suggests: the discarded branch is never emitted, and the behavior is identical (the condition was constant to begin with). Both are code fixes, not warning suppressions (CORE-010). No public API change (resolveIoEntry is a detail template), so laige-api.json is unchanged and the api-manifest job stays green. Validation: local g++ Debug tree 42/42 ctest, clang++ Debug and clang++ ASan trees rebuilt and re-tested; the PR carries the ci:windows label so the windows-msvc job verifies MSVC 2022 in CI.
…piler-specific spelling
The Windows job's next failure layer (with the compile errors fixed):
SystemScheduler.MacroCarriesDependsOnSpec asserted the EXACT spelling
of the no-deps spec built by LAIGE_SYSTEM(SchNoop, 1):
EXPECT_STREQ(SchNoop_Def.dependsOn, "");
The empty variadic pack's spelling is a preprocessor property, and the
compilers diverge:
- GCC/Clang: #__VA_ARGS__ stringizes the empty pack to ""
([cpp.stringize]/2: "The character string literal corresponding to
an empty stringizing argument is \"\"")
- MSVC 2022: the empty pack stays an empty token sequence, so the
4th initializer is absent and dependsOn is value-initialized to
nullptr.
Both are the documented no-deps form (SystemDef::dependsOn: "nullptr
or "" = no dependencies"; parseDepSpec: "nullptr or a whitespace-only
spec is Ok with count 0"), and the engine consumes the spec through
parseDepSpec only — every other scheduler test passed on MSVC, so the
observable behavior is identical. The assertion pinned one compiler's
spelling, which is exactly the kind of platform-dependent raw assertion
that does not travel across P0 platforms (the M1-ECS-03 churn-check
fix, same reasoning).
The test now pins the contract (no deps <=> nullptr or ""), keeping
the exact-spelling KATs for the one- and two-name cases, which
stringize identically on all three compiler families.
system.h's macro documentation updated to state both spellings
(DOC-003). laige-api.json regenerated: the documentation lines shifted
the system.h symbol lines (27 entries, line numbers only — no
signature or summary changes).
Validation: local g++ and clang++ Debug trees, 42/42 ctest each;
the api-real-tree drift check and the api-manifest job see the
regenerated manifest.
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
1 participant
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.
Fixes the red
windows-msvcjob on master. Two failure layers, both MSVC-specific:1. Compile errors (fatal under the engine policy's
/W4 /WX, NFR-8.10)size_t→uint32_t) initer_order_tests.cpp:304— the per-round Fisher-Yates shuffle loop variable is initialized fromlive.size()(64-bit on Windows x64).liveholds at mostkScenarioEntities(16) elements, so an explicit narrowing cast is lossless; the loop is otherwise unchanged.World::resolveIoEntry(entity.h, M1-SYS-01) — theIo<T, Access>tag's access is a compile-time constant; the branch is nowif constexpr, exactly as the compiler's own diagnostic suggests. The discarded branch is never emitted; behavior identical.Both are code fixes, not warning suppressions (CORE-010).
2.
SystemScheduler.MacroCarriesDependsOnSpec(runtime)The test asserted the exact spelling of the no-deps spec built by
LAIGE_SYSTEM(SchNoop, 1):EXPECT_STREQ(SchNoop_Def.dependsOn, ""). The empty variadic pack's spelling is a preprocessor property and the compilers diverge:#__VA_ARGS__stringizes the empty pack to""([cpp.stringize]/2: "The character string literal corresponding to an empty stringizing argument is""").dependsOnis value-initialized tonullptr.Both are the documented no-deps form (
SystemDef::dependsOn: "nullptr or""= no dependencies";parseDepSpectreats them alike), and the engine consumes the spec throughparseDepSpeconly — every other scheduler test passed on MSVC. The assertion pinned one compiler's spelling — the same class of platform-dependent raw assertion the M1-ECS-03 churn-check fix removed. The test now pins the contract (no deps ⟺nullptror""), keeping the exact-spelling KATs for the one- and two-name cases (stringized identically on all compiler families).system.hdocumentation updated to state both spellings (DOC-003).Manifest
laige-api.jsonregenerated: the doc/comment lines shifted 27system.hsymbol line numbers (no signature or summary changes). Theapi-manifestjob and the in-suiteapi-real-treecheck see the regenerated manifest.Validation
api-real-tree).ci:windowslabel, so thewindows-msvcjob (MSVC 2022) runs in PR CI to verify on the failing platform; on merge to master the full P0 matrix runs again perci.yml.