[PATCH] D47607: [libcxx] Almost fix some UB in <map> and <unordered_map>

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

Overall this change seems reasonable to me. Thanks for working on this. Initially I was concerned we would hit issues optimizing const objects,
but I should have read your description first! Thanks for ensuring Clang doesn't optimize on this.

One other concern I have is if there will be any performance impact to using `launder` all the time, but better safe than sorry.

Other than the inline comments, this LGTM.



================
Comment at: libcxx/include/map:633
+#if _LIBCPP_STD_VER > 14
+        return _VSTD::launder(this)->__cc;
+#else
----------------
rsmith wrote:
> Formally this should be `return *_VSTD::launder(&this->__cc);`, because `this` must already point to an in-lifetime object or the member function call would have been invalid.
`return *_VSTD::launder(_VSTD::addressof(this->__cc));` because ADL.


================
Comment at: libcxx/include/map:648
+    _LIBCPP_INLINE_VISIBILITY
+    __nc_ref_pair_type __ref()
+    {
----------------
I think `__ref` can be private.


================
Comment at: libcxx/include/map:653
+    }
+    _LIBCPP_INLINE_VISIBILITY
+    __nc_rref_pair_type __move()
----------------
Add a newline?


================
Comment at: libcxx/include/map:708
+public:
+    value_type& __get_value() { return __cc; }
+    const value_type& __get_value() const { return __cc; }
----------------
`_LIBCPP_INLINE_VISIBILITY` here and below it.


================
Comment at: libcxx/include/unordered_map:597
+public:
+    value_type& __get_value()
+    {
----------------
`_LIBCPP_INLINE_VISIBILITY`


================
Comment at: libcxx/include/unordered_map:606
+
+    const value_type& __get_value() const
+    {
----------------
`_LIBCPP_INLINE_VISIBILITY`


================
Comment at: libcxx/include/unordered_map:615
+
+    __nc_ref_pair_type __ref()
+    {
----------------
This can be private.


================
Comment at: libcxx/include/unordered_map:621
+
+    __nc_rref_pair_type __move()
+    {
----------------
`_LIBCPP_INLINE_VISIBILITY`


================
Comment at: libcxx/include/unordered_map:677
+public:
+    value_type& __get_value() { return __cc; }
+    const value_type& __get_value() const { return __cc; }
----------------
`_LIBCPP_INLINE_VISIBILITY`


Repository:
  rCXX libc++

https://reviews.llvm.org/D47607