Skip to content

avoid some implicit conversions#270

Closed
firewave wants to merge 3 commits intocppcheck-opensource:masterfrom
firewave:implicit
Closed

avoid some implicit conversions#270
firewave wants to merge 3 commits intocppcheck-opensource:masterfrom
firewave:implicit

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Aug 29, 2022

This does not change any types. It just makes implicit conversions explicit.

Here's the warnings in question for reference:

/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:401:76: warning: implicit conversion loses integer precision: 'int' to 'const unsigned short' [-Wimplicit-int-conversion]
        const unsigned short bom = (static_cast<unsigned char>(istr.get()) << 8);
                             ~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:541:55: warning: implicit conversion loses integer precision: 'long' to 'unsigned int' [-Wshorten-64-to-32]
                    lineDirective(location.fileIndex, std::atol(numtok->str().c_str()), &location);
                    ~~~~~~~~~~~~~                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:550:35: warning: implicit conversion loses integer precision: 'long' to 'unsigned int' [-Wshorten-64-to-32]
                                  std::atol(numtok->str().c_str()), &location);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:780:20: warning: implicit conversion loses integer precision: 'int' to 'const char' [-Wimplicit-int-conversion]
    const char c = std::tolower(tok->str()[0]);
               ~   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:1266:25: warning: implicit conversion loses integer precision: 'unsigned long' to 'unsigned int' [-Wshorten-64-to-32]
    return files.size() - 1U;
    ~~~~~~ ~~~~~~~~~~~~~^~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:2101:35: warning: implicit conversion loses integer precision: 'int' to 'char' [-Wimplicit-int-conversion]
                    windowsPath = toupper(driveLetter);
                                ~ ^~~~~~~~~~~~~~~~~~~~
/mnt/s/GitHub/simplecpp-fw/simplecpp.cpp:2105:35: warning: implicit conversion loses integer precision: 'int' to 'char' [-Wimplicit-int-conversion]
                    windowsPath = toupper(driveLetter);
                                ~ ^~~~~~~~~~~~~~~~~~~~

And an example for the warnings with the assertions:

/mnt/s/GitHub/simplecpp-fw/test.cpp:1845:29: warning: implicit conversion loses integer precision: 'std::vector::size_type' (aka 'unsigned long') to 'const unsigned int' [-Wshorten-64-to-32]
    ASSERT_EQUALS(1U, files.size());
    ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~

Comment thread simplecpp.cpp
// The UTF-16 BOM is 0xfffe or 0xfeff.
if (ch1 >= 0xfe) {
const unsigned short bom = (static_cast<unsigned char>(istr.get()) << 8);
const unsigned short bom = static_cast<unsigned short>((static_cast<unsigned char>(istr.get()) << 8));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see that this change improves the readability or makes the code more robust.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It makes the implicit conversions explicit so you know what actually happens. If possible the code should be changed to avoid these. Since those might not be straight-forward I did not attempt this yet.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I just don't agree. The code is longer and I don't see that it makes the code easier. On the contrary..

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The original code is shorter and 100% safe. I consider it a false positive from the compiler to warn about this. There can be no loss of precision.

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