Menu
▾
▴
Re: [Tclap-discuss] Sign conversion errors
|
From: Daniel A. <dan...@gm...> - 2020-04-15 12:47:51
|
I'm aware of these, I think VS also emits these warnings. I've considered
fixing these multiple times over the years (they are all OK btw) but in
some places changing to unsigned may actually lead to *more* bugs due to
potential wrap-around issues (i.e., where the code expects that results can
be negative) - so some care needs to be taken. In general use of unsigned
ints should be discouraged when the only purpose is to signal that a value
is never less than zero and I believe most folks these days agree that it
was a mistake to use unsigned ints in STL containers.
That said, in some places the fix is actually trivial and should probably
be done (it's just that nobody except me has cared about it so far) - so if
you want to send some patches to fix these, I'm happy to review.
For example, this is just ridiculous:
inline void Arg::trimFlag(std::string& flag, std::string& value) const
{
int stop = 0;
for ( int i = 0; static_cast<unsigned int>(i) < flag.length(); i++ )
if ( flag[i] == Arg::delimiter() )
...
On Mon, Apr 13, 2020 at 3:23 AM Aaron Boxer <bo...@gm...> wrote:
> Hello!
>
> I am using tclap in an image compression project, where sign conversion
> can lead to subtle bugs, so I turn on "-Wall -Wconversion". I also build
> with clang, which seems to catch more warnings than gcc.
>
> I see a number of conversion warnings in tclap, for example:
>
>
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> /home/aaron/src/grok/src/include/tclap/Arg.h:563:18: warning: implicit
> conversion changes signedness: 'int' to 'std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::size_type' (aka 'unsigned
> long') [-Wsign-conversion]
> if (flag[i] == Arg::delimiter()) {
> ~~~~ ^
> /home/aaron/src/grok/src/include/tclap/Arg.h:569:34: warning: implicit
> conversion changes signedness: 'int' to 'std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::size_type' (aka 'unsigned
> long') [-Wsign-conversion]
> value = flag.substr(stop + 1);
> ~~~~~~ ~~~~~^~~
> /home/aaron/src/grok/src/include/tclap/Arg.h:570:31: warning: implicit
> conversion changes signedness: 'int' to 'std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::size_type' (aka 'unsigned
> long') [-Wsign-conversion]
> flag = flag.substr(0, stop);
> ~~~~~~ ^~~~
> /home/aaron/src/grok/src/include/tclap/Arg.h:579:15: warning: implicit
> conversion changes signedness: 'int' to 'std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::size_type' (aka 'unsigned
> long') [-Wsign-conversion]
> if (s[i] == Arg::blankChar()) return true;
> ~ ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:28:
> /home/aaron/src/grok/src/include/tclap/SwitchArg.h:204:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (argMatches(args[*i])) {
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/SwitchArg.h:206:23: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _setBy = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/SwitchArg.h:210:43: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> } else if (combinedSwitchesMatch(args[*i])) {
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/SwitchArg.h:214:40: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (combinedSwitchesMatch(args[*i]))
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/SwitchArg.h:222:34: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> return lastCombined(args[*i]);
> ~~~~ ^~
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:29:
> /home/aaron/src/grok/src/include/tclap/MultiSwitchArg.h:132:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (argMatches(args[*i])) {
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/MultiSwitchArg.h:135:23: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _setBy = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/MultiSwitchArg.h:143:43: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> } else if (combinedSwitchesMatch(args[*i])) {
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/MultiSwitchArg.h:151:43: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> while (combinedSwitchesMatch(args[*i])) ++_value;
> ~~~~ ^~
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:31:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledMultiArg.h:27:
> /home/aaron/src/grok/src/include/tclap/MultiArg.h:271:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:38:
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:441:30: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> std::string indentString(indentSpaces, ' ');
> ~~~~~~~~~~~~ ^~~~~~~~~~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:448:44: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> os << indentString << s.substr(from) << std::endl;
> ~~~~~~ ^~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:459:53: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> tooFar = s.find_first_of(splitChars, to + 1);
> ~~~~~~~~~~~~~ ~~~^~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:468:15: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> if (s[to] != ' ') {
> ~ ^~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:473:40: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> os << indentString << s.substr(from, to - from) << '\n';
> ~~~~~~ ^~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:473:49: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> os << indentString << s.substr(from, to - from) << '\n';
> ~~~~~~ ~~~^~~~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:476:18: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> for (; s[to] == ' '; to++) {}
> ~ ^~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:481:53: warning:
> implicit conversion changes signedness: 'int' to
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') [-Wsign-conversion]
> indentString.insert(indentString.end(), secondLineOffset, ' ');
> ~~~~~~ ^~~~~~~~~~~~~~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:444:17: warning:
> implicit conversion loses integer precision:
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') to 'int'
> [-Wshorten-64-to-32]
> int end = s.length();
> ~~~ ~~^~~~~~~~
> /home/aaron/src/grok/src/include/tclap/StdOutput.h:459:24: warning:
> implicit conversion loses integer precision:
> 'std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::size_type' (aka 'unsigned long') to 'int'
> [-Wshorten-64-to-32]
> tooFar = s.find_first_of(splitChars, to + 1);
> ~ ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> /home/aaron/src/grok/src/include/tclap/CmdLine.h:488:49: warning: implicit
> conversion changes signedness: 'int' to 'std::vector::size_type' (aka
> 'unsigned long') [-Wsign-conversion]
> if (!matched && _emptyCombined(args[i])) matched = true;
> ~~~~ ^
> /home/aaron/src/grok/src/include/tclap/CmdLine.h:494:26: warning: implicit
> conversion changes signedness: 'int' to 'std::vector::size_type' (aka
> 'unsigned long') [-Wsign-conversion]
> args[i]));
> ~~~~ ^
> /home/aaron/src/grok/src/include/tclap/CmdLine.h:543:15: warning: implicit
> conversion changes signedness: 'int' to 'std::__cxx11::basic_string<char,
> std::char_traits<char>, std::allocator<char> >::size_type' (aka 'unsigned
> long') [-Wsign-conversion]
> if (s[i] != Arg::blankChar()) return false;
> ~ ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:289:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (_hasBlanks(args[*i])) return false;
> ~~~~ ^~
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:579:22: note: in
> instantiation of member function 'TCLAP::ValueArg<unsigned
> int>::processArg' requested here
> ValueArg<uint32_t> kernelBuildOptionsArg("k",
> "KernelBuild",
> ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:308:36: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _extractValue(args[*i]);
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:289:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (_hasBlanks(args[*i])) return false;
> ~~~~ ^~
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:586:22: note: in
> instantiation of member function 'TCLAP::ValueArg<unsigned
> short>::processArg' requested here
> ValueArg<uint16_t> rsizArg("Z", "RSIZ", "RSIZ", false, 0U,
> ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:308:36: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _extractValue(args[*i]);
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:289:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (_hasBlanks(args[*i])) return false;
> ~~~~ ^~
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:595:20: note: in
> instantiation of member function
> 'TCLAP::ValueArg<std::__cxx11::basic_string<char> >::processArg' requested
> here
> ValueArg<string> IMFArg("z", "IMF", "IMF profile", false,
> "", "string",
> ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:308:36: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _extractValue(args[*i]);
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:289:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (_hasBlanks(args[*i])) return false;
> ~~~~ ^~
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:609:21: note: in
> instantiation of member function 'TCLAP::ValueArg<int>::processArg'
> requested here
> ValueArg<int32_t> deviceIdArg("G", "DeviceId", "Device
> ID", false, 0U,
> ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:308:36: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _extractValue(args[*i]);
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:289:25: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> if (_hasBlanks(args[*i])) return false;
> ~~~~ ^~
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:627:21: note: in
> instantiation of member function 'TCLAP::ValueArg<unsigned
> char>::processArg' requested here
> ValueArg<uint8_t> tpArg("u", "TP", "Tile part generation",
> false, 0U,
> ^
> In file included from
> /home/aaron/src/grok/src/bin/jp2/grk_compress.cpp:102:
> In file included from /home/aaron/src/grok/src/include/tclap/CmdLine.h:30:
> In file included from
> /home/aaron/src/grok/src/include/tclap/UnlabeledValueArg.h:28:
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:291:29: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> std::string flag = args[*i];
> ~~~~ ^~
> /home/aaron/src/grok/src/include/tclap/ValueArg.h:308:36: warning:
> implicit conversion changes signedness: 'int' to 'std::vector::size_type'
> (aka 'unsigned long') [-Wsign-conversion]
> _extractValue(args[*i]);
>
>
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> I think this could solved by using size_t instead of int in a number of
> places.
>
> Interested to know your thoughts,
> Aaron
>
>
>
>
> _______________________________________________
> Tclap-discuss mailing list
> Tcl...@li...
> https://lists.sourceforge.net/lists/listinfo/tclap-discuss
>
|