Skip to content

Fixed casting warning in benchmark_uncompress on MSVC#2035

Merged
Dead2 merged 1 commit intozlib-ng:developfrom
nmoinvaz:fixes/benchmark-cast
Dec 3, 2025
Merged

Fixed casting warning in benchmark_uncompress on MSVC#2035
Dead2 merged 1 commit intozlib-ng:developfrom
nmoinvaz:fixes/benchmark-cast

Conversation

@nmoinvaz
Copy link
Copy Markdown
Member

@nmoinvaz nmoinvaz commented Dec 3, 2025

benchmark_uncompress.cc(55,93): warning C4244: 'argument': conversion from 'int64_t' to 'size_t', possible loss of data

@nmoinvaz
Fixed casting warning in benchmark_uncompress on MSVC
5ea8fec 
benchmark_uncompress.cc(55,93): warning C4244: 'argument': conversion from 'int64_t' to 'size_t', possible loss of data
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Modified the type of the sizes array in the uncompress_bench class from int64_t to size_t. Initialization values and all benchmark logic remain unchanged. This is a type signature update for test configuration data.

Changes

Cohort / File(s) Summary
Type signature update
test/benchmarks/benchmark_uncompress.cc		 Changed sizes array element type from int64_t to size_t in uncompress_bench class

Possibly related PRs

Suggested reviewers

  • Dead2

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately describes the main change: fixing a casting warning in benchmark_uncompress on MSVC.
Description check ✅ Passed The description provides the specific compiler warning that motivated the fix, which relates to the changeset of converting int64_t to size_t.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ceef7bf and 5ea8fec.

📒 Files selected for processing (1)
  • test/benchmarks/benchmark_uncompress.cc (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: samrussell
Repo: zlib-ng/zlib-ng PR: 1914
File: arch/generic/crc32_chorba_c.c:30-32
Timestamp: 2025-05-13T06:56:12.407Z
Learning: In zlib-ng, pointer casts between z_word_t* and uint64_t* are safe because: 1) On 64-bit platforms, z_word_t is defined as uint64_t, making the cast a no-op; 2) On 32-bit platforms where z_word_t is uint32_t, the code paths using uint64_t* are skipped in favor of 32-bit specific implementations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
  • GitHub Check: EL10 GCC S390X DFLTCC UBSAN
  • GitHub Check: Ubuntu GCC Compat Symbol Prefix
  • GitHub Check: Ubuntu GCC ASAN
🔇 Additional comments (2)
test/benchmarks/benchmark_uncompress.cc (2)

30-30: LGTM! Appropriate type for size values.

Changing from int64_t to size_t is correct and fixes the MSVC warning at line 55. The size_t type is the standard type for representing sizes in C/C++, and all values are within valid range.

69-69: Verify no new signed/unsigned comparison warning introduced.

After changing sizes to size_t, this line now compares size_t (unsigned) with int64_t (signed) from state.range(0). While functionally safe given the positive values, some compilers may warn about signed/unsigned mismatch.

Please confirm that MSVC doesn't produce a new warning at this location. If a warning does appear, consider adding an explicit cast for clarity:

-while (sizes[index] != state.range(0)) ++index;
+while (static_cast<int64_t>(sizes[index]) != state.range(0)) ++index;

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 3, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.69%. Comparing base (ceef7bf) to head (5ea8fec).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff              @@
##           develop    #2035       +/-   ##
============================================
+ Coverage         0   79.69%   +79.69%     
============================================
  Files            0      162      +162     
  Lines            0    12883    +12883     
  Branches         0     3174     +3174     
============================================
+ Hits             0    10267    +10267     
- Misses           0     1589     +1589     
- Partials         0     1027     +1027

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Dead2 Dead2 merged commit 909d4e2 into zlib-ng:develop Dec 3, 2025
156 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 20, 2025
Merged
@Dead2 Dead2 mentioned this pull request Jan 30, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Warnings fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nmoinvaz @Dead2