Skip to content

Conversation

@andersfugmann
Copy link
Owner

  • Extend tests for large zigzag encoded integers
  • Fix zigzag encoding for large integers

Closes #45

@andersfugmann andersfugmann requested a review from Copilot August 6, 2025 11:42

This comment was marked as outdated.

@andersfugmann andersfugmann force-pushed the andersfugmann/fix_zigzag_decoding branch from 3f3aad4 to e8f9e48 Compare August 6, 2025 12:31
@andersfugmann andersfugmann requested a review from Copilot August 6, 2025 12:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes zigzag decoding for large integers by correcting the division operation and extends test coverage for edge cases. The fix addresses issue #45 by replacing integer division with logical right shift operations in zigzag decoding functions.

  • Replace integer division (/) with logical right shift (lsr) in zigzag decoding functions
  • Add comprehensive test coverage for extreme integer values (min/max integers)
  • Update expect tests to include new test cases for better edge case coverage

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/ocaml_protoc_plugin/deserialize.ml Fixes zigzag decoding logic by using logical right shift instead of division and adds comprehensive test cases
test/int_types_native_test.ml Extends test coverage to include minimum and maximum integer values for both 32-bit and 64-bit signed integers
src/ocaml_protoc_plugin/serialize.ml Minor formatting fix to expect test output

Repository owner deleted a comment from Copilot AI Aug 6, 2025
@andersfugmann andersfugmann force-pushed the andersfugmann/fix_zigzag_decoding branch from 9a4b273 to 7d742e6 Compare August 28, 2025 18:49
@andersfugmann andersfugmann merged commit f9e8a66 into main Aug 28, 2025
3 checks passed
@andersfugmann andersfugmann deleted the andersfugmann/fix_zigzag_decoding branch August 28, 2025 19:38
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.

OCaml Protoc Plugin incorretly parses Int64.min_int in sint64 field

1 participant