[libcxx-commits] [libcxx] 56df1d8 - [libc++] Use addressof in vector.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Thu Oct 21 08:28:36 PDT 2021


Author: Mark de Wever
Date: 2021-10-21T17:28:17+02:00
New Revision: 56df1d80e2911a8cd23b70e9b7e76fcb386c7956

URL: https://github.com/llvm/llvm-project/commit/56df1d80e2911a8cd23b70e9b7e76fcb386c7956
DIFF: https://github.com/llvm/llvm-project/commit/56df1d80e2911a8cd23b70e9b7e76fcb386c7956.diff

LOG: [libc++] Use addressof in vector.

This addresses the usage of `operator&` in `<vector>`.

I now added tests for the current offending cases. I wonder whether it
would be better to add one addressof test per directory and test all
possible violations. Also to guard against possible future errors?

(Note there are still more headers with the same issue.)

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D111961

Added: 
    libcxx/test/std/containers/sequences/vector/vector.cons/assign_move.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.cons/move.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_rvalue.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_value.addressof.compile.pass.cpp
    libcxx/test/std/containers/sequences/vector/vector.special/swap.addressof.compile.pass.cpp

Modified: 
    libcxx/include/__iterator/wrap_iter.h
    libcxx/include/vector

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index 0a978914b1aa9..28872f9fa41af 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -13,6 +13,7 @@
 #include <__config>
 #include <__debug>
 #include <__iterator/iterator_traits.h>
+#include <__memory/addressof.h>
 #include <__memory/pointer_traits.h>
 #include <type_traits>
 
@@ -54,7 +55,7 @@ class __wrap_iter
             : __i(__u.base())
     {
 #if _LIBCPP_DEBUG_LEVEL == 2
-        __get_db()->__iterator_copy(this, &__u);
+        __get_db()->__iterator_copy(this, _VSTD::addressof(__u));
 #endif
     }
 #if _LIBCPP_DEBUG_LEVEL == 2
@@ -62,14 +63,14 @@ class __wrap_iter
     __wrap_iter(const __wrap_iter& __x)
         : __i(__x.base())
     {
-        __get_db()->__iterator_copy(this, &__x);
+        __get_db()->__iterator_copy(this, _VSTD::addressof(__x));
     }
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
     __wrap_iter& operator=(const __wrap_iter& __x)
     {
-        if (this != &__x)
+        if (this != _VSTD::addressof(__x))
         {
-            __get_db()->__iterator_copy(this, &__x);
+            __get_db()->__iterator_copy(this, _VSTD::addressof(__x));
             __i = __x.__i;
         }
         return *this;
@@ -180,7 +181,7 @@ _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
 bool operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
+    _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(_VSTD::addressof(__x), _VSTD::addressof(__y)),
                    "Attempted to compare incomparable iterators");
 #endif
     return __x.base() < __y.base();
@@ -264,7 +265,7 @@ operator-(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXC
 #endif // C++03
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
+    _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(_VSTD::addressof(__x), _VSTD::addressof(__y)),
                    "Attempted to subtract incompatible iterators");
 #endif
     return __x.base() - __y.base();

diff  --git a/libcxx/include/vector b/libcxx/include/vector
index c9b121b3177df..1d8098d522596 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1296,7 +1296,7 @@ vector<_Tp, _Allocator>::vector(vector&& __x)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
     __get_db()->__insert_c(this);
-    __get_db()->swap(this, &__x);
+    __get_db()->swap(this, _VSTD::addressof(__x));
 #endif
     this->__begin_ = __x.__begin_;
     this->__end_ = __x.__end_;
@@ -1319,7 +1319,7 @@ vector<_Tp, _Allocator>::vector(vector&& __x, const __identity_t<allocator_type>
         this->__end_cap() = __x.__end_cap();
         __x.__begin_ = __x.__end_ = __x.__end_cap() = nullptr;
 #if _LIBCPP_DEBUG_LEVEL == 2
-        __get_db()->swap(this, &__x);
+        __get_db()->swap(this, _VSTD::addressof(__x));
 #endif
     }
     else
@@ -1395,7 +1395,7 @@ vector<_Tp, _Allocator>::__move_assign(vector& __c, true_type)
     this->__end_cap() = __c.__end_cap();
     __c.__begin_ = __c.__end_ = __c.__end_cap() = nullptr;
 #if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->swap(this, &__c);
+    __get_db()->swap(this, _VSTD::addressof(__c));
 #endif
 }
 
@@ -1716,7 +1716,7 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::erase(const_iterator __position)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::erase(iterator) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1735,10 +1735,10 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::erase(const_iterator __first, const_iterator __last)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__first) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__first)) == this,
         "vector::erase(iterator,  iterator) called with an iterator not"
         " referring to this vector");
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__last) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__last)) == this,
         "vector::erase(iterator,  iterator) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1776,7 +1776,7 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::insert(const_iterator __position, const_reference __x)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::insert(iterator, x) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1813,7 +1813,7 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::insert(const_iterator __position, value_type&& __x)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::insert(iterator, x) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1846,7 +1846,7 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::emplace(const_iterator __position, _Args&&... __args)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::emplace(iterator, x) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1881,7 +1881,7 @@ typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::insert(const_iterator __position, size_type __n, const_reference __x)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::insert(iterator, n, x) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1932,7 +1932,7 @@ typename enable_if
 vector<_Tp, _Allocator>::insert(const_iterator __position, _InputIterator __first, _InputIterator __last)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::insert(iterator, range) called with an iterator not"
         " referring to this vector");
 #endif
@@ -1985,7 +1985,7 @@ typename enable_if
 vector<_Tp, _Allocator>::insert(const_iterator __position, _ForwardIterator __first, _ForwardIterator __last)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
-    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(&__position) == this,
+    _LIBCPP_ASSERT(__get_const_db()->__find_c_from_i(_VSTD::addressof(__position)) == this,
         "vector::insert(iterator, range) called with an iterator not"
         " referring to this vector");
 #endif
@@ -2066,7 +2066,7 @@ vector<_Tp, _Allocator>::swap(vector& __x)
     _VSTD::__swap_allocator(this->__alloc(), __x.__alloc(),
         integral_constant<bool,__alloc_traits::propagate_on_container_swap::value>());
 #if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->swap(this, &__x);
+    __get_db()->swap(this, _VSTD::addressof(__x));
 #endif
 }
 

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.cons/assign_move.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_move.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..168a9e804f67a
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_move.addressof.compile.pass.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// vector& operator=(vector&& c);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> vo;
+  std::vector<operator_hijacker> v;
+  v = std::move(vo);
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.cons/move.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/move.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..cae17b3550dd0
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/move.addressof.compile.pass.cpp
@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <vector>
+
+// vector(vector&& c);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  {
+    std::vector<operator_hijacker> vo{};
+    std::vector<operator_hijacker> v{std::move(vo)};
+  }
+  {
+    std::vector<operator_hijacker> vo{};
+    std::vector<operator_hijacker> v{std::move(vo), std::allocator<operator_hijacker>{}};
+  }
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..0875705a4c42f
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.addressof.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <vector>
+
+// template <class... Args> iterator emplace(const_iterator pos, Args&&... args);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  v.emplace(v.end());
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..0fce3498fec7e
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.addressof.compile.pass.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// iterator erase(const_iterator position);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  v.erase(v.end());
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..bc90fa783e98f
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.addressof.compile.pass.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// iterator erase(const_iterator position);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  v.erase(v.begin(), v.end());
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..a8eb860e64877
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_iter_iter.addressof.compile.pass.cpp
@@ -0,0 +1,32 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// template <class Iter>
+//   iterator insert(const_iterator position, Iter first, Iter last);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+#include "test_iterators.h"
+
+void test() {
+  {
+    std::vector<operator_hijacker> v;
+    cpp17_input_iterator<std::vector<operator_hijacker>::iterator> i;
+    v.insert(v.end(), i, i);
+  }
+  {
+    std::vector<operator_hijacker> v;
+    v.insert(v.end(), v.begin(), v.end());
+  }
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_rvalue.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_rvalue.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..ba497779a9673
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_rvalue.addressof.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <vector>
+
+// iterator insert(const_iterator position, value_type&& x);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  v.insert(v.end(), operator_hijacker{});
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..c02b92a4998e8
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_size_value.addressof.compile.pass.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// iterator insert(const_iterator position, size_type n, const value_type& x);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  operator_hijacker val;
+  v.insert(v.end(), 1, val);
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_value.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_value.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..fbf1a4f50b974
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_iter_value.addressof.compile.pass.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// iterator insert(const_iterator position, const value_type& x);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> v;
+  operator_hijacker val;
+  v.insert(v.end(), val);
+}

diff  --git a/libcxx/test/std/containers/sequences/vector/vector.special/swap.addressof.compile.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.special/swap.addressof.compile.pass.cpp
new file mode 100644
index 0000000000000..4e908d9ff6eac
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.special/swap.addressof.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// template <class T, class Alloc>
+//   void swap(vector<T,Alloc>& x, vector<T,Alloc>& y);
+
+// Validate whether the container can be copy-assigned with an ADL-hijacking operator&
+
+#include <vector>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::vector<operator_hijacker> vo;
+  std::vector<operator_hijacker> v;
+  v.swap(vo);
+}


        


More information about the libcxx-commits mailing list