Is std::move safe in an arguments list when the argument is forwarded, not move constructed?

Trying to provide a solution to std::string_view and std::string in std::unordered_set, I'm playing around with replacing std::unordered_set<std::string> with std::unordered_map<std::string_view, std::unique_ptr<std::string>> (the value is std::unique_ptr<std::string> because the small string optimization would mean that the address of the string's underlying data will not always be transferred as a result of std::move).

My original test code, that seems to work, is (omitting headers):

using namespace std::literals;

int main(int argc, char **argv) {
    std::unordered_map<std::string_view, std::unique_ptr<std::string>> mymap;

    for (int i = 1; i < argc; ++i) {
        auto to_insert = std::make_unique<std::string>(argv[i]);
        
        mymap.try_emplace(*to_insert, std::move(to_insert));
    }

    for (auto&& entry : mymap) {
        std::cout << entry.first << ": " << entry.second << std::endl;
    }

    std::cout << std::boolalpha << "\"this\" in map? " << (mymap.count("this") == 1) << std::endl;
    std::cout << std::boolalpha << "\"this\"s in map? " << (mymap.count("this"s) == 1) << std::endl;
    std::cout << std::boolalpha << "\"this\"sv in map? " << (mymap.count("this"sv) == 1) << std::endl;
    return EXIT_SUCCESS;
}

I compile with g++ 7.2.0, compile line is g++ -O3 -std=c++17 -Wall -Wextra -Werror -flto -pedantic test_string_view.cpp -o test_string_view receiving no warnings of any kind, then run, getting the following output:

$ test_string_view this is a test this is a second test
second: second
test: test
a: a
this: this
is: is
"this" in map? true
"this"s in map? true
"this"sv in map? true

which is what I expected.

My main concern here is whether:

        mymap.try_emplace(*to_insert, std::move(to_insert));

has defined behavior. The *to_insert relies on to_insert not being emptied (by move constructing the std::unique_ptr stored in the map) until after the string_view is constructed. The two definitions of try_emplace that would be considered are:

try_emplace(const key_type& k, Args&&... args);

and

try_emplace(key_type&& k, Args&&... args);

I'm not sure which would be chosen, but either way, it seems like key_type would be constructed as part of calling try_emplace, while the arguments to make the mapped_type (the "value", though maps seem to use value_type to refer to the combined key/value pair) are forwarded along, and not used immediately, which makes the code defined. Am I correct in this interpretation, or is this undefined behavior?

My concern is that other, similar constructions that seem definitely undefined still seem to work, e.g.:

mymap.insert(std::make_pair<std::string_view,
                            std::unique_ptr<std::string>>(*to_insert,
                                                          std::move(to_insert)));

produces the expected output, while similar constructs like:

mymap.insert(std::make_pair(std::string_view(*to_insert),
                            std::unique_ptr<std::string>(std::move(to_insert))));

trigger a Segmentation fault at runtime, despite none of them raising warnings of any kind, and both constructs seemingly equally unsequenced (unsequenced implicit conversions in the working insert, unsequenced explicit conversion in the segfaulting insert), so I don't want to say "try_emplace worked for me, so it's okay."

Note that while this question is similar to C++11: std::move() call on arguments' list, it's not quite a duplicate (it's what presumably makes the std::make_pair here unsafe, but not necessarily applicable to try_emplace's forwarding based behavior); in that question, the function receiving the arguments receives std::unique_ptr, triggering construction immediately, while try_emplace is receiving arguments for forwarding, not std::unique_ptr, so while the std::move has "occurred" (but done nothing yet), I think we're safe since the std::unique_ptr is constructed "later".

Yes, your call to try_emplace is perfectly safe. std::move actually doesn't move anything, it just casts the passed variable to an xvalue. No matter what order the arguments are initialized, nothing will ever be moved because the parameters are all references. References bind directly to objects, they do not call any constructors.

If you look at your second snippet, you'll see that std::make_pair also takes its parameters by reference, so in that case too a move will not be made except in the constructor body.

Your third snippet however does have the problem of UB. The difference is subtle, but if the arguments of make_pair are evaluated left-to-right, then a temporary std::unique_ptr object will get initialized with the moved from value of to_insert. This means that now, to_insert is null because a move actually happened because you are explicitly constructing an object that actually performs the move.

Recommended topics

Hot tags