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?

Unrelated, but because you're passing `value` by-value to your function, the `tranformValue` local is unneeded. You already have a temp copy in `value`. You can forego the `const` on the parameter and use `value` in your code instead of `tranformValue` (though I would recc keeping it as-is but making the param a const-ref). — WhozCraig, Aug 09 '13 at 10:56
Maybe it wants you to copy the `begin()` and `end()` iterators, and pass the copies to `std::transform`. — juanchopanza, Aug 09 '13 at 10:56
If you only want an case insensitive compare look here for a lot of answers http://stackoverflow.com/questions/11635/case-insensitive-string-comparison-in-c — Jan Herrmann, Aug 09 '13 at 11:04
@WhozCraig: In this particular case, pass by-value is better than pass by ref-to-const. See [here](http://stackoverflow.com/a/7592741/1137388). Otherwise, I agree with everything else that you said. — Cassio Neri, Aug 09 '13 at 11:10
@CassioNeri yeah, I concur. Its a consistence thing with me, hard habit to break when it so-often is done by const-ref. Agreed. — WhozCraig, Aug 09 '13 at 11:15

Cassio Neri · Accepted Answer · 2013-08-09T11:27:25.603

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).

Whats difference between using begin() end() and cbegin() and cbegin() cend()? Because that's all it takes to make it happy: std::transform(transformValue.cbegin(), transformValue.cend(), transformValue.cbegin(), &::tolower); — MathiasWestin, Aug 09 '13 at 11:09
@MathiasWestin `cbegin()` returns a `const_iterator`. Frankly I'm shocked your output iterator as a const-iterator even *compiles*. Are you sure that isn't just `begin()` in your code (the third param)? — WhozCraig, Aug 09 '13 at 11:17
@WhozCraig Your are right it doesen't compile, I was happy that PCLint didn't throw any errors! :) So it has to be the way you show in the sample above. — MathiasWestin, Aug 09 '13 at 11:25

score 2 · Answer 2 · answered Aug 09 '13 at 12:14

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).

score 0 · Answer 3 · answered Jul 02 '16 at 14:00

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.

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

3 Answers3