Skip to content

Conversation

@stephanlachnit
Copy link
Contributor

On GCC with -Werror=sign-compare, using atomic_queue gives the following error:

../subprojects/atomic_queue-1.6.6/include/atomic_queue/atomic_queue.h:337:27: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
  337 |         return was_size() >= static_cast<int>(static_cast<Derived const&>(*this).size_);
      |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't see any good reason why the size should be cast to an integer. In fact, the second expression is just the capacity anyway, so let's use the capacity instead. Then both expressions use unsigned for a well defined comparison.

@max0x7ba
Copy link
Owner

max0x7ba commented May 1, 2025

On GCC with -Werror=sign-compare, using atomic_queue gives the following error:

../subprojects/atomic_queue-1.6.6/include/atomic_queue/atomic_queue.h:337:27: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
  337 |         return was_size() >= static_cast<int>(static_cast<Derived const&>(*this).size_);
      |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't see any good reason why the size should be cast to an integer. In fact, the second expression is just the capacity anyway, so let's use the capacity instead. Then both expressions use unsigned for a well defined comparison.

You are right.

This is a remnant of an original expression where the left signed term got replaced with unsigned was_size().

@max0x7ba max0x7ba merged commit 78d5964 into max0x7ba:master May 1, 2025
6 checks passed
@max0x7ba
Copy link
Owner

max0x7ba commented May 2, 2025

This change is available in tag https://github.com/max0x7ba/atomic_queue/releases/tag/v1.6.7

@stephanlachnit stephanlachnit deleted the p-fix-w-sign-compare branch May 2, 2025 08:44
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