fix: support optional member detection with MSVC#353
Merged
Conversation
Check that an optional field accessor exists before forming setter expressions that depend on its type. This prevents MSVC from instantiating references to absent generated core members while building mx::api.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.5% | 7237 / 8560 |
| Functions | 74.9% | 2511 / 3352 |
| Branches | 52.4% | 6327 / 12068 |
Core HTML report | API HTML report
Commit caec11106a87a4aa66e3cd6b74a9ce882a7b04a0.
gen-quality
|
webern
approved these changes
Jul 14, 2026
webern
left a comment
webern
left a comment
Owner
There was a problem hiding this comment.
Thank you. I'll get Windows added to CI soon.
4 tasks
webern
added a commit
that referenced
this pull request
Jul 15, 2026
## Human Summary CI was hard to understand so I am attempting to simplify it. ## Summary The Makefile target names and the CI job layout had both drifted into something hard to follow. This pass makes both legible around one mental model: a single product (`mx::api`) on a substrate (`mx::core`), plus a dev tool (`gen/`). Makefile: - Targets renamed to a consistent area-first `<area>-<action>` scheme: `core-build` / `core-roundtrip-test` / `core-unit` / `core-coverage`, `api-lib` / `api-build` / `api-test` / `api-roundtrip` / `api-coverage`, plus `gen-test`, `audit-test`, `fmt-check`, and the `api-roundtrip-*` helpers. - New `test-all` runs every C++ suite at once (core roundtrip + unit + api-test + api-roundtrip) — the deep gate CI uses on Linux. - Deleted four low-value or redundant targets and their drivers: `validate-cpp` (plus `mxtest/validate/` and the `mxtest-validate` CMake target), `probe-cpp` (plus `mxtest/probe/`), `check-core-dev`, and `gen-check` (its rename and collision checks already run during real generation). CI, from ten always-on jobs down to a leaner set: - `test-linux` (`make test-all`), `test-macos` (native AppleClang, api tier), `test-windows` (new — native MSVC, driving CMake directly), `swift`, `quality` (fmt-check + generated-output drift), and a `gen` job gated on changes under `gen/`. - `test-windows` is a required check and is expected to stay red until the code is MSVC-clean; it exists so that fallout runs on every PR from now on. - Coverage moved out of the always-on run into a new manual-only `coverage.yaml`, triggered by the workflow's Run button or a `/coverage` comment on a PR. It posts the same gcovr tables to the PR as before. `pr-comment.yaml` is unchanged; it still posts the gen-quality comment for fork PRs. Docs, skills, and instruction files are updated to the new names. ## Testing This is a CI/build change, so the workflows running on this PR are the real test. Verified locally: - [x] `make help` renders, and `make -n test-all` resolves the full dependency chain in both the in-container and docker-wrapper branches - [x] Every dual-branch target is defined exactly once per branch, and no stale target names remain outside `docs/ai/design/` (kept as historical) - [x] All three workflow files parse as YAML - [x] Full suites and coverage run in CI on this PR (not run locally) ## References - Related: #354 (the `small` / `rpcndr.h` MSVC conflict the new Windows job will surface) and #353 (the recent MSVC build fix this CI job guards going forward) --- _Generated by [Claude Code](https://claude.ai/code/session_01K3cW6EXsTowPxbmPSRVNAH)_
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
2 participants
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.
Summary
Fixes the mx::api build with MSVC by making optional-member setter detection two-stage. Resolves #352.
The helper macros now confirm that a generated core type exposes the field accessor before evaluating setter expressions that depend on the field’s optional value type. This prevents MSVC from diagnosing absent attributes such as font decoration and stop-length fields on types that do not support them.
Validation
mxwith CMake and Visual Studio 2022/MSVC.mx.liblinks successfully.git diff --checkpasses.Notes
/W4, so users should use/W3for MSVC builds.FontData.hwith thesmallmacro in the WinAPI headerrpcndr.h. This PR does not address that issue.