Dead code warning of Coverity Scan on "luabind/detail/format_signature.hpp"

[Wohlstand]

Coverity Scan results a next warning in the luabind/detail/format_signature.hpp file:

140                        lua_pushstring(L, ")");

   assignment: Assigning: sz = luabind::meta::size<luabind::meta::type_list<void, PGE_Texture const *, double, double, float, lua_State *> >::value.

141                        size_t sz = meta::size<Signature>::value;
142                        size_t ncat = sz * 2 + 2;

   const: At condition sz == 1UL, the value of sz must be equal to 6.
   dead_error_condition: The condition sz == 1UL cannot be true.

143                        if(sz == 1)

   CID 1368487 (#15 of 15): Logically dead code (DEADCODE)dead_error_line: Execution cannot reach this statement: ++ncat;.

144                                ++ncat;
145                        lua_concat(L, ncat);

Because the "Signature" type was given by template, it is possible a false positive (code is alive when template has given that "Signature" which will produce result of meta::size<Signature>::value equal to 1).

[decimad]

Yes, I would totally vote for this being a false positive, as it's parametrized for the number of elements (return + this + arguments). I bet the analyzer gets confused by the typelist element counter.

[Wohlstand]

I think I have found a way for a workaround to avoid any similar things and keep logic work: https://stackoverflow.com/questions/11363822/compile-time-conditional-member-function-call-in-c I'll try something at me, and if will success, I'll pull that to you.

[decimad]

Great, I'll stay tuned. Btw, I'm amazed how well the cmake integration works with Visual Studio nowadays! ;)

[Wohlstand]

Something I did, wasn't tested yet that:

        template <size_t S>
        typename std::enable_if<S != 1>::type increase_if_sz_is_not_1(size_t &)
        {
            //Do nothing!
        }

        template <size_t S>
        typename std::enable_if<S == 1>::type increase_if_sz_is_not_1(size_t &ncat)
        {
            ++ncat;
        }

        template <class Signature>
        void format_signature(lua_State* L, char const* function, Signature)
        {
            using first = typename meta::front<Signature>::type;

            type_to_string<first>::get(L);

            lua_pushstring(L, " ");
            lua_pushstring(L, function);

            lua_pushstring(L, "(");
            format_signature_aux(
                L
                , true
                , typename meta::pop_front<Signature>::type()
            );
            lua_pushstring(L, ")");
            const size_t sz = meta::size<Signature>::value;
            size_t ncat = sz * 2 + 2;
            increase_if_sz_is_not_1<sz>(ncat);
            lua_concat(L, ncat);
        }

[decimad]

Hrmm, I feel uneasy adding enable_if tricks for a static analysis. Maybe removing "sz" in favor for duplicate calls to meta::size::value would do the trick already?

[Wohlstand]

The thing is: that meta::size::value is represents as const value which doesn't changes, and logical condition that never uses second way is dead.

Anyway, trick number two:

        template <size_t S>
        typename std::enable_if<S != 1>::type get_ncat()
        {
            return S * 2 + 2;
        }

        template <size_t S>
        typename std::enable_if<S == 1>::type get_ncat()
        {
            return S * 2 + 2 + 1;
        }

        template <class Signature>
        void format_signature(lua_State* L, char const* function, Signature)
        {
            using first = typename meta::front<Signature>::type;

            type_to_string<first>::get(L);

            lua_pushstring(L, " ");
            lua_pushstring(L, function);

            lua_pushstring(L, "(");
            format_signature_aux(
                L
                , true
                , typename meta::pop_front<Signature>::type()
            );
            lua_pushstring(L, ")");
            //const size_t sz = meta::size<Signature>::value;
            //size_t ncat = sz * 2 + 2;
            size_t ncat = get_ncat<meta::size<Signature>::value>();
            lua_concat(L, static_cast<int>(ncat));
        }

P.S. Seems here I did some mistake to use std::enable_if, and I'll fix that

[decimad]

What about something like

ncat + std::conditional_t<meta::size<Signature>::value == 1, std::integral_constant<int, 1>, std::integral_constant<int, 0>>::value

which would maybe make special code unnecessary?

[Wohlstand]

Oh, I totally forgot about constexpr thing:

lua_pushstring(L, ")");
//const size_t sz = meta::size<Signature>::value;
//size_t ncat = sz * 2 + 2;
constexpr size_t ncat = meta::size<Signature>::value * 2 + 2 +
                (meta::size<Signature>::value == 1 ? 1 : 0);
lua_concat(L, static_cast<int>(ncat));

So, because the meta::size::value is constant based on input type, I have to use constexpr that will be processed in compile time

What about something like

ncat + std::conditional_tmeta::size<Signature::value == 1, std::integral_constant<int, 1>, std::integral_constant<int, 0>>::value

which would maybe make special code unnecessary?

I think, also may be usedful, need to also check out that

[decimad]

Right, does Coverity understand constexpr? Since there's still a hidden if/else there in my understanding.

[Wohlstand]

Yes, since it supports C++11 and constexpr is C++11 feature, at my build machine on Travis CI it builds with GCC 6.3. I'll try to push that to my side and if Converity Scan will confirm fixing of that, I'll send change to you

[decimad]

I don't know the inner workings of Coverity and how it relates to GCC. If the constexpr/ternary solution works, then so much better. Though that breaks the backwards compatibility to msvc 2013 I suppose... would anyone still care?

[Wohlstand]

MSVC 2013 is usually still be used in companies who are too lazy to do various updates. MSVC 2013 lacks lots of things are already existing in CLang and GCC, and code that compatible with MSVC 2013 must be C++98, and are rare features of C++11 are existing.

My opinion: MSVC 2015 is the first version which is much more friendly with modern code. I have used the constexpr because I have already found two usages of it in the inheritance.cpp file (and a two another usages I have added to fix "undefined reference" linking failing).

Components of my game engine project are not compatible with MSVCs because of wide C++11/14 code usage, and on Windows, I have used MinGW and MinGW-w64. One small exception is come when I did to be compatible some parts of it (some libraries, and music player), but with MSVC 2015 and higher only.

[decimad]

Well, then I'd probably bump to 2015 requirement and revert to using a std::conditional construct, if somebody objects? I'm running the community edition for this private stuff, so I was always on the "latest" version since its existance.

[Wohlstand]

Okay, Coverity Scan has been confirmed fix:

I'll prepare pull request

[Wohlstand]

Fixed!