Skip to content

FileSystem/Archives: fix narrowing and aggregate-init portability (AppleClang/libc++)#3076

Merged
sprunk merged 2 commits into
beyond-all-reason:masterfrom
tomjn:fix/archives-clang-portability
Jun 30, 2026
Merged

FileSystem/Archives: fix narrowing and aggregate-init portability (AppleClang/libc++)#3076
sprunk merged 2 commits into
beyond-all-reason:masterfrom
tomjn:fix/archives-clang-portability

Conversation

@tomjn

@tomjn tomjn commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What

Fix three small portability issues in the VFS archive readers that break compilation under AppleClang / libc++, while remaining correct on every compiler. They touch overlapping files, so they land as one PR.

The archive readers build their entry vectors with emplace_back() of an aggregate struct (FileEntry / Files). That relies on parenthesised aggregate initialisation (P0960R3), which AppleClang 15 / libc++ does not fully implement, so the emplace_back calls fail to compile.

Changes

  • Switch emplace_back(...) to push_back(T{...}) brace-initialisation, which works on every compiler.
  • Add explicit narrowing casts to the declared member types (UInt32/UInt64/uLong -> int/uint32_t). Brace-init is stricter about narrowing than paren-init; the stored values are identical, since the old paren-init performed the same conversions implicitly.
  • Use fileEntries.back() instead of capturing the call result: push_back() returns void, not a reference to the inserted element.

Files:

  • rts/System/FileSystem/Archives/DirArchive.cpp
  • rts/System/FileSystem/Archives/SevenZipArchive.cpp
  • rts/System/FileSystem/Archives/ZipArchive.cpp

No behavioural change on platforms that already compiled.

Provenance

Ports the code fixes from ExaDev/RecoilEngine commits 0095a7e5c5, bba6321b05, bc0457cf7b. The CI/runner (macos-15) changes in those commits are deliberately excluded as fork-specific.

Testing

  • Confirmed the edits exactly reproduce the net intended state of the three source commits, and that current master matches the base those commits applied against.
  • Verified the cast targets against the struct definitions in the headers (FileEntry.fp/size are int, crc/modTime are uint32_t; Files is {string, string, int32_t, uint32_t}) — every cast is value-preserving.
  • A clean archives target build (gcc-16 and clang, arm64 macOS) is currently blocked by an unrelated streflop_cond.h sqrtf error under the macOS SDK that affects every translation unit in the target (including files untouched here); that is being addressed separately. The changes here introduce no compile errors of their own up to that point.

Human verification of a full build on a fixed toolchain is still required before merge.

AI disclosure

These changes were extracted and ported with AI assistance (Claude Code). The diffs were reviewed by a human; the verification limitation above is stated honestly and has not been worked around.

…pleClang/libc++)

The archive readers build their entry vectors with emplace_back() of an
aggregate struct, which relies on parenthesised aggregate initialisation
(P0960R3). AppleClang 15 / libc++ does not fully implement it, so the
emplace_back calls fail to compile.

Switch to push_back() with brace-initialisation, which works on every
compiler. Brace-init is stricter about narrowing, so add explicit casts
to the declared member types (UInt32/UInt64/uLong -> int/uint32_t). The
stored values are unchanged: the previous paren-init converted to the
same member types implicitly. push_back() returns void rather than a
reference, so use fileEntries.back() instead of capturing the result.

No behavioural change on platforms that already compiled.

Files: DirArchive.cpp, SevenZipArchive.cpp, ZipArchive.cpp

Ports the code fixes from ExaDev/RecoilEngine commits 0095a7e,
bba6321 and bc0457c (CI/runner changes deliberately excluded).
@sprunk

sprunk commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Static casts look fine.

const auto& fd = fileEntries.emplace_back(
i, //fp
SzArEx_GetFileSize(&db, i), // size
fileEntries.push_back(FileEntry{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
fileEntries.push_back(FileEntry{
const auto& fd = fileEntries.emplace_back(FileEntry{

You could just do this

n-morales noted push_back + fileEntries.back() can collapse back into a
single emplace_back. Passing a single brace-initialised FileEntry/Files
object avoids the parenthesised-aggregate-init (P0960R3) path that
AppleClang/libc++ rejects, while emplace_back's returned reference (C++17)
restores the named fd binding and drops the extra back() lookup. Applied
to the SevenZip and Zip readers, and to the Dir reader for consistency.
@sprunk sprunk merged commit 8141bf2 into beyond-all-reason:master Jun 30, 2026
3 checks passed
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.

3 participants