Fix warnings about pessimizing return moves for C++11 and higher

[dim]

Summary

Throughout the libc++ headers, there are a few instances where
_VSTD::move() is used to return a local variable. Howard commented in
r189039 that these were there "for non-obvious reasons such as to help
things limp along in C++03 language mode".

However, when compiling these headers with warnings on, and in C++11 or
higher mode (like we do in FreeBSD), they cause the following complaints
about pessimizing moves:

In file included from tests.cpp:26:
In file included from tests.hpp:29:
/usr/include/c++/v1/map:1368:12: error: moving a local object in a return statement prevents copy elision [-Werror,-Wpessimizing-move]
    return _VSTD::move(__h);  // explicitly moved for C++03
           ^
/usr/include/c++/v1/__config:368:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_NAMESPACE
              ^

Attempt to fix this by adding a _LIBCPP_EXPLICIT_MOVE() macro to
__config, which gets defined to _VSTD::move for pre-C++11, and to
nothing for C++11 and later.

I am not completely satisfied with the macro name (I also considered
_LIBCPP_COMPAT_MOVE and some other variants), so suggestions are
welcome. :)

[• arthur.j.odwyer]

From the peanut gallery: Could someone explain why this warning is actually
a false positive in C++03 mode? It seems to me that the std::move there
disables NRVO in C++03 to exactly the same degree as it disables NRVO in
C++11. Admittedly I have no idea what _VSTD::move(x) does to x in C++03
mode; I'd have expected it to be a no-op, except for disabling RVO.

–Arthur

[EricWF]

Thanks for the patch. I ran into this issue the other day and I'm glad to see it fixed.

A little rational: The explicit move's are needed in order to "move" a unique_ptr in C++03. There is a special definition of std::move in memory at line 3100 that performs some hacks to make unique_ptr movable. I don't think any other classes benefit from the "explicit move" in C++03.

I am not completely satisfied with the macro name (I also considered
_LIBCPP_COMPAT_MOVE and some other variants), so suggestions are
welcome. :)

I think the name is fine and there is no need to change it. However maybe the name should reflect the fact that it is only needed for unique_ptr? Something like _LIBCPP_UNIQUE_PTR_MOVE would better convey the weirdness this macro is doing.

include/__config

719

We need the explicit move whenever _LIBCPP_HAS_NO_RVALUE_REFERENCES is defined. Please use that instead.

include/algorithm

854

This one seems suspect. I don't really know if we need the move here. Do you know why this here?

[dim]

Used _LIBCPP_HAS_NO_RVALUE_REFERENCES instead of checking __cplusplus.

[mclow.lists]

include/__config

724

I think that this needs more exploration; someone needs to look at the codegen.

return x;

is different from:

return (x);   // this is an expression

I suspect that only one will trigger RVO.

[dim]

include/__config

724

Would you really think so? I would think the parentheses are superfluous, and can be removed by parsing stages before it gets to RVO. But I will try to check the codegen anyway.

Alternatively, since the macro is for 'internal use only', we might try to just remove the parentheses, e.g.:

#ifdef _LIBCPP_HAS_NO_RVALUE_REFERENCES
#  define _LIBCPP_EXPLICIT_MOVE(x) _VSTD::move(x)
#else
#  define _LIBCPP_EXPLICIT_MOVE(x) x
#endif

It is a bit ugly, but at least the code itself does not have to be littered with #ifdefs.

[EricWF]

LGTM.

[EricWF]

@dim Has this been committed or is there something holding you back?

[dim]

@EricWF, no, this just dropped off my radar, sorry. I will commit it now. :)

include/__config

 #endif
 
 #if _LIBCPP_STD_VER > 11 && !defined(_LIBCPP_HAS_NO_CXX14_CONSTEXPR)
 #define _LIBCPP_CONSTEXPR_AFTER_CXX11 constexpr
 #else
 #define _LIBCPP_CONSTEXPR_AFTER_CXX11
 #endif
 
+#ifdef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+#  define _LIBCPP_EXPLICIT_MOVE(x) _VSTD::move(x)
+#else
+#  define _LIBCPP_EXPLICIT_MOVE(x) (x)
+#endif
+
 #ifndef _LIBCPP_HAS_NO_ASAN
 extern "C" void __sanitizer_annotate_contiguous_container(
   const void *, const void *, const void *, const void *);
 #endif
 
 // Try to find out if RTTI is disabled.
 // g++ and cl.exe have RTTI on by default and define a macro when it is.
 // g++ only defines the macro in 4.3.2 and onwards.

include/__hash_table

 __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node(const value_type& __v)
 {
     __node_allocator& __na = __node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_), __v);
     __h.get_deleter().__value_constructed = true;
     __h->__hash_ = hash_function()(__h->__value_);
     __h->__next_ = nullptr;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template <class _Tp, class _Hash, class _Equal, class _Alloc>
 typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_holder
 __hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node(const value_type& __v,
                                                            size_t __hash)
 {
     __node_allocator& __na = __node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_), __v);
     __h.get_deleter().__value_constructed = true;
     __h->__hash_ = __hash;
     __h->__next_ = nullptr;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 template <class _Tp, class _Hash, class _Equal, class _Alloc>
 typename __hash_table<_Tp, _Hash, _Equal, _Alloc>::iterator
 __hash_table<_Tp, _Hash, _Equal, _Alloc>::erase(const_iterator __p)
 {
     __node_pointer __np = __p.__node_;
 #if _LIBCPP_DEBUG_LEVEL >= 2

include/__tree

 template <class _Tp, class _Compare, class _Allocator>
 typename __tree<_Tp, _Compare, _Allocator>::__node_holder
 __tree<_Tp, _Compare, _Allocator>::__construct_node(const value_type& __v)
 {
     __node_allocator& __na = __node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_), __v);
     __h.get_deleter().__value_constructed = true;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template <class _Tp, class _Compare, class _Allocator>
 pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
 __tree<_Tp, _Compare, _Allocator>::__insert_unique(const value_type& __v)
 {

include/algorithm

 
 template <class _InputIterator, class _Function>
 inline _LIBCPP_INLINE_VISIBILITY
 _Function
 for_each(_InputIterator __first, _InputIterator __last, _Function __f)
 {
     for (; __first != __last; ++__first)
         __f(*__first);
-    return _VSTD::move(__f);  // explicitly moved for (emulated) C++03
+    return _LIBCPP_EXPLICIT_MOVE(__f);  // explicitly moved for (emulated) C++03
 }
 
 // find
 
 template <class _InputIterator, class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY
 _InputIterator
 find(_InputIterator __first, _InputIterator __last, const _Tp& __value_)

include/ext/hash_map

 hash_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__construct_node(const key_type& __k)
 {
     __node_allocator& __na = __table_.__node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.first), __k);
     __h.get_deleter().__first_constructed = true;
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.second));
     __h.get_deleter().__second_constructed = true;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 hash_map<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first,
                                                        _InputIterator __last)

include/map

 map<_Key, _Tp, _Compare, _Allocator>::__construct_node_with_key(const key_type& __k)
 {
     __node_allocator& __na = __tree_.__node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.first), __k);
     __h.get_deleter().__first_constructed = true;
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.second));
     __h.get_deleter().__second_constructed = true;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 template <class _Key, class _Tp, class _Compare, class _Allocator>
 _Tp&
 map<_Key, _Tp, _Compare, _Allocator>::operator[](const key_type& __k)
 {
     __node_base_pointer __parent;
     __node_base_pointer& __child = __find_equal_key(__parent, __k);

include/unordered_map

 unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::__construct_node_with_key(const key_type& __k)
 {
     __node_allocator& __na = __table_.__node_alloc();
     __node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.first), __k);
     __h.get_deleter().__first_constructed = true;
     __node_traits::construct(__na, _VSTD::addressof(__h->__value_.__cc.second));
     __h.get_deleter().__second_constructed = true;
-    return _VSTD::move(__h);  // explicitly moved for C++03
+    return _LIBCPP_EXPLICIT_MOVE(__h);  // explicitly moved for C++03
 }
 
 template <class _Key, class _Tp, class _Hash, class _Pred, class _Alloc>
 template <class _InputIterator>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::insert(_InputIterator __first,
                                                        _InputIterator __last)