Compiler error with std::reference_wrapper and types in C++

Question

I've been diving into C++ lately, and I've been giving STL a go. I'm encountering a problem that I cannot seem to solve. I'm guessing it involves me not understanding how C++ deduces types, but I'm not sure. Here is the code that works

template <typename T>
auto most_frequent_element(const std::vector<std::reference_wrapper<T>> &vector) -> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
    if (vector.empty()) {
        return{};
    }
    std::map<std::reference_wrapper<T>, size_t> individual_counts = {};
    std::for_each(vector.begin(), vector.end(), [&individual_counts](auto &element) {
        auto result = individual_counts.emplace(element, 0);
        (*result.first).second++;
     });
    return *std::max_element(individual_counts.begin(), individual_counts.end(), [](auto &a, auto &b) {
        return a.second < b.second;
    });
}

And this is how I might call it

auto main(int argc, char *argv[]) -> int {
    std::vector<char> test = { 'a', 'a', 'b', 'b', 'c'};
    auto result = most_frequent_element(std::vector<std::reference_wrapper<char>>(test.begin(), test.end()));
    if (result) {
        std::cout << (*result).first << " " << (*result).second << std::endl;
    }
    else {
        std::cout << "Empty input list." << std::endl;
    }
    return EXIT_SUCCESS;
}

So that works. But if I create a simple method to eliminate the need to copy the vector manualy, like so

template <typename T>
auto most_frequent_element_2(const std::vector<T> &vector) -> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
    most_frequent_element(std::vector<std::reference_wrapper<T>>(vector.begin(), vector.end()));
}

and call it like this

auto main(int argc, char *argv[]) -> int {
    std::vector<char> test = { 'a', 'a', 'b', 'b', 'c'};
    auto result = most_frequent_element_2(test);
    if (result) {
        std::cout << (*result).first << " " << (*result).second << std::endl;
    }
    else {
        std::cout << "Empty input list." << std::endl;
    }
    return EXIT_SUCCESS;
}

I get the following error

Error C2664 'std::reference_wrapper<char>::reference_wrapper(std::reference_wrapper<char> &&)': cannot convert argument 1 from 'const char' to 'char &'

I'm confused because how I see it they should do the same thing. If I am wrong can someone point me in the right direction.

P.S. I'm working in Visual Studio 2015

Update: Added const constraint to return type of functions to correctly reflect the fact that the *max_element returns a const key. (suggested by @dyp)

As far as I can see, this is a constness issue: the `most_frequent_element_2` functions has a const reference to a vector, so the elements are const when iterating (`const_iterator`s). However, the `reference_wrapper` is a `reference_wrapper`, so it requires non-const elements for its initialization. — dyp, Sep 11 '15 at 23:45
Additionally, there's a constness issue with the return types, which might be due to `boost::optional` being slightly restrictive with its implicit constructors. That is, clang++ rejects your original code because the `*std::max_element` yields a `pair, size_t>` since the key of a map node is const. — dyp, Sep 11 '15 at 23:49
This is why I feel confused. When i try to add const-ness I stumble upon the fact that the C++ Standard forbids containers of const elements. — hitripekac, Sep 12 '15 at 00:01
So, one solution is to remove the const constraint on the second function. Is there a way make that constraint possible? — hitripekac, Sep 12 '15 at 00:23
I don't quite see why containers of `const` elements should be illegal. Would you mind citing the passage that forbids this? — dyp, Sep 12 '15 at 10:44
@hitripekac "_the C++ Standard forbids containers of const elements_" That's wrong. The *specific case* of `std::vector` does forbid that, via its allocator requirements, but what you said is not a generality at all. See https://stackoverflow.com/questions/6954906/does-c11-allow-vectorconst-t — underscore_d, Sep 23 '18 at 11:15
@underscore_d you are absolutely correct, it is 100% wrong. I should have deleted the comment, but then the subsequent comments from dyp wouldn't make sense. Anyhow, I now know not spout nonsense without referencing the relevant documents :) — hitripekac, Sep 26 '18 at 14:48

score 2 · Accepted Answer · answered Sep 12 '15 at 11:01

The issues are related to constness. Originally, there's been an issue with the implicit conversion from

optional<pair<const reference_wrapper<T>, size_t>> // A

to

optional<pair<reference_wrapper<T>, size_t>> // B

The first one is const because the key of std::map nodes is const - preventing mutation of the key shall ensure that the tree data structure remains valid.

This implicit conversion is not allowed; this might have to do with the issue described for std::pair in Improving pair and tuple - N4387.

A quick fix is to use A as the return type, instead of B. Alternatively, use explicit conversion (a cast or construction of a named object).

The second issue related to constness is that a

reference_wrapper<T>

cannot be initialized with a

T const&

Just like a T& cannot be initialized from an lvalue-expression of type T const. This issue occurs because the most_frequent_element_2 function takes its argument by const&, then the calls to begin and end produce const_iterators.

A quick and dirty solution is to create a vector<reference_wrapper<T const>> instead:

most_frequent_element(std::vector<std::reference_wrapper<T const>>(vector.begin(), vector.end()));
//                                                         ^^^^^

And then adjust the return type of the _2 function to

boost::optional<std::pair<const std::reference_wrapper<T const>, size_t>>
//                                                       ^^^^^

Live demonstration

A better solution might be to recognize that the _2 function does not need a vector, but only two iterators to create a new vector. (The same argument holds for the original most_frequent_element function.) You could write it instead as:

template <typename InIt,
          typename T = std::remove_reference_t<decltype(*std::declval<InIt&>())>>
auto most_frequent_element_2(InIt b, InIt e)
  -> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
    return most_frequent_element(std::vector<std::reference_wrapper<T>>(b, e));
}

where I've used the second (defaulted) template parameter merely for convenience (a typedef within the declaration).

Live demo

Thanks for the detailed answer. The change to iterator is definitely better as it provides a more general solution. — hitripekac, Sep 12 '15 at 21:02

Compiler error with std::reference_wrapper and types in C++

1 Answers1