c++ : unordered map with pair of string_viewes

Question

Here is a code snippet I have :

    struct PairHasher {
        size_t operator()(const std::pair<std::string_view, std::string_view>& stop_stop) const {
            return hasher(stop_stop.first) + 37*hasher(stop_stop.second);
        }
        std::hash<std::string_view> hasher;
    };


BOOST_FIXTURE_TEST_CASE(unordered_map_string_view_pair_must_be_ok, TestCaseStartStopMessager)
{
    const std::vector<std::string> from_stops = {"from_0", "from_1", "from_2"};
    const std::vector<std::string> to_stops = {"to_0", "to_1", "to_2"};

    std::unordered_map<std::pair<std::string_view, std::string_view>, std::int32_t, TransportCatalogue::PairHasher> distance_between_stops;
    for ( std::size_t idx = 0; idx < from_stops.size(); ++idx) {
        std::cout << from_stops[idx] << " : " << to_stops[idx] << std::endl;
        distance_between_stops[std::pair(from_stops[idx], to_stops[idx])] = idx;
    }

    std::cout << "MAP CONTENT :" << std::endl;
    for (auto const& x : distance_between_stops)
    {
        std::cout << x.first.first << " : " << x.first.second << std::endl;
    }
}

I expect to see 3 pairs inside the container, but there is only 1 concerning to the output :

MAP CONTENT :
from_2 : to_2

So, where are two more pair lost? What am I doing wrong?

Answer 1

Moving my comment to an answer.

This is pretty sneaky. I noticed in Compiler Explorer that changing:

distance_between_stops[std::pair(from_stops[idx], to_stops[idx])] = idx;

to

distance_between_stops[std::pair(std::string_view{from_stops[idx]}, std::string_view{to_stops[idx]})] = idx;

fixes the bug. This hints that the problem lies in some implicit string -> string_view conversion. And indeed that is the case, but it is hidden behind one extra layer.

std::pair(from_stops[idx], to_stops[idx]) creates a std::pair<std::string, std::string>, but distance_between_stops requires a std::pair<std::string_view, std::string_view>. When we insert values into the map, this conversion happens implicitly via overload #5 here:

template <class U1, class U2>
constexpr pair(pair<U1, U2>&& p);

5. Initializes first with std::forward<U1>(p.first) and second with std::forward<U2>(p.second).

• This constructor participates in overload resolution if and only if std::is_constructible_v<first_type, U1&&> and std::is_constructible_v<second_type, U2&&> are both true.
• This constructor is explicit if and only if std::is_convertible_v<U1&&, first_type> is false or std::is_convertible_v<U2&&, second_type> is false.

(For reference, std::is_constructible_v<std::string_view, std::string&&> and std::is_convertible_v<std::string&&, std::string_view> are both true, so we know this overload is viable and implicit.)

See the problem yet? When we use the map's operator[], it has to do an implicit conversion to create a key with the proper type. This implicit conversion constructs a pair of string_views that are viewing the temporary memory from the local pair of strings, not the underlying strings in the vector. In other words, it is conceptually similar to:

std::string_view foo(const std::string& s) {
    std::string temp = s + " foo";
    return temp;
}
int main() {
    std::string_view sv = foo("hello");
    std::cout << sv << "\n";
}

Clang emits a warning for this small example, but not OP's full example, which is unfortunate:

warning: address of stack memory associated with local variable 'temp' returned [-Wreturn-stack-address]
    return temp;
           ^~~~