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

fixing sign compare error #276

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

gyulamad
Copy link

Before:

./libs/argparse/include/argparse/argparse.hpp: In function ‘std::ostream& argparse::operator<<(std::ostream&, const argparse::Argument&)’:
./libs/argparse/include/argparse/argparse.hpp:686:53: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
686 | while ((pos = argument.m_help.find('\n', prev)) != std::string::npos) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

After:

Build OK

@@ -683,7 +683,7 @@ class Argument {
auto hspace = " "; // minimal space between name and help message
stream << name_stream.str();
std::string_view help_view(argument.m_help);
while ((pos = argument.m_help.find('\n', prev)) != std::string::npos) {
while ((pos = argument.m_help.find('\n', prev)) != (signed)std::string::npos) {
Copy link

Choose a reason for hiding this comment

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

Instead of c-style casting npos to signed I would change auto pos = 0 to size_t pos = 0. The root problem is that "pos" is being auto-typed to an int whereas the return type of string::find() and the type of string::npos are both size_t.

Choose a reason for hiding this comment

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

It also gets auto-typed correctly if you mark the zeros as unsigned:

    auto pos = 0u;
    auto prev = 0u;

Copy link

Choose a reason for hiding this comment

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

I resolved/suppressed the warning by using a static_cast, my minimal reasoning was that the std::string::npos will always be a signed int (of -1) according to the std::string::npos https://cplusplus.com/reference/string/string/npos/
while ((pos = argument.m_help.find('\n', prev)) != static_cast<int>(std::string::npos)) {

@wanjiadenghuo111
Copy link

wanjiadenghuo111 commented Sep 18, 2023 via email

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.

5 participants