Contents
V1098. The 'emplace' / 'insert' function call contains potentially dangerous move operation. Moved object can be destroyed even if there is no insertion.
- The 'try_emplace' function for 'std::map' / 'std::unordered_map'
- The 'lower_bound' and 'emplace_hint' functions for 'std::set' / 'std::map'
- Note N1
- Note N2
The analyzer has detected a potentially dangerous move operation. An object is moved into an associative container 'std::set' / 'std::map' / 'std::unordered_map' by calling the 'emplace' / 'insert' function. If an element with the specified key already exists in the container, the moved object may cause premature release of resources.
Let's take a look at an example:
using pointer_type = std::unique_ptr<void, void (*)(void *)>;
std::unordered_map<uintmax_t, pointer_type> Cont;
// Unique pointer should be moved only if
// there is no element in the container by the specified key
bool add_entry(uintmax_t key, pointer_type &&ptr)
{
auto [it, inserted] = Cont.emplace(key, std::move(ptr));
if (!inserted)
{
// dereferencing the potentially null pointer 'ptr' here
}
return inserted;
}In the example, the 'add_entry' function receives a smart pointer to some resource and its corresponding key. According to the programmer's intention, a smart pointer should be moved into an associative container only if there was no insertion with the same key before. If the insertion did not happen, the resource will be handled by a smart pointer.
However, such code contains two problems:
- If no insertion has occurred, the resource from the 'ptr' pointer can still be moved. This will cause the premature release of resource.
- The 'ptr' pointer can turn null, and its dereference will lead to undefined behavior.
Let's consider the possible ways to fix these issues.
The 'try_emplace' function for 'std::map' / 'std::unordered_map'
Since the C++17 standard, the 'try_emplace' function has been added for the 'std::map' and 'std::unordered_map' containers. The function ensures that if an element with the specified key already exists, the function arguments will not be copied or moved. Therefore, it is recommended to use the 'try_emplace' function instead of 'emplace' and 'insert' for 'std::map' and 'std::unordered_map' containers.
Here's the fixed code:
using pointer_type = std::unique_ptr<void, void (*)(void *)>;
std::unordered_map<uintmax_t, pointer_type> Cont;
bool add_entry(uintmax_t key, pointer_type &&ptr)
{
auto [it, inserted] = Cont.try_emplace(key, std::move(ptr));
if (!inserted)
{
// dereferencing the 'ptr' here
// 'ptr' is guaranteed to be non-null
}
return inserted;
}The 'lower_bound' and 'emplace_hint' functions for 'std::set' / 'std::map'
If the 'try_emplace' function is unavailable, you can split finding and inserting operations for sorted associative containers ('std::set', 'std::map'):
- the 'lower_bound' function will find either an element by a given key, or the insertion position for a new element;
- the 'emplace_hint' function will effectively insert an element.
Let's replace the container with 'std::map' and use the 'lower_bound' and 'emplace_hint' functions for the previous example:
using pointer_type = std::unique_ptr<void, void (*)(void *)>;
std::map<uintmax_t, pointer_type> Cont;
// Unique pointer should be moved only if
// there is no element in the container by the specified key
bool add_entry(uintmax_t key, pointer_type &&ptr)
{
bool inserted;
auto it = Cont.lower_bound(key);
if (it != Cont.end() && key == it->first)
{
// key exists
inserted = false;
}
else
{
// key doesn't exist
it = Cont.emplace_hint(it, key, std::move(ptr));
inserted = true;
}
if (!inserted)
{
// dereferencing the 'ptr' here
// 'ptr' is guaranteed to be non-null
}
return inserted;
}Note N1
The analyzer can issue warnings to the similar code:
using pointer_type = std::unique_ptr<void, void (*)(void *)>;
std::map<uintmax_t, pointer_type> Cont;
// Unique pointer should be moved only if
// there is no element in the container by the specified key
bool add_entry(uintmax_t key, pointer_type &&ptr)
{
bool inserted;
auto it = Cont.find(key);
if (it == Cont.end())
{
std::tie(it, inserted) = Cont.emplace(key, std::move(ptr)); // <=
}
else
{
inserted = false;
}
if (!inserted)
{
// dereferencing the 'ptr' here
// 'ptr' is guaranteed to be non-null
}
return inserted;
}There is no error in the example: if the element with the specified key does not exist, the insertion will surely happen. However, the code is unoptimized: first, the element is searched by the key, then the search is repeated to find the insertion position inside the 'emplace' function. Therefore, it is recommended to optimize the code using one of the methods described above.
Note N2
The diagnostic has two certainty levels. The first level is issued for move-only objects, i.e. when the user-defined type has no copy constructors/operators. This means that, in case of an unsuccessful insertion, the premature release of resources may happen. This is true for 'std::unique_ptr' and 'std::unique_lock', for example. Otherwise, the second certainty level is issued.
The diagnostic does not handle types that don't have the move constructor, because, in this case, the objects are copied.
This diagnostic is classified as:
Was this page helpful?
