Menu
▾
▴
Re: [Tclap-discuss] Sign conversion errors
|
From: Aaron B. <bo...@gm...> - 2020-04-15 14:47:04
|
On Wed, Apr 15, 2020 at 8:47 AM Daniel Aarno <dan...@gm...> wrote:
> 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() )
>
> ...
>
>
Yes, exactly. I have a patch here:
https://github.com/boxerab/tclap/tree/warnings
By the way, I would highly recommend moving the project to Github or Gitlab.
Workflow is so much nicer, and you don't get bombarded by SF ads.
I was very hesitiant to register with SF discussion list.
It would probably only take a few hours to sign up and migrate the repo.
Not sure about issue migration.
Just my 2 cents :)
> 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
>>
>
|