Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix signed/unsigned comparison warning #970

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Feb 4, 2022

After #966 builds with full warnings enabled as errors could fail with the following error:

/miniconda3/envs/cudf_dev/include/rmm/device_buffer.hpp: In member function ‘int64_t rmm::device_buffer::ssize() const’:
/home/jlowe/miniconda3/envs/cudf_dev/include/rmm/device_buffer.hpp:327:19: error: comparison of integer expressions of different signedness: ‘std::size_t’ {aka ‘long unsigned int’} and ‘long int’ [-Werror=sign-compare]
  327 |     assert(size() < std::numeric_limits<int64_t>::max() && "Size overflows signed integer");
      |            ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

This fixes the warning by casting the numerical limit to std::size_t before comparing.

@jlowe jlowe requested a review from a team as a code owner February 4, 2022 18:50
@jlowe jlowe requested review from vyasr and jrhemstad February 4, 2022 18:50
@github-actions github-actions bot added the cpp Pertains to C++ code label Feb 4, 2022
@vyasr vyasr added 3 - Ready for review Ready for review by team 5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working non-breaking Non-breaking change and removed 3 - Ready for review Ready for review by team labels Feb 4, 2022
@vyasr
Copy link
Contributor

vyasr commented Feb 4, 2022

@gpucibot merge

@harrism
Copy link
Member

harrism commented Feb 5, 2022

Ah, sorry. This would only have been a debug build failure, which indeed I didn't test. Thanks @jlowe for fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working cpp Pertains to C++ code non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants