[libcxx-commits] [PATCH] D62779: [2/2] Fix complexity of map insert_or_assign with a hint.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a reviewer: libc++.
ldionne added inline comments.


================
Comment at: libcxx/include/map:1274
+    {
+         return __tree_.__insert_or_assign_hint(__h.__i_, __k,  _VSTD::forward<_Vp>(__v));
+    }
----------------
Mordante wrote:
> ldionne wrote:
> > I think it would be better if `__emplace_hint_unique_key_args` returned a pair of `bool` and `iterator`, like `__emplace_unique_key_args` does. We could then reuse it to implement this functionality. WDYT?
> I think it could work something like:
> ```
>       auto [__r, __inserted] = __tree_.__emplace_hint_unique_key_args(     
>           __h.__i_, __k, _VSTD::move(__k), _VSTD::forward<_Vp>(__v));           
>         
>       if (!__inserted)                                                          
>         __r->__get_value().second = _VSTD::forward<_Vp>(__v);              
> 
>       return __r;                                                          
> 
> ```
> I'm not fond of using `__k` and moving it in the same function call, but `try_emplace(key_type&& __k, _Args&&... __args)` uses the same method for the key. Since it's safe here and I see no reason why it would become unsafe in the future I don't mind.
> 
> I like that this approach keeps the header file smaller so I'll test this approach further.
Excellent. I like that it avoids code duplication, but I agree the `move` is a bit sneaky. Let's use that approach, though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62779/new/

https://reviews.llvm.org/D62779