Skip to content

Include <compare> in a generated semantic type - #191

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/blissful-euler-fzuap2
Sep 14, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/blissful-euler-fzuap2

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

main is red on the macOS cell, which #188 put back in the matrix. Two of the four failures are this, and it is a real generator bug rather than anything about the runner.

A generated semantic type defaults operator<=>, which answers with one of the ordering types in <compare>. The header never included it:

Kilograms.gen.hpp:35:36: error: cannot default 'operator<=>' because type
'std::partial_ordering' was not found; include <compare>

libstdc++ happens to have declared those types by the time the generated header is read and libc++ does not, so the type compiled under GCC and was refused by Clang. Nothing about the schema says which standard library the target uses, so that is not a difference a generator may leave to whoever includes its output.

Why it went unseen

Every test here that compiles generated C++ takes the first of g++ or clang++ on PATH, and on a Linux runner that is GCC. On macOS g++ is a shim for AppleClang and the whole suite compiles against libc++ — so #188 restoring the macOS cell is what surfaced it, not what broke it. The bug is as old as the emitted <=>.

I reproduced it locally by putting a g++ on PATH that execs clang++, which is the same shape macOS has. Schema.Cpp.Test goes 100/101 → 101/101 under that shim, and stays 101/101 under real GCC.

ExemplarSemanticTypeTests pins the header byte for byte, so it now carries the include too — which is what keeps this checked on every platform rather than only on the one whose standard library is strict about it.

This does not make main green on its own

The other two macOS failures are in Schema.Editor.Test — MenuTests.OpeningARecentFileLoadsIt throws Item 'recent/recalled.schema.json' was not drawn in the most recent frame, an ImGui harness timing difference on macOS/arm64. Those tests should not be running on macOS at all. #188's own rule says UI test projects are Linux-only, and it implements that as:

ktsubuild test all --workspace "$GITHUB_WORKSPACE" --verbose --exclude "**/*.UITests/*"

This repository's UI test project is Schema.Editor.Test, not *.UITests, so the glob matches nothing here and the suite runs on every platform — 2m10s of the macOS job, and flaky on it.

I have left that alone deliberately. The fix is either in the shared workflow's glob or in this repository's project name, and .github/workflows/dotnet.yml is the file #188 just made byte-identical across every ktsu .NET repository — editing it here would re-introduce exactly the drift that PR removed. That is a call for whoever owns the shared workflow.

Why this is urgent rather than tidy

#190's release never published. #188 merged nine minutes after it and its run cancelled #190's Analyze & Release mid-flight (Release step skipped), then run 771 failed on macOS. So ktsu.schema.tool is still at 1.34.0, and matt-edmondson/Holotype#16 is waiting on 1.34.1 — it needs #190's fix for a quantity with a default, which three of its members have.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UGHDsYaaTQdzVR4XBR6miu


Generated by Claude Code

A generated semantic type defaults `operator<=>`, which answers with one
of the ordering types in <compare>. The header never included it.

libstdc++ happens to have declared those types by the time the generated
header is read and libc++ does not, so the type compiled under GCC and
was refused by Clang:

  Kilograms.gen.hpp:35: error: cannot default 'operator<=>' because type
  'std::partial_ordering' was not found; include <compare>

Which is exactly the kind of difference a generator must not leave to
whoever includes its output. Nothing about the schema says which standard
library the target uses.

It went unseen because every test that compiles generated C++ takes the
first of `g++` or `clang++` on PATH, and on a Linux runner that is GCC.
macOS returning to the test matrix in #188 is what surfaced it: there
`g++` is a shim for AppleClang, and the whole suite compiles against
libc++.

The exemplar test pins the header byte for byte, so it now carries the
include too - which is what keeps this checked on every platform rather
than only on the one whose standard library is strict about it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UGHDsYaaTQdzVR4XBR6miu

Copy link
Copy Markdown
Contributor Author

Test on macos-latest is red, and the result confirms the fix works on the platform that exposed the bug.

Before (run 771, main at 753b6b5) — four failures:

assembly
Schema.Cpp.Test (net10.0|arm64) failed with 1 error
Schema.Cpp.Test (net9.0|arm64) failed with 1 error
Schema.Editor.Test (net10.0|arm64) failed with 2 errors

After (this PR, f00c5aa) — two:

assembly
Schema.Cpp.Test (net10.0|arm64) passed (15s 869ms)
Schema.Cpp.Test (net9.0|arm64) passed (15s 994ms)
Schema.Editor.Test (net10.0|arm64) failed with 2 errors

1832 total, 4 failed → 2 failed. Schema.Test passes on net8.0, net9.0 and net10.0 either way.

So the missing <compare> was both of the Schema.Cpp.Test failures, on the real AppleClang/libc++ rather than the shim I reproduced with.

What remains is not this PR's. Schema.Editor.Test fails identically on main at 753b6b5 and here at f00c5aa — two different commits, same two errors — which is stronger evidence than a re-run that it is the base branch's and not the diff's. This PR touches one generator file and one exemplar string; it cannot reach the ImGui harness.

No fix for it exists yet to port, and I am not writing one: the failing assertion is a frame-timing difference on macOS/arm64 that I cannot reproduce without a macOS runner, and by the workflow's own stated rule this suite should not be running there at all — --exclude "**/*.UITests/*" does not match Schema.Editor.Test. Making the test robust on a platform it was never meant to run on would be fixing the wrong thing, and disabling it is not on the table.

I am spending the one re-run on it, for a reason rather than out of habit: the failure is a timing assertion, so if it is genuinely intermittent rather than deterministic on this runner, a second run answers that and lets this PR merge. If it fails again it is deterministic, and the decision above belongs to whoever owns the shared workflow.

Why this matters beyond tidiness. #190's Analyze & Release was cancelled mid-flight by #188's run nine minutes later (its Release step reads skipped), and run 771 then failed — so ktsu.schema.tool is still at 1.34.0 and nothing will publish while main is red. matt-edmondson/Holotype#16 is waiting on 1.34.1.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Re-run done, and it answers the question: deterministic, not intermittent. Byte for byte the same result — 1832 total, failed: 2, succeeded: 1830, the same two Schema.Editor.Test errors. Schema.Cpp.Test passed again on both net9.0 and net10.0, so the <compare> fix is now confirmed twice on real AppleClang.

That is the one re-run spent. I am not running it again, and I am not touching the test or the workflow — so this PR stands as it is: it fixes what it says it fixes, and main goes from four macOS failures to two.

Getting to zero needs a decision I should not make unilaterally, because each option costs something different:

  • Change the shared workflow's glob — --exclude "**/*.UITests/*" → something that also matches *.Editor.Test. Correct in intent, but .github/workflows/dotnet.yml was just made byte-identical across every ktsu .NET repository by ci: adopt the consolidated .NET workflow (needs a repo variable set before merge) #188, so this is a change to that shared file rather than to this one.
  • Rename Schema.Editor.Test to the *.UITests convention the glob expects. Repo-local, but it touches the project file, the namespace, tests/ references and the SONAR_COVERAGE_EXCLUSIONS_EXTRA value ci: adopt the consolidated .NET workflow (needs a repo variable set before merge) #188 asks for.
  • Fix the harness timing so the suite genuinely passes on macOS/arm64. The most thorough, and the only one that keeps macOS coverage of the editor — but it is a UI-timing fix on a platform with no local reproduction here.

Worth noting the cost while it is unresolved: Schema.Editor.Test is 2m45s of a ~2m47s macOS test run, on a runner billed at roughly ten times Linux — which is the expense #188's Linux-only rule exists to avoid, and is not being avoided here.


Generated by Claude Code

matt-edmondson pushed a commit that referenced this pull request Sep 14, 2026
The exclusion works now - macOS run 34828747684 reported:

    Excluding tests/Schema.Editor.UITests/Schema.Editor.UITests.csproj from the test run.

Schema.Editor.Test passed there, so the thirteen frameless tests still run
on macOS, and Schema.Test passed on all three frameworks. The only failures
left on that cell are Schema.Cpp.Test on net9.0 and net10.0, which is #191's
fix and not this branch's work.

Rather than wait for #191 to merge, its change is ported here so this PR can
go green on its own. It is copied verbatim - same placement, same comment -
so the two are byte-identical and merge without a conflict: the post-image
blob is 6d74192 on both sides. Once #191 lands this commit is a no-op.

Schema.Cpp.Test: 101/101.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015qxqZVzN8CJcDxTb5gtWua
@matt-edmondson
matt-edmondson merged commit d3b7e87 into main Sep 14, 2026
16 of 18 checks passed
@matt-edmondson
matt-edmondson deleted the claude/blissful-euler-fzuap2 branch September 14, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants