Change flat_tree::unsafe_emplace to actually emplace

base/containers/flat_tree.h

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_CONTAINERS_FLAT_TREE_H_
 #define BASE_CONTAINERS_FLAT_TREE_H_
 
 #include <algorithm>
 #include <vector>
 
@@ skipping 230 matching lines @@
   }
 
   friend bool operator<=(const flat_tree& lhs, const flat_tree& rhs) {
     return !(lhs > rhs);
   }
 
   friend void swap(flat_tree& lhs, flat_tree& rhs) { lhs.swap(rhs); }
 
  protected:
   // Emplaces a new item into the tree that is known not to be in it. This
-  // is for implementing map [] and at().
+  // is for implementing map operator[].
   template <class... Args>
   iterator unsafe_emplace(const_iterator position, Args&&... args);
 
  private:
   // Helper class for e.g. lower_bound that can compare a value on the left
   // to a key on the right.
   struct KeyValueCompare {
     // The key comparison object must outlive this class.
     explicit KeyValueCompare(const key_compare& key_comp)
         : key_comp_(key_comp) {}
@@ skipping 414 matching lines @@
     flat_tree& other) {
   std::swap(impl_, other.impl_);
 }
 
 template <class Key, class Value, class GetKeyFromValue, class KeyCompare>
 template <class... Args>
 auto flat_tree<Key, Value, GetKeyFromValue, KeyCompare>::unsafe_emplace(
     const_iterator position,
     Args&&... args) -> iterator {
   // We have to cast away const because of crbug.com/677044.
-  return impl_.body_.insert(const_cast_it(position),
-                            value_type(std::forward<Args>(args)...));
+  return impl_.body_.emplace(const_cast_it(position),
+                             std::forward<Args>(args)...);

[jdoerrie, 2017/04/04 14:14:13]

There was some discussion in the emplace thread claiming that sometimes
explicitly constructing the type is clearer and worth more than the small
performance cost of doing an extra move
(https://groups.google.com/a/chromium.org/forum/#!topic/cxx/m3cblzEta7A).

However, I think given that this is library code and this particular method is
not supposed to be called directly by customers we should try to be efficient as
possible.

[dyaroshev, 2017/04/04 15:48:21]

This wasn't the intention (at least from my point of view).

 }
 
 // For containers like sets, the key is the same as the value. This implements
 // the GetKeyFromValue template parameter to flat_tree for this case.
 template <class Key>
 struct GetKeyFromValueIdentity {
   const Key& operator()(const Key& k) const { return k; }
 };
 
 }  // namespace internal
@@ skipping 10 matching lines @@
 void EraseIf(base::internal::flat_tree<Key, Value, GetKeyFromValue, KeyCompare>&
                  container,
              Predicate pred) {
   container.erase(std::remove_if(container.begin(), container.end(), pred),
                   container.end());
 }
 
 }  // namespace base
 
 #endif  // BASE_CONTAINERS_FLAT_TREE_H_