fix(base): spell File's types the way the rest of base does - #16
Merged
Merged
Conversation
base/filesystem/file.h names uint8_t, uint32_t and int64_t across its signatures and includes nothing that declares them. They arrived transitively until base stopped pulling <cstdint> in for itself (#14), so whether the build survives now depends on the standard library: gcc 15 still lands them through another include, gcc 13 does not, and the library stops dead at base/filesystem/file.h:134:26: error: 'uint32_t' has not been declared base/filesystem/file.h:192:48: error: 'uint8_t' was not declared in this scope with the Span<uint8_t> signatures under it collapsing into 40 more diagnostics. A consumer on ubuntu 24.04 (gcc 13.3) cannot build devel5. The fix is not to include <stdint.h>. base has these types already -- u8, u32, i64, mem_size out of base/arch.h, which includes nothing itself by design -- and 54 of the 56 headers in base that name a fixed-width type use them. file.h was one of the two that did not, so it now says what its neighbours say, and the platform definitions in posix/file_posix.cc and win/file_win.cc follow the declarations. Casts and locals inside those files keep the C spellings, which is right: that code is talking to POSIX and Win32. base/hashing/crc.h is the other one, and it has the same gap without having failed yet: uint32_t reaches it only through whichever of <nmmintrin.h>, <intrin.h> or <arm_acle.h> its #if chain picks, and a target matching no branch gets none of them. That leaves base with no C fixed-width types in any public signature, and no new include to carry. Library and tests build clean on gcc 13.4 and gcc 15.2.
Force67
force-pushed
the
fix/stdint-includes
branch
from
September 16, 2026 19:17
92eb134 to
5d94b26
Compare
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
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.
base/filesystem/file.hnamesuint8_t,uint32_tandint64_tacross itssignatures and includes nothing that declares them. They arrived transitively
until base stopped pulling
<cstdint>in for its own use (#14), so whether abuild survives now depends on the standard library it meets:
and the
Span<uint8_t>signatures below collapse into ~40 further diagnostics.A consumer on ubuntu 24.04 cannot build
devel5at all — which is how itsurfaced: a downstream CI job went red at the commit that pinned equilibrium
forward, on the runner's gcc 13.3, while the same tree built fine locally on
gcc 15.
Not by adding the include back
The reflex fix is
#include <stdint.h>, and it would work. But base already hasthese types —
u8,u32,i64,mem_sizefrombase/arch.h, a header thatby its own comment "may never include other files" — and 54 of the 56
headers in base that name a fixed-width type already use them.
file.hwasone of the two stragglers. Adding a C header back into base one commit after
#14 took the C runtime out of it is pulling in the wrong direction.
So
File's surface is spelled the way its neighbours are, and the platformdefinitions in
posix/file_posix.ccandwin/file_win.ccfollow thedeclarations. Casts and locals inside those files keep
size_tand friends,which is correct — that code is talking to POSIX and Win32.
base/hashing/crc.his the other straggler, with the same gap and no failureyet: its
uint32_tarrives only through whichever of<nmmintrin.h>,<intrin.h>or<arm_acle.h>the#ifchain selects, and a target matching nobranch gets none of them.
The result is no C fixed-width type left in a base public signature, and no new
include to carry.
Verification
Library and tests build clean and all four test binaries pass on gcc 13.4
and gcc 15.2.
file_win.ccis a textual conversion I could not compilehere; its definitions were diffed against the header signature by signature.