Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

clang complaint about string comparison



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


  • Moderators

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


  • Lifetime Qt Champion

    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.


  • Moderators

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


  • Lifetime Qt Champion

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



  • @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


  • Moderators

    @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 :(


  • Moderators

    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.



  • @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.



  • 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


Log in to reply