Skip to content

fix: require <compare> in a generated semantic type header - #192

Closed
matt-edmondson wants to merge 1 commit into
mainfrom
claude/happy-rubin-w67sx1
Closed

matt-edmondson wants to merge 1 commit into
mainfrom
claude/happy-rubin-w67sx1

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes the macOS-only C++ failure that #188 exposed when it added the macOS matrix cell.

The defect

A semantic type emits a defaulted operator<=>, whose return type is std::strong_ordering — or whichever sibling the representation yields — so the header depends on <compare>. It never asked for it:

#include <cstdint>
#include <type_traits>
// ...
[[nodiscard]] friend constexpr auto operator<=>(EntityId, EntityId) noexcept = default;

One line in CppFileBuilder.SemanticType, beside the operator it is for:

mapper.Require("<compare>");

Why only macOS saw it

Not because the bug is macOS-specific — it never was. Every test that compiles generated C++ includes the whole emitted set as one translation unit, and under libstdc++ another header in that set reaches <compare> while under libc++ none does. Apple clang was simply the only compiler ever asked to compile the header without something else having supplied it first.

The regression test, and the part that nearly went wrong

EveryGeneratedHeaderCompilesOnItsOwn compiles each header alone. That alone would not have caught this one, and I confirmed it does not: a defaulted comparison has no return type deduced until something orders two values, so a lone include and an empty main compile happily on every platform. The first version of this test passed with the fix reverted.

So OrderingASemanticTypeCompilesWithOnlyItsOwnHeader orders a pair, which neither libstdc++ nor libc++ can do without <compare>. With the include removed it now fails on Linux, reproducing the original error against g++:

EntityId.gen.hpp:37:41: error: 'strong_ordering' is not a member of 'std'
EntityId.gen.hpp:12:1: note: 'std::strong_ordering' is defined in header '<compare>';
                             did you forget to '#include <compare>'?

That is the point of it: the next omission of this kind fails wherever it is compiled, rather than waiting for a macOS run.

Verification

  • Schema.Cpp.Test — 103/103 on net10.0
  • Schema.Test — 476/476
  • Fix reverted → OrderingASemanticTypeCompilesWithOnlyItsOwnHeader fails with the error above; restored → green
  • net9.0 was not run locally (no 9.0 runtime in this container); CI covers it

Still open on macOS, and not addressed here

The macOS cell reported 4 failures across two projects. This PR fixes the Schema.Cpp.Test one (1 per TFM). The other two are in Schema.Editor.Test and are unrelated ImGui probe failures:

MenuTests.OpeningARecentFileLoadsIt
System.InvalidOperationException: Item 'recent/recalled.schema.json' was not drawn
in the most recent frame, so its recorded position is stale.

Those are headless UI tests. Worth noting that the consolidated workflow excludes UI suites from non-Linux runners by the **/*.UITests/* pattern, and Schema.Editor.Test does not match it — so unlike every other repo's UI suite, this one now runs on Windows and macOS. Whether the right answer is to fix the tests or to rename/exclude the project is a call I have left to you rather than folding into a one-line generator fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_015qxqZVzN8CJcDxTb5gtWua


Generated by Claude Code

A semantic type emits a defaulted `operator<=>`, whose return type is
`std::strong_ordering` - or whichever sibling the representation yields -
so the header depends on `<compare>`. It never said so.

Nothing caught it because every test that compiles generated C++ includes
the whole emitted set as one translation unit, and under libstdc++ another
header in that set reaches `<compare>` while under libc++ none does. Apple
clang was therefore the only compiler to refuse it, and macOS the only
platform that reported a defect every platform was carrying.

The regression test compiles each header on its own. That alone would not
have caught this one: a defaulted comparison has no return type deduced
until something orders two values, so a lone include and an empty `main`
compile anywhere. The second test orders a pair, which neither libstdc++
nor libc++ can do without `<compare>` - with the include removed it fails
on Linux, so the next omission of its kind cannot wait for a macOS run.

Verified: 103/103 in Schema.Cpp.Test and 476/476 in Schema.Test, and the
new test reproduces the original `'strong_ordering' is not a member of
'std'` against g++ when the fix is reverted.

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

Copy link
Copy Markdown
Contributor Author

Closing this as a duplicate of #191, which got there first and should be the one that merges.

I opened this without checking for an existing PR — my mistake. #191's production change is the same one-liner in the same place, mapper.Require("<compare>") in CppFileBuilder.SemanticType, with the same update to the ExemplarSemanticTypeTests byte-for-byte pin. There is nothing to choose between them on the fix itself, and #191 additionally carries the release context: #190's publish was cancelled mid-flight, so ktsu.schema.tool is still at 1.34.0 with a downstream consumer waiting on 1.34.1.

#191's reproduction is also the better one — putting a g++ on PATH that execs clang++ reproduces the macOS toolchain locally, rather than inferring it as I did.

One thing here is not in #191, and I am noting it rather than pushing it anywhere: a compile test with teeth on Linux.

#191 keeps the include honest with the exemplar's text pin, which does catch someone deleting the Require line, on any platform. What it does not catch is the next construct that needs a new header. A compile test is the thing that would, and the obvious form of it does not work:

  • EveryGeneratedHeaderCompilesOnItsOwn — compiles each generated header alone, empty main. This passes with the fix reverted. A defaulted comparison has no return type deduced until something orders two values, so the missing include is invisible.
  • OrderingASemanticTypeCompilesWithOnlyItsOwnHeader — compiles #include "EntityId.gen.hpp" and orders a pair. Neither libstdc++ nor libc++ reaches <compare> through <cstdint> or <type_traits>, so with the include removed this fails on Linux, reproducing the original error against g++:
EntityId.gen.hpp:37:41: error: 'strong_ordering' is not a member of 'std'
  note: 'std::strong_ordering' is defined in header '<compare>';
        did you forget to '#include <compare>'?

That is the property worth having: an omission of this kind fails wherever it is compiled, instead of waiting for a macOS run. The branch claude/happy-rubin-w67sx1 keeps it if it is wanted as a follow-up once #191 lands.

Both of us independently reached the same conclusion about the remaining macOS redness, which #191 states well: the other two failures are Schema.Editor.Test, the shared workflow's **/*.UITests/* glob does not match that project name, and the fix is either the glob or the project name rather than anything editable in this repository.


Generated by Claude Code

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