Skip to content

Fix sign-extension in FNV-1/FNV-1a byte processing; add std::byte support - #4

Merged
rwindegger merged 3 commits into
feature/fnv_1from
copilot/sub-pr-3
Mar 22, 2026
Merged

Fix sign-extension in FNV-1/FNV-1a byte processing; add std::byte support#4
rwindegger merged 3 commits into
feature/fnv_1from
copilot/sub-pr-3

Conversation

Copilot AI commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

update() was casting range elements directly to std::size_t, causing sign-extension on platforms where char is signed (bytes ≥ 0x80 would expand to e.g. 0xFFFFFFFFFFFFFF80), producing incorrect and platform-dependent hashes.

Changes

  • fnv_1.h / fnv_1a.h: update() now uses if constexpr to dispatch byte normalization:
    • std::bytestd::to_integer<std::uint8_t>() (idiomatic)
    • all other types → static_cast<std::uint8_t>() (strips sign-extension)
    • Added <cstdint> and <type_traits> includes
for (auto const &byte : data) {
    using value_type = std::remove_cvref_t<decltype(byte)>;
    std::uint8_t b;
    if constexpr (std::is_same_v<value_type, std::byte>) {
        b = std::to_integer<std::uint8_t>(byte);
    } else {
        b = static_cast<std::uint8_t>(byte);
    }
    hash_ ^= b;
    hash_ *= prime();
}
  • fnv_1_tests.cpp / fnv_1a_tests.cpp: Added high_byte_values_test (std::vector<signed char> with values ≥ 0x80, e.g. static_cast<signed char>(0x80) = -128) and std_byte_test (std::vector<std::byte>) — both asserting identical results against known-correct 32-bit and 64-bit reference values. Using signed char directly exercises the sign-extension path the fix guards against.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] [WIP] Address feedback on FNV-1 and FNV-1a hash implementations and tests Fix sign-extension in FNV-1/FNV-1a byte processing; add std::byte support Mar 22, 2026
Copilot AI requested a review from rwindegger March 22, 2026 12:00
Comment thread tests/fnv_1_tests.cpp Outdated
Comment thread tests/fnv_1a_tests.cpp Outdated
…tension fix

Co-authored-by: rwindegger <12802869+rwindegger@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rwindegger/hash23/sessions/6053785c-94b8-4fc9-bf5c-d1cce881240e
@rwindegger

Copy link
Copy Markdown
Owner

@copilot is this pr ready?

Copilot AI commented Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot is this pr ready?

Yes — all review threads are resolved and all CI checks on this branch are green. The PR is ready to merge.

Copilot AI requested a review from rwindegger March 22, 2026 12:11
@rwindegger
rwindegger marked this pull request as ready for review March 22, 2026 12:11
@rwindegger
rwindegger merged commit 894bf3b into feature/fnv_1 Mar 22, 2026
2 checks passed
@rwindegger
rwindegger deleted the copilot/sub-pr-3 branch March 22, 2026 12:12
rwindegger added a commit that referenced this pull request Mar 22, 2026
* Add FNV-1 and FNV-1a hash implementations with corresponding tests

* Refactor FNV-1 and FNV-1a hash implementations to remove buffer usage and improve hash calculation efficiency

* Add string and empty input tests for FNV-1 and FNV-1a hash implementations

* Fix type casting in FNV-1 and FNV-1a hash update methods for byte processing

* Add missing includes for cstddef and string in FNV-1 and FNV-1a headers and tests

* Fix type casting in FNV-1 and FNV-1a hash update methods to use uint8_t

* Fix sign-extension in FNV-1/FNV-1a byte processing; add std::byte support (#4)

* Initial plan

* Handle std::byte via std::to_integer and add high-byte value tests

Co-authored-by: rwindegger <12802869+rwindegger@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rwindegger/hash23/sessions/d5360797-5255-4a5c-894c-557dbe66639c

* Use signed char in high_byte_values_test to directly exercise sign-extension fix

Co-authored-by: rwindegger <12802869+rwindegger@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rwindegger/hash23/sessions/6053785c-94b8-4fc9-bf5c-d1cce881240e

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rwindegger <12802869+rwindegger@users.noreply.github.com>

* Reorder includes and update high byte values in FNV-1 and FNV-1a tests

* Restrict FNV-1 and FNV-1a update method to contiguous ranges of byte-sized types

* Restrict FNV-1 and FNV-1a methods to contiguous ranges of byte-sized types

* Update FNV-1 and FNV-1a methods to use std::ranges::range_value_t for type checks

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rwindegger <12802869+rwindegger@users.noreply.github.com>
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.

2 participants