Friday, November 2, 2012

functional and warnings

I like and make heavy use of the "new" functional programming facilities of C++. If you still don't know what function, bind and lambas do, you should really go into it.

Recently I came into an issue when using this kind of paradigm.
Take this simplified code: it contains a stupid, yet subtle bug.

#include <functional>

std::function<bool()> Temp ()
{
     return [] { return true;};
}

int main(int argc, char* argv[])
{
 Temp ();
 return 0;
}

even worse:

int main(int argc, char* argv[])
{
      if (Temp ())
          return 1;
      else
          return 0;
}

if you compile this code with GCC or MSVC , with all the warnings enabled (-Wall , /Wall) , you won't get any warning.

The problem is that the Temp () function call returns a function object, which is not used at all. It's missing an extra () to do the actual function object function call.

int main(int argc, char* argv[])
{
      if (Temp ()())
          return 1;
      else
          return 0;
}

even worst, the function<> class is convertible to bool: it returns true if the function object is valid.
So if you're using it inside a conditional statement you won't get any error too.
The compiler correctly doesn't generate a warning because the statement is a Temp function call; it is just the return value that is unused.

You probably don't want a compiler that complains for every unused function value.
Still, spotting this specific problem can be time-consuming, because the Temp () seems a regular function call and can unnoticed even in a code review.

Probably, a static analizer could catch this kind of errors.
Else, using a more verbose code style can avoid this error.
int main(int argc, char* argv[])
{
        std::function<bool()> RetFn = Temp ();
        assert(RetFn);
        if (RetFn())
            return 1;
        else
            return 0;
}
This is just a simple example, very straight to read and fix. But you'd better not forget a () when doing functional programming or you'll get a problematic code piece that can go unnoticed.
I could not find any idiom to detect this at compile time. Any idea? :)

3 comments:

  1. > it is just the return value that is unused.
    You even stated yourself that this is wrong. The return value is used by calling std::function::operator bool().

    int main(int argc, char* argv[]) {
    Temp ();
    }
    should show a Wunused-return-value, as that time, it actually is unused.

    ReplyDelete
  2. Nice, I didn't know about that GCC extension!

    it requires an attribute __attribute__((warn_unused_result))
    to be added to the function.

    It seems that the MSVC equivalent is the SAL annotation _Check_return_ ,in that case one needs to use the /analyze switch.

    ReplyDelete
    Replies
    1. Testing with GCC, it doesn't seem to work when returning non-empty or templated classes. So, this case is still unhandled.
      There are some GCC bugs here and there related to this is issue.

      Delete