hitripekac
hitripekac

Reputation: 83

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

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)

Upvotes: 2

Views: 1700

Answers (1)

dyp
dyp

Reputation: 39121

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

Upvotes: 2

Related Questions