Guard 64-bit iterator size static_asserts for non 64-bit targets#217
Open
adityasingh2400 wants to merge 1 commit into
Open
Guard 64-bit iterator size static_asserts for non 64-bit targets#217adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
The integer range iterator tests assert the exact byte size of several iterator types using hardcoded values that assume a 64-bit data model, for example sizeof(IntegerRangeIterator<...>) == 24. These iterators are built from std::size_t-width members, so on platforms with narrower pointers and std::size_t, such as wasm32, the actual sizes are smaller and the static_asserts fail to compile. The byte counts are only meaningful as a layout check on 64-bit targets, so wrap them in a __SIZEOF_POINTER__ == 8 guard. This mirrors the existing platform gating in these files and removes the false build failures reported when compiling with emscripten, while keeping the layout checks fully enforced on the primary 64-bit configurations.
46a04c2 to
922ae2e
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.
Problem
The integer range iterator tests assert the exact byte size of several iterator types with hardcoded values that assume a 64-bit data model, for example:
These iterators are composed of std::size_t-width members. On platforms with narrower pointers and std::size_t, such as wasm32, the iterators are smaller, so these static_asserts fail to compile. This is one of the remaining failures from issue #128 when compiling the project with emscripten, after the unrelated
iterator_two phase lookup fix in #129. The reporter confirmed these size and death test asserts were the only remaining errors.Root cause
The asserted byte counts are valid only under a 64-bit data model. They are layout sanity checks, not portability requirements, but they were written unconditionally, so any target where
sizeof(std::size_t)is not 8 breaks the build. I also verified that the sizes are not a clean multiple ofsizeof(std::size_t)on 32-bit targets because of alignment of the iterator wrappers, so a per-platform recomputation would be fragile.Fix
Wrap the size static_asserts in a
defined(__SIZEOF_POINTER__) && __SIZEOF_POINTER__ == 8guard in the three affected test files:test/integer_range_iterator_test.cpptest/circular_integer_range_iterator_test.cpptest/filtered_integer_range_iterator_test.cppThis mirrors the existing platform gating already used in these files, keeps the layout checks fully enforced on the primary 64-bit configurations, and skips them where the exact byte count is not meaningful. No library or runtime behavior changes, this only affects compile-time layout assertions in tests.
Verification
__SIZEOF_POINTER__ == 8: all three test translation units compile with-std=c++20, the guarded asserts stay active and pass.-m32, where__SIZEOF_POINTER__ == 4, reproducing the narrower wasm32 width: an isolated probe of the same asserts failed before this change with the exact errors reported in the issue, and compiles clean after, since the guard now skips them.Out of scope: the
EXPECT_DEATHerrors under emscripten come from gtest death tests not being available on that platform, which is a gtest and toolchain limitation rather than a fixed-containers issue, so they are left untouched.Fixes #128