Search code examples
c++compiler-warnings

Count in substring - fix compiler warnings


The count_occurences code finds the number of occurrences of character c in the string between positions start (included) and end (excluded). E.g. the following code counts , in ,b, (substring of a,b,c from 1 to 4), which is 2.

#include <iostream>
#include <algorithm>

size_t count_occurences(const std::string& str, char c, size_t start, size_t end) {
    return std::count(str.begin() + start, str.begin() + end, c);
}

int main() {
    std::string str = "a,b,c";
    size_t start = 1, end = 4;
    std::cout << count_occurences(str, ',', start, end);
    return 0;
}

When compiled with clang++ -Wconversion, I get the following warnings:

a.cpp:5:12: warning: implicit conversion changes signedness: '_Iter_diff_t<_String_const_iterator<_String_val<_Simple_types<char>>>>' (aka 'long long') to 'size_t' (aka 'unsigned long long') [-Wsign-conversion]
    5 |     return std::count(str.begin() + start, str.begin() + end, c);
      |     ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
a.cpp:5:58: warning: implicit conversion changes signedness: 'size_t' (aka 'unsigned long long') to 'difference_type' (aka 'long long') [-Wsign-conversion]
    5 |     return std::count(str.begin() + start, str.begin() + end, c);
      |                                                        ~ ^~~
a.cpp:5:37: warning: implicit conversion changes signedness: 'size_t' (aka 'unsigned long long') to 'difference_type' (aka 'long long') [-Wsign-conversion]
    5 |     return std::count(str.begin() + start, str.begin() + end, c);
      |                                   ~ ^~~~~

What is the proper way to fix these warnings without changing the signature of count_occurences? I can of course use static_cast everywhere, but it's more about silencing the warning than fixing it.

(In the real code, start and end are fields, so the function only has 2 parameters.)


Solution

  • You could silence these warnings individually for the function, but it's not worth the effort:

    #pragma clang diagnostic push
    #pragma clang diagnostic ignored "-Wconversion"
    size_t count_occurences(const std::string& str, char c, size_t start, size_t end) {
        return std::count(str.begin() + start, str.begin() + end, c);
    }
    #pragma clang diagnostic pop
    

    Note that GCC would give you warnings about unrecognized pragmas, so you'd actually need a lot more than just this. _Pragma could help you minimize this somewhat, but it's still much more complicated than just using static_cast:

    size_t count_occurences(const std::string& str, char c, size_t start, size_t end) {
        // strictly speaking, the right type is std::string::difference_type,
        // not ptrdiff_t
        return std::count(str.begin() + static_cast<ptrdiff_t>(start),
                          str.begin() + static_cast<ptrdiff_t>(end),
                          c);
    }
    

    This is still the most self-documenting solution. It may not be a satisfying answer, but it is the right way.

    The underlying problem is that iterators usually require a signed integer such as ptrdiff_t and you're doing math with an unsigned integer. Your warning isn't a false positive, and static_cast would document that you're aware of these type difference and knowingly convert.

    Alternative solutions in this specific case

    @TimRoberts has also pointed out that you could write

    std::count(&str[start], &str[end], c);
    

    This solution only works for contiguous containers, but it is more compact than static_cast.*

    Yet another solution presents itself in C++17:

    size_t count_occurences(std::string_view str, char c) {
        return std::count(str.begin(), str.end(), c);
    }
    

    The user can simply call count(my_view.substr(...), 'c') if they want a specific start and end.