How to use std::transform without violating MISRA C++ 2008 Advisory Rule 5-2-10?

Question

I get these error in PC-Lint (au-misra-cpp.lnt):

ConverterUtil.cpp(90): error 864: (Info -- Expression involving variable 'transformValue' possibly depends on order of evaluation [MISRA C++ Rule 5-2-10])

ConverterUtil.cpp(90): error 864: (Info -- Expression involving variable 'transformValue' possibly depends on order of evaluation [MISRA C++ Rule 5-2-10])

ConverterUtil.cpp(90): error 534: (Warning -- Ignoring return value of function 'std::transform(std::_String_iterator>>, std::_String_iterator>>, std::_String_iterator>>, int (*)(int))' (compare with line 998, file C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\include\algorithm) [MISRA C++ Rules 0-1-7 and 8-4-6], [MISRA C++ Rule 0-3-2])

On this code:

/**Conversion from std::string to bool*/
bool ConverterUtil::ConvertStdStringToBool(const std::string value)
{
    std::string transformValue = value;
    bool retValue = false;

    std::transform(transformValue.begin(), transformValue.end(), transformValue.begin(), &::tolower);


    if(transformValue == std::string(static_cast<const char *>("true")))
    {
        retValue = true;
    }

    return retValue;
}

I guessed it didn't like the fact that I use the same std::string as input and output in the transform, but using an other string as output gives the same error.

Is it possible to make the std::transform MISRA compliant?

Answer 1

I'm just guessing here (and I'll probably remove the answer if it doesn't solve your problem).

Try to replace the line containning std::transform with these two:

auto dest = transformValue.begin();
std::transform(transformValue.cbegin(), transformValue.cend(), dest, &::tolower);

Notice the use of cbegin() and cend() (rather than begin() and end()).

On another topic: You're copying the string passed to ConvertStdStringToBool twice when you could do it just once. To do this, replace:

bool ConverterUtil::ConvertStdStringToBool(const std::string value)
{
    std::string transformValue = value;

with

bool ConverterUtil::ConvertStdStringToBool(std::string transformValue)
{

(You might want to rename transformValue to value after this change).

Update: My explanation why I think it's going to help.

First of all, notice that transformValue is non const. Hence, transformValue.begin() and transformValue.end() will call these overloads:

iterator begin(); // non const overload
iterator end();   // non const overload

Hence the static analyser (rightly) concludes that begin() and end() might change the state of transformValue. In this case, the final state of transformValue might depend on which of begin() and end() is called first.

Now, when you call cbegin() and cend(), the overloads are these:

const_iterator cbegin() const; // notice the const 
const_iterator cend() const;   // notice the const

In this case, the static analyser will not deduce that these calls will change the state of transformValue and doesn't raise the issue. (Strictly speaking even though the methods are const they could change the state because there could exist mutable data members in the class or the methods could use an evil const_cast. IMHO, the static analyser shouldn't be blamed for that.)

Final remark: The call

std::transform(transformValue.cbegin(), transformValue.cend(), transformValue.cbegin(), &::tolower);
                                                                              ^^^^^^

is wrong. The third argument must be the non const iterator, that is, it must be transformValue.begin() (only the first two arguments are the c* methods).

However, I guess, for a similar reasoning as above, just using transformValue.begin() as the third argument won't be enough and that's why I suggested creating another variable (dest).

Answer 2

This isn't a separate answer, more like a comment on Cassio's answer, but it's too long for a comment.

Here's how directly using transformValue.begin() as the third parameter could actually easily fail: in C++03 (not in 11, but GCC so far hasn't switched), a reference-counted implementation of std::string was allowed. libstdc++ has one. With such a version, value and transformValue would share their internal buffer.

Now, when transformValue.begin() is called, the resulting non-const iterator can be used to modify the buffer, which would be bad because it would change value too. So begin() has to unshare the buffer, i.e. allocate a unique buffer just for transformValue. Doing so invalidates all existing iterators!

Thus, in the C++98 version of calling cbegin and cend:

const std::string& constValue = transformValue;
std::transform(constValue.begin(), constValue.end(),
               transformValue.begin(), &::tolower);

you have got a real order dependency. If the non-const version of begin() is called before the const calls, everything is fine. But if the const version (or end()) is called first, the newly returned iterators will be invalidated by the non-const call, yielding undefined behavior.

Nastily enough, the code will still probably work, because the invalidated iterators would point to the old buffer, which is kept alive by value. Since in the majority of cases where such unsharing happens, the old copy will be around longer than the new copy, this is a really nasty sleeper bug that won't surface until either

• the old copy can disappear during the operation, e.g. because the old copy already went away, but another copy exists on another thread and could disappear at any time, or
• the code is changed in a way that expects the changes made through the non-const iterator to be reflected in the const iterators (which point to a different buffer and thus won't see them).

Answer 3

Although the iterator begin() and end() are constant in this call, the checker suspects side effects from calling these. So the result might be different depending on the sequence of calls. I had a similar problem and had to fix it by using two local variables to shut up the checker.