Add Optional 128-bit Integer Support to C++ Runtime#239
Conversation
|
@jasongraffius Ready for another look |
- Update prelude.emb to allow UInt and Int sizes up to 128 bits
(previously limited to 64 bits). Added documentation noting that
128-bit support requires platform support (__uint128_t/__int128_t).
- Update constraints_test.py to test with sizes > 128 bits now that
128-bit fields are valid.
- Add comprehensive unit tests for 128-bit integer support:
- emboss_bit_util_test.cc: ByteSwap, MaskToNBits, IsPowerOfTwo tests
- emboss_memory_util_test.cc: MemoryAccessor, BitBlock tests for
128-bit values including non-full-width (72, 96 bits) and
cross-64-bit-boundary operations
- emboss_prelude_test.cc: UIntView and IntView tests for 128-bit
read/write, sign extension, and CouldWriteValue
- Add int128_sizes_test.cc integration test for generated code with
128-bit integer fields (UInt128Sizes, Int128Sizes, big-endian
variants, and arrays).
- Update int128_sizes.emb documentation and remove AnonymousBits128
struct (128-bit bits types require separate implementation work).
- Add EMBOSS_INT128_T and EMBOSS_UINT128_T macros for extensibility. - Change include path in emboss_cpp_types.h to relative. - Update copyright years to 2026 in testcode and .emb files. - Restore testdata/__init__.py copyright header. - Revert unrelated whitespace change.
UIntView::CouldWriteValue and IntView::CouldWriteValue range-checked the candidate value after casting it to uint64_t / int64_t. Once fields wider than 64 bits are allowed, the view's ValueType is __int128 and those casts truncate the value before it is compared against the field bounds, so an out-of-range value whose low 64 bits happen to fall in range is accepted. TryToWrite then proceeds into WriteUInt, which either aborts (EMBOSS_CHECK) or silently truncates -- so the "safe" write path crashes on out-of-range input instead of returning false. Additionally, IntView::CouldWriteValue gated its lower-bound check on ::std::is_signed<IntT>, which is false for __int128_t under a strict -std=c++NN (extended integer types are not "integral types" in the standard sense). The method is *enabled* via ::std::numeric_limits<>::is_integer, so the signedness test is now made the same way, via ::std::numeric_limits<>::is_signed, keeping the two consistent. Compare against the field bounds in the widest supported integer type (MaxWidthUnsigned / MaxWidthSigned) so no truncation occurs. Add regression tests at a non-power-of-two width above 64 bits (72) covering both the CouldWriteValue result and that TryToWrite fails gracefully rather than aborting.
…pport - emboss_defines.h: revert the incidental EMBOSS_ALIAS_SAFE_POINTER_CAST whitespace change that was flagged in review as unrelated to this change. - parser_types.py: revert an unrelated trailing-whitespace-only edit. - emboss_bit_util.h: define the 128-bit ByteSwap overload in terms of EMBOSS_UINT128_T rather than the raw __uint128_t, so that a client that overrides the type macro (e.g. to a library 128-bit type) also gets a consistent ByteSwap, matching how the type is used elsewhere. - testdata/BUILD: sort int128_sizes.emb before int_sizes.emb (buildifier).
3a9942c to
357dad7
Compare
DecodeInteger gated its handling of a leading '-' on ::std::is_signed<IntType>, which is false for __int128_t under a strict -std=c++NN. As a result a negative value could not be parsed into a signed field wider than 64 bits (the '-' was treated as an invalid character and parsing failed), even though the same value encodes fine on the way out. Use ::std::numeric_limits<>::is_signed instead, matching the fix applied to IntView::CouldWriteValue. Add text-format round-trip tests for values that do not fit in 64 bits, including a negative value into a >64-bit signed field and rejection of an out-of-range text value.
robrussell
left a comment
There was a problem hiding this comment.
The docs at https://github.com/google/emboss/blob/master/doc/language-reference.md#uint still say max size of Int and UInt are both 64 bits.
The different limit for the size of anonymous bit fields should be mentioned at https://github.com/google/emboss/blob/master/doc/language-reference.md#anonymous-bits .
| -- 128-bit integers (__uint128_t). The C++ backend will use EMBOSS_HAS_INT128 | ||
| -- to detect availability. On platforms without 128-bit integer support, | ||
| -- fields larger than 64 bits will fail to compile. | ||
| [static_requirements: $is_statically_sized && 1 <= $static_size_in_bits <= 128] |
There was a problem hiding this comment.
It looks like there's a place where a check for this this could be included at
emboss/compiler/front_end/constraints.py
Line 704 in b9f89e3
| template <int kBits> | ||
| struct LeastWidthInteger final { | ||
| static_assert(kBits <= 64, "Only bit sizes up to 64 are supported."); | ||
| using Unsigned = typename LeastWidthInteger<kBits + 1>::Unsigned; |
There was a problem hiding this comment.
I'm a little confused about this (including the previously existing implementation)
If kBits is 64 then this is
using Unsigned = typename LeastWidthInteger<65>::Unsigned;
Is this using just never applicable because the specializations for 64, 32, 16, and 8 always apply? Or is there something else I'm missing?
… support - Document the new 128-bit UInt/Int limits (and the platform-support caveat) in the language reference, and note the unchanged 64-bit limit on bits types, including anonymous bits. - Move the bits-size note out of testdata/int128_sizes.emb into the docs. - Reword test comments (drop "now"; -(2**120) instead of ambiguous -2**120) and use a hex literal in the text-format test so the value is easy to relate to the expected shifted constant. - Explain the LeastWidthInteger rounding-up recursion in a comment and make the no-int128 static_assert message say why the width is unsupported.
Adds optional support for 128-bit
UIntandIntfields (65–128 bits) to the C++ backend and runtime, using the compiler's native__int128_t/__uint128_twhere available.What this does
prelude.emb:UIntandIntnow permit static sizes up to 128 bits (previously 64).EMBOSS_HAS_INT128(derived from__SIZEOF_INT128__). When it is unavailable, fields wider than 64 bits fail to compile, exactly as before.EMBOSS_INT128_T/EMBOSS_UINT128_Tmacros, which a client may override (e.g. to a library 128-bit type) before including the runtime.LeastWidthInteger,BitBlock,MemoryAccessor, andByteSwapare extended to 128 bits. Reads and writes work for full-width, non-full-width (e.g. 72/96-bit), little- and big-endian, arrays, and the text format.Scope / limitations
bitstypes remain limited to 64 bits (widening them would require further work inOffsetBitBlockand the constraints checker).__int128(64-bit GCC/Clang). 32-bit targets and MSVC are unaffected and continue to reject >64-bit fields at compile time.Testing
ByteSwap,MaskToNBits,IsPowerOfTwo,LeastWidthInteger,MemoryAccessor(LE/BE, non-full-width), andUIntView/IntViewread/write/sign-extension at 128 bits.int128_sizes.emb) covering little-/big-endian structs, arrays, min/max,CopyFrom, out-of-range rejection, and text round-trips.Changes since the previous review
This revision rebases onto current
masterand fixes correctness issues in the write/validation path that the earlier version did not handle:UIntView/IntView::CouldWriteValuecompared the candidate value against the field bounds after casting it touint64_t/int64_t. With a 128-bitValueTypethis truncated the value, so an out-of-range value for a 65–127-bit field was accepted, andTryToWritethen aborted (or silently truncated) instead of returningfalse. The comparison is now done in the widest supported integer type.is_signedvs__int128.std::is_signed<__int128_t>isfalseunder a strict-std=c++NN, so the signed lower-bound check (and negative-value text parsing) was compiled out for 128-bit signed fields. Signedness is now determined viastd::numeric_limits<>::is_signed, consistent with how these methods are enabled.ByteSwapin terms ofEMBOSS_UINT128_Tfor override consistency, and sorted thetestdata/BUILDentry.