clang complaint about string comparison

koahnig · 7 Aug 2018, 06:08

I am still getting used to clang model in new creator version

Today I stumbled over where str is std::string.

//    if ( str.substr( 0, 9 ) == "Name week")
    if ( ! strncmp ( str.c_str(), "Name week", 9 ) )

the commented line got the complaint this is not defined and one should use "strncmp". I am not really a C++ purist and can accept things when working.
However, that construct was not really going down well and I used google to find a bit of explanation. std::string has even its own compare function since c++98 standard.

Personally I try to get away from pure C and tend to go for C++.

Therefore a couple of questions thrown to the experts:
Why the recommendation is to strncmp and not string::compare?
Is this because std::string is standard template and not Qt?
Also does one know why the direct comparison is not a good choice? At first sight possible ambiguities are not obvious.

Also the clang modeler is a mighty tool, I am certainly impressed. However, when changing to strncmp it did not tell that the appropriate include is missing. ;)

ambershark · wrote on 7 Aug 2018, 00:23

I can't answer all the questions you have but I can say you should definitely be using string::compare if you using std::string. I look at the strncmp as incorrect. It's odd that the clang modeler is viewing it as wrong. I would say that this is just a bug in the modeler more than a reason to switch.

Personally I try to get away from pure C and tend to go for C++.

If you are being a C++ purist you don't want to use C string compare functions for sure. Now-a-days People that learn C++ from scratch probably don't even know the old C functions since STL has all that for them now. C and C++ aren't as mixed as they used to be with the C++98/11/17/etc standards.

Also does one know why the direct comparison is not a good choice?

I think it is the good/proper choice. :)

aha_1980 · 8 Jul 2018, 06:13

hi all,

just a guess: the second parameter is a string constant, and it may be cheaper to convert with c_str() (effective a pointer) than converting the constant to std::string.

that all is given there is no const char * overload for compare (which I haven't checked).

Edit: or it is rather the substr() which is not cheap. does it allocate?

Edit2: try compare() as @ambershark suggested. it shoul work best.

J.Hilk · 7 Aug 2018, 06:42

@koahnig what version of QtCreator are we talking about, the 4.7rc ?
Because with 4.6.2 , this:

std::string str("Name week blablabla");
    if ( str.substr( 0, 9 ) == "Name week")
        qDebug() << "is substr";

does not create a warning for me.

aha_1980 · 7 Aug 2018, 06:45

@J.Hilk Do you have Clang Code Model enabled?

koahnig · J J.Hilk 7 Aug 2018, 06:08

@J.Hilk said in clang complaint about string comparison:

@koahnig what version of QtCreator are we talking about, the 4.7rc ?
Because with 4.6.2 , this:
std::string str("Name week blablabla");
    if ( str.substr( 0, 9 ) == "Name week")
        qDebug() << "is substr";
does not create a warning for me.

I have already upgraded to 4.7.0 as soon as it came out. I doubt that you see it with 4.6.2

J.Hilk · wrote on 7 Aug 2018, 06:45

@aha_1980
oh, you're right, Iassumed it was a new default feature, but it's on all my PCs disabled because the corresponding plugin is not loaded

Well, I'm out of the discussion than :(

J.Hilk · 7 Aug 2018, 09:01

I didn't realize, that 4.7 was released, its still in my Preview in the maintenance repo, but I was able to update on my macos device.

So, under what diagnostic configuration das this accure? For me, it's by default set to Clang-only checks for almost everything [build-in] and I don't have a warning with the above mentioned code.

koahnig · J J.Hilk 7 Aug 2018, 06:57

@J.Hilk

That is basically the set I am using as well. The only difference that I added a setting for avoiding the qDebug() warning issue in release mode encountered with Qt 5.4.

Some times you have to wait a moment for warnings to show.

koahnig · 8 Jul 2018, 09:33

Basically I was a bit thrown off by the recommendation to use old c-style while in other cases I am receiving warnings when I am using c-style casting. The discussion has been started to back up by bearings. Sometimes one (I) tend to stick to something ineffective.

std::string:compare has even in one version the requested substr part integrated.

Thanks to all for reponses