Skip to content

Review: restore C11 labels and C declaration ownership - #26

Open
Alexbeav wants to merge 1 commit into
review-base/mstan-8d56cf659fd8from
review/c11-c-linkage-20260907
Open

Review: restore C11 labels and C declaration ownership#26
Alexbeav wants to merge 1 commit into
review-base/mstan-8d56cf659fd8from
review/c11-c-linkage-20260907

Conversation

@Alexbeav

@Alexbeav Alexbeav commented Sep 7, 2026

Copy link
Copy Markdown
Owner

GCC 9 rejects the local declaration immediately after retry_candidates: in overlay_loader.c. Add the required null statement. It then reaches linking and exposes two local C++ declarations of fntrace_is_game_started; remove those duplicate declarations so the existing fntrace.h C declaration owns the linkage.

Two source-owned checks cover the actual label/declaration excerpt under -std=c11 -pedantic-errors and the header-owned declaration. They fail on the unchanged source and pass on this branch. They are registered in CTest.

Native Linux GCC 9 full-runtime controls:

  • Unchanged base and corrected candidate: the base fails at the C label, while this exact candidate compiles and links all 484 actions and passes both focused tests.
  • Label-only control: fixing only the label reaches the final link and fails on the C++-mangled fntrace_is_game_started(), independently demonstrating the second correction.

The CI workflow is on a separate build-only branch and is not part of this contribution.

Validation and limits

  • Exact base: 8d56cf659fd84367c3f778e17851674ac7896371; review head: d8725466bcbae7b235f33a6d334bfd455e39e7f6.
  • Native Windows GCC 16 runtime builds and passes 70/70 enabled runtime tests; two additional tests are disabled in the source configuration. The build has UI, setup wizard, debug tools, netplay, rewind and Vulkan disabled.
  • Owned Ghost in the Shell (SCES-01050) with owned SCPH1001: verification, current-source generation, native build and an isolated 50-second startup pass; normal SDL window close at frame 2456, process exit 0. Executable SHA-256 ebf755b918c52d9625e08aedb31e3b6d4baaeebbbdfe6022d5a573257e46fe67. This is bounded startup and normal shutdown, not gameplay completion or listening acceptance. Audio used SDL dummy output. No native Linux/macOS game route is claimed.
  • The unchanged current recompiler builds and has 58/61 enabled tests passing, with three pre-existing baseline failures (dirty_text_continuation_guards, aot_overlay_discovery, release_zip) and three disabled tests. This branch does not change that recompiler source. Those baseline failures are not reported as passes.
  • Diff audit: general source repair, synthetic tests and documentation only; no retail disc/BIOS, generated game code, player data or private tooling.

Scope and fork review

This review targets immutable review-base/mstan-8d56cf659fd8. It contains 1 commit and 6 changed files. Do not merge it into the fork default branch; it is the review record before upstream submission.

  • docs/C11_SOURCE_COMPATIBILITY.md: behavior and verification documentation.
  • runtime/CMakeLists.txt: source-owned regression and test registration.
  • runtime/src/main.cpp: bounded correction.
  • runtime/src/overlay_loader.c: bounded correction.
  • runtime/tests/test_fntrace_c_linkage.py: source-owned regression and test registration.
  • runtime/tests/test_overlay_retry_c11.py: source-owned regression and test registration.

Developed with AI assistance; validated as described (test evidence in PR body). AI writes the code and the PR, but I always test before I send something up. Happy to iterate on this process with your feedback.


Summary by cubic

Fixes GCC 9 C11 compilation and linking in the runtime so the source builds under -std=c11 -pedantic-errors and links without symbol conflicts.

Bug Fixes

  • Adds a null statement after the retry_candidates: label in overlay_loader.c to satisfy the C11 rule that a label must precede a statement.
  • Removes two local extern declarations of fntrace_is_game_started from main.cpp so the fntrace.h header owns the C linkage.
  • Registers two source-owned regression tests in CTest covering the label and the header-owned declaration.

Written for commit d872546. Summary will update on new commits.

Review in cubic

@Alexbeav

Alexbeav commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 7, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this PR

@Alexbeav I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Re-trigger cubic

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.

1 participant