[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/__hash_table:2261
+_NodeHandle
+__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_extract_unique(
+    key_type const& __key)
----------------
If I'm not mistaken, `__node_handle_extract_unique` and `__node_handle_extract_multi` have the exact same implementation. This is intentional, no? If so can't we just use one implementation? 


================
Comment at: libcxx/include/__node_handle:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
This header needs to be added to the `module.modulemap`.


================
Comment at: libcxx/include/__node_handle:82
+    __node_handle_base(__node_handle_base&& __other) noexcept
+            : __ptr_(_VSTD::move(__other.__ptr_)),
+              __alloc_(_VSTD::move(__other.__alloc_))
----------------
I'm not sure we should by trying to move a pointer. It's misleading, and we shouldn't expect it to act any differently.


================
Comment at: libcxx/include/__node_handle:118
+
+    _LIBCPP_INLINE_VISIBILITY
+    bool empty() const { return __ptr_ == nullptr; }
----------------
Missing `_LIBCPP_NODISCARD_AFTER_CXX17`


================
Comment at: libcxx/test/std/containers/associative/set/merge.pass.cpp:20
+#include <set>
+#include "Counter.h"
+
----------------
Please include `test_macros.h` directly.


https://reviews.llvm.org/D46845