lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] numeric_io_traits problems, with a patch


From: Vadim Zeitlin
Subject: [lmi] numeric_io_traits problems, with a patch
Date: Sun, 25 Feb 2018 00:26:08 +0100

 Hello,

 While testing compilation with g++ 7 I got the following error:

        In file included from numeric_io_cast.hpp:27:0,
                         from value_cast.hpp:28,
                         from rate_table.cpp:30:
        numeric_io_traits.hpp: In function ‘std::__cxx11::string 
simplify_floating_point(const string&)’:
        numeric_io_traits.hpp:98:19: error: this statement may fall through 
[-Werror=implicit-fallthrough=]
                 case '0': if(++ri != s.rend()) goto loop;
                           ^~
        numeric_io_traits.hpp:99:9: note: here
                 case '.': ++ri;
                 ^~~~
        cc1plus: all warnings being treated as errors

The warning could be corrected by adding [[fallthrough]] for g++7 or
__attribute__((fallthrough)) for the previous gcc versions (or just use the
latter for all of them), but the code actually seems wrong to me because
when 0 is the first character of the string and "ri" is already equal to
"s.rend()" after the first increment, we still increment it again because
of falling through, and the iterator becomes invalid.

 Somehow it still "works" with gcc, even when using -D_GLIBCXX_DEBUG or
-fsanitize=address, but in clang build with -fsanitize=address it does
throw an exception due to the use of invalid iterator from std::string ctor
(which is still unexpected, i.e. I don't understand why doesn't this happen
without address sanitizer, but I didn't spend time on this).

 So the real fix would seem to be to add a "break" statement here, but I'm
really tempted to rewrite this code to avoid the very confusing, IMO, use
of goto with switch, so I've created https://github.com/vadz/lmi/pull/70
instead. If you disagree with this PR, please let me know and I'll add a
"break" instead (or a fallthrough attribute if I'm wrong in my analysis
above).

 Finally, to explain the use of plural in the title, there is another weird
problem in this code: I've also tried using valgrind while testing the bug
above and my fix for it and, somehow, using valgrind results in

        ???? test failed:   '15' == '16'
        [file numeric_io_test.cpp, line 148]

when running this test, while it passes without it. I guess this must be
due to a bug in valgrind itself because I don't see why would this function
call give a different result when running under it otherwise, but it's
going to be difficult to fix it there, so I'd like to find some way of
skipping this test, either only when using valgrind or completely under
Linux. What do you think?

 Thanks in advance,
VZ


reply via email to

[Prev in Thread] Current Thread [Next in Thread]