Skip to content

Commit

Permalink
[libc++] Comma-operator-proof a lot of algorithm/container code.
Browse files Browse the repository at this point in the history
Detected by evil-izing the widely used `MoveOnly` testing type.
I had to patch some tests that were themselves using its comma operator,
but I think that's a worthwhile cost in order to catch more places
in our headers that needed comma-proofing.

The trick here is that even `++ptr, SomeClass()` can find a comma operator
by ADL, if `ptr` is of type `Evil*`. (A comma between two operands
of non-class-or-enum type is always treated as the built-in
comma, without ADL. But if either operand is class-or-enum, then
ADL happens for _both_ operands' types.)

Differential Revision: https://reviews.llvm.org/D109414
  • Loading branch information
Arthur O'Dwyer committed Sep 8, 2021
1 parent c00cb52 commit 16bf433
Show file tree
Hide file tree
Showing 16 changed files with 50 additions and 47 deletions.
4 changes: 2 additions & 2 deletions libcxx/include/__algorithm/stable_sort.h
Expand Up @@ -38,14 +38,14 @@ __merge_move_construct(_InputIterator1 __first1, _InputIterator1 __last1,
{
if (__first1 == __last1)
{
for (; __first2 != __last2; ++__first2, ++__result, (void)__d.template __incr<value_type>())
for (; __first2 != __last2; ++__first2, (void) ++__result, __d.template __incr<value_type>())
::new ((void*)__result) value_type(_VSTD::move(*__first2));
__h.release();
return;
}
if (__first2 == __last2)
{
for (; __first1 != __last1; ++__first1, ++__result, (void)__d.template __incr<value_type>())
for (; __first1 != __last1; ++__first1, (void) ++__result, __d.template __incr<value_type>())
::new ((void*)__result) value_type(_VSTD::move(*__first1));
__h.release();
return;
Expand Down
4 changes: 2 additions & 2 deletions libcxx/include/__hash_table
Expand Up @@ -2348,7 +2348,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__rehash(size_type __nbc)
size_type __chash = __constrain_hash(__cp->__hash(), __nbc);
__bucket_list_[__chash] = __pp;
size_type __phash = __chash;
for (__pp = __cp, __cp = __cp->__next_; __cp != nullptr;
for (__pp = __cp, void(), __cp = __cp->__next_; __cp != nullptr;
__cp = __pp->__next_)
{
__chash = __constrain_hash(__cp->__hash(), __nbc);
Expand Down Expand Up @@ -2758,7 +2758,7 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::bucket_size(size_type __n) const
{
for (__np = __np->__next_; __np != nullptr &&
__constrain_hash(__np->__hash(), __bc) == __n;
__np = __np->__next_, ++__r)
__np = __np->__next_, (void) ++__r)
;
}
return __r;
Expand Down
8 changes: 4 additions & 4 deletions libcxx/include/__memory/uninitialized_algorithms.h
Expand Up @@ -145,7 +145,7 @@ _ForwardIterator uninitialized_default_construct_n(_ForwardIterator __first, _Si
#ifndef _LIBCPP_NO_EXCEPTIONS
try {
#endif
for (; __n > 0; (void)++__idx, --__n)
for (; __n > 0; ++__idx, (void) --__n)
::new ((void*)_VSTD::addressof(*__idx)) _Vt;
return __idx;
#ifndef _LIBCPP_NO_EXCEPTIONS
Expand Down Expand Up @@ -183,7 +183,7 @@ _ForwardIterator uninitialized_value_construct_n(_ForwardIterator __first, _Size
#ifndef _LIBCPP_NO_EXCEPTIONS
try {
#endif
for (; __n > 0; (void)++__idx, --__n)
for (; __n > 0; ++__idx, (void) --__n)
::new ((void*)_VSTD::addressof(*__idx)) _Vt();
return __idx;
#ifndef _LIBCPP_NO_EXCEPTIONS
Expand All @@ -203,7 +203,7 @@ _ForwardIt uninitialized_move(_InputIt __first, _InputIt __last, _ForwardIt __fi
#ifndef _LIBCPP_NO_EXCEPTIONS
try {
#endif
for (; __first != __last; (void)++__idx, ++__first)
for (; __first != __last; ++__idx, (void) ++__first)
::new ((void*)_VSTD::addressof(*__idx)) _Vt(_VSTD::move(*__first));
return __idx;
#ifndef _LIBCPP_NO_EXCEPTIONS
Expand All @@ -223,7 +223,7 @@ uninitialized_move_n(_InputIt __first, _Size __n, _ForwardIt __first_res) {
#ifndef _LIBCPP_NO_EXCEPTIONS
try {
#endif
for (; __n > 0; ++__idx, (void)++__first, --__n)
for (; __n > 0; ++__idx, (void) ++__first, --__n)
::new ((void*)_VSTD::addressof(*__idx)) _Vt(_VSTD::move(*__first));
return {__first, __idx};
#ifndef _LIBCPP_NO_EXCEPTIONS
Expand Down
4 changes: 2 additions & 2 deletions libcxx/include/__split_buffer
Expand Up @@ -239,7 +239,7 @@ __split_buffer<_Tp, _Allocator>::__construct_at_end(_InputIter __first, _InputIt
size_type __old_cap = __end_cap() - __first_;
size_type __new_cap = _VSTD::max<size_type>(2 * __old_cap, 8);
__split_buffer __buf(__new_cap, 0, __a);
for (pointer __p = __begin_; __p != __end_; ++__p, ++__buf.__end_)
for (pointer __p = __begin_; __p != __end_; ++__p, (void) ++__buf.__end_)
__alloc_traits::construct(__buf.__alloc(),
_VSTD::__to_address(__buf.__end_), _VSTD::move(*__p));
swap(__buf);
Expand All @@ -259,7 +259,7 @@ typename enable_if
__split_buffer<_Tp, _Allocator>::__construct_at_end(_ForwardIterator __first, _ForwardIterator __last)
{
_ConstructTransaction __tx(&this->__end_, _VSTD::distance(__first, __last));
for (; __tx.__pos_ != __tx.__end_; ++__tx.__pos_, ++__first) {
for (; __tx.__pos_ != __tx.__end_; ++__tx.__pos_, (void) ++__first) {
__alloc_traits::construct(this->__alloc(),
_VSTD::__to_address(__tx.__pos_), *__first);
}
Expand Down
4 changes: 2 additions & 2 deletions libcxx/include/deque
Expand Up @@ -2214,7 +2214,7 @@ deque<_Tp, _Allocator>::insert(const_iterator __p, size_type __n, const value_ty
size_type __de = __base::size() - __pos;
if (__n > __de)
{
for (size_type __m = __n - __de; __m; --__m, ++__i, ++__base::size())
for (size_type __m = __n - __de; __m; --__m, (void) ++__i, ++__base::size())
__alloc_traits::construct(__a, _VSTD::addressof(*__i), __v);
__n = __de;
}
Expand Down Expand Up @@ -2317,7 +2317,7 @@ deque<_Tp, _Allocator>::insert(const_iterator __p, _BiIter __f, _BiIter __l,
if (__n > 0)
{
iterator __oen = __old_end - __n;
for (iterator __j = __oen; __j != __old_end; ++__i, ++__j, ++__base::size())
for (iterator __j = __oen; __j != __old_end; ++__i, (void) ++__j, ++__base::size())
__alloc_traits::construct(__a, _VSTD::addressof(*__i), _VSTD::move(*__j));
if (__n < __de)
__old_end = _VSTD::move_backward(__old_end - __de, __oen, __old_end);
Expand Down
14 changes: 7 additions & 7 deletions libcxx/include/list
Expand Up @@ -1408,7 +1408,7 @@ list<_Tp, _Alloc>::assign(_InpIter __f, _InpIter __l,
{
iterator __i = begin();
iterator __e = end();
for (; __f != __l && __i != __e; ++__f, ++__i)
for (; __f != __l && __i != __e; ++__f, (void) ++__i)
*__i = *__f;
if (__i == __e)
insert(__e, __f, __l);
Expand All @@ -1425,7 +1425,7 @@ list<_Tp, _Alloc>::assign(size_type __n, const value_type& __x)
{
iterator __i = begin();
iterator __e = end();
for (; __n > 0 && __i != __e; --__n, ++__i)
for (; __n > 0 && __i != __e; --__n, (void) ++__i)
*__i = __x;
if (__i == __e)
insert(__e, __n, __x);
Expand Down Expand Up @@ -1495,7 +1495,7 @@ list<_Tp, _Alloc>::insert(const_iterator __p, size_type __n, const value_type& _
try
{
#endif // _LIBCPP_NO_EXCEPTIONS
for (--__n; __n != 0; --__n, ++__e, ++__ds)
for (--__n; __n != 0; --__n, (void) ++__e, ++__ds)
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
Expand Down Expand Up @@ -1561,7 +1561,7 @@ list<_Tp, _Alloc>::insert(const_iterator __p, _InpIter __f, _InpIter __l,
try
{
#endif // _LIBCPP_NO_EXCEPTIONS
for (++__f; __f != __l; ++__f, (void) ++__e, (void) ++__ds)
for (++__f; __f != __l; ++__f, (void) ++__e, ++__ds)
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), *__f);
Expand Down Expand Up @@ -1909,7 +1909,7 @@ list<_Tp, _Alloc>::resize(size_type __n)
try
{
#endif // _LIBCPP_NO_EXCEPTIONS
for (--__n; __n != 0; --__n, ++__e, ++__ds)
for (--__n; __n != 0; --__n, (void) ++__e, ++__ds)
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_));
Expand Down Expand Up @@ -1967,7 +1967,7 @@ list<_Tp, _Alloc>::resize(size_type __n, const value_type& __x)
try
{
#endif // _LIBCPP_NO_EXCEPTIONS
for (--__n; __n != 0; --__n, ++__e, ++__ds)
for (--__n; __n != 0; --__n, (void) ++__e, ++__ds)
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
Expand Down Expand Up @@ -2244,7 +2244,7 @@ list<_Tp, _Alloc>::merge(list& __c, _Comp __comp)
{
size_type __ds = 1;
iterator __m2 = _VSTD::next(__f2);
for (; __m2 != __e2 && __comp(*__m2, *__f1); ++__m2, ++__ds)
for (; __m2 != __e2 && __comp(*__m2, *__f1); ++__m2, (void) ++__ds)
;
base::__sz() += __ds;
__c.__sz() -= __ds;
Expand Down
6 changes: 3 additions & 3 deletions libcxx/include/string
Expand Up @@ -2525,7 +2525,7 @@ basic_string<_CharT, _Traits, _Allocator>::assign(_ForwardIterator __first, _For
__grow_by(__cap, __n - __cap, __sz, 0, __sz);
}
pointer __p = __get_pointer();
for (; __first != __last; ++__first, ++__p)
for (; __first != __last; ++__p, (void) ++__first)
traits_type::assign(*__p, *__first);
traits_type::assign(*__p, value_type());
__set_size(__n);
Expand Down Expand Up @@ -2703,7 +2703,7 @@ basic_string<_CharT, _Traits, _Allocator>::append(
if (__cap - __sz < __n)
__grow_by(__cap, __sz + __n - __cap, __sz, __sz, 0);
pointer __p = __get_pointer() + __sz;
for (; __first != __last; ++__p, ++__first)
for (; __first != __last; ++__p, (void) ++__first)
traits_type::assign(*__p, *__first);
traits_type::assign(*__p, value_type());
__set_size(__sz + __n);
Expand Down Expand Up @@ -2881,7 +2881,7 @@ basic_string<_CharT, _Traits, _Allocator>::insert(const_iterator __pos, _Forward
__sz += __n;
__set_size(__sz);
traits_type::assign(__p[__sz], value_type());
for (__p += __ip; __first != __last; ++__p, ++__first)
for (__p += __ip; __first != __last; ++__p, (void) ++__first)
traits_type::assign(*__p, *__first);
}
else
Expand Down
6 changes: 3 additions & 3 deletions libcxx/include/vector
Expand Up @@ -1063,7 +1063,7 @@ vector<_Tp, _Allocator>::__construct_at_end(size_type __n)
{
_ConstructTransaction __tx(*this, __n);
const_pointer __new_end = __tx.__new_end_;
for (pointer __pos = __tx.__pos_; __pos != __new_end; ++__pos, __tx.__pos_ = __pos) {
for (pointer __pos = __tx.__pos_; __pos != __new_end; __tx.__pos_ = ++__pos) {
__alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__pos));
}
}
Expand All @@ -1081,7 +1081,7 @@ vector<_Tp, _Allocator>::__construct_at_end(size_type __n, const_reference __x)
{
_ConstructTransaction __tx(*this, __n);
const_pointer __new_end = __tx.__new_end_;
for (pointer __pos = __tx.__pos_; __pos != __new_end; ++__pos, __tx.__pos_ = __pos) {
for (pointer __pos = __tx.__pos_; __pos != __new_end; __tx.__pos_ = ++__pos) {
__alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__pos), __x);
}
}
Expand Down Expand Up @@ -1775,7 +1775,7 @@ vector<_Tp, _Allocator>::__move_range(pointer __from_s, pointer __from_e, pointe
pointer __i = __from_s + __n;
_ConstructTransaction __tx(*this, __from_e - __i);
for (pointer __pos = __tx.__pos_; __i < __from_e;
++__i, ++__pos, __tx.__pos_ = __pos) {
++__i, (void) ++__pos, __tx.__pos_ = __pos) {
__alloc_traits::construct(this->__alloc(),
_VSTD::__to_address(__pos),
_VSTD::move(*__i));
Expand Down
Expand Up @@ -246,15 +246,15 @@ test_move()
c.insert(c.end(), std::move_iterator<I>(&mo), std::move_iterator<I>(&mo+1));
}
int j = 0;
for (CI i = c.begin(); i != c.end(); ++i, ++j)
for (CI i = c.begin(); i != c.end(); ++i, (void) ++j)
assert(*i == MoveOnly(j));
{
MoveOnly mo(1);
typedef cpp17_input_iterator<MoveOnly*> I;
c.insert(c.end(), std::move_iterator<I>(I(&mo)), std::move_iterator<I>(I(&mo+1)));
}
j = 0;
for (CI i = c.begin(); i != c.end(); ++i, ++j)
for (CI i = c.begin(); i != c.end(); ++i, (void) ++j)
assert(*i == MoveOnly(j));
#endif
}
Expand Down
Expand Up @@ -54,11 +54,11 @@ test(int P, C& c1, int x)
assert(c1.size() == c1_osize + 1);
assert(static_cast<std::size_t>(distance(c1.begin(), c1.end())) == c1.size());
i = c1.begin();
for (int j = 0; j < P; ++j, ++i)
for (int j = 0; j < P; ++j, (void) ++i)
assert(*i == MoveOnly(j));
assert(*i == MoveOnly(x));
++i;
for (int j = P; static_cast<std::size_t>(j) < c1_osize; ++j, ++i)
for (int j = P; static_cast<std::size_t>(j) < c1_osize; ++j, (void) ++i)
assert(*i == MoveOnly(j));
}

Expand Down
Expand Up @@ -53,7 +53,7 @@ void test(int size)
{
C c = make<C>(size, rng[j]);
typename C::const_iterator it = c.begin();
for (int i = 0; i < size; ++i, ++it)
for (int i = 0; i < size; ++i, (void) ++it)
assert(*it == MoveOnly(i));
}
}
Expand Down
Expand Up @@ -55,7 +55,7 @@ test(C& c1, int x)
I i = c1.begin();
assert(*i == MoveOnly(x));
++i;
for (int j = 0; static_cast<std::size_t>(j) < c1_osize; ++j, ++i)
for (int j = 0; static_cast<std::size_t>(j) < c1_osize; ++j, (void) ++i)
assert(*i == MoveOnly(j));
}

Expand Down
Expand Up @@ -34,7 +34,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(10));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == n);
assert(n == 10);
assert(c1.get_allocator() == A(10));
Expand All @@ -51,7 +51,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(11));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == n);
assert(n == 10);
assert(c1.get_allocator() == A(11));
Expand All @@ -68,7 +68,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(10));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == 10+n);
assert(n == 4);
assert(c1.get_allocator() == A(10));
Expand All @@ -85,7 +85,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(11));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == 10+n);
assert(n == 4);
assert(c1.get_allocator() == A(11));
Expand All @@ -103,7 +103,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(10));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == n);
assert(n == 10);
assert(c1.get_allocator() == A(10));
Expand All @@ -120,7 +120,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(11));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == n);
assert(n == 10);
assert(c1.get_allocator() == A(10));
Expand All @@ -137,7 +137,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(10));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == 10+n);
assert(n == 4);
assert(c1.get_allocator() == A(10));
Expand All @@ -154,7 +154,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A(11));
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == 10+n);
assert(n == 4);
assert(c1.get_allocator() == A(10));
Expand All @@ -171,7 +171,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A());
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == n);
assert(n == 10);
assert(c1.get_allocator() == A());
Expand All @@ -188,7 +188,7 @@ int main(int, char**)
C c1(I(std::begin(t1)), I(std::end(t1)), A());
c1 = std::move(c0);
int n = 0;
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, ++n)
for (C::const_iterator i = c1.cbegin(); i != c1.cend(); ++i, (void) ++n)
assert(*i == 10+n);
assert(n == 4);
assert(c1.get_allocator() == A());
Expand Down
Expand Up @@ -32,7 +32,7 @@ int main(int, char**)
C c0(I(std::begin(t)), I(std::end(t)), A(10));
C c = std::move(c0);
int n = 0;
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, ++n)
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, (void) ++n)
assert(*i == n);
assert(n == std::end(t) - std::begin(t));
assert(c0.empty());
Expand All @@ -47,7 +47,7 @@ int main(int, char**)
C c0(I(std::begin(t)), I(std::end(t)), A(10));
C c = std::move(c0);
int n = 0;
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, ++n)
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, (void) ++n)
assert(*i == n);
assert(n == std::end(t) - std::begin(t));
assert(c0.empty());
Expand All @@ -62,7 +62,7 @@ int main(int, char**)
C c0(I(std::begin(t)), I(std::end(t)), A());
C c = std::move(c0);
int n = 0;
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, ++n)
for (C::const_iterator i = c.begin(), e = c.end(); i != e; ++i, (void) ++n)
assert(*i == n);
assert(n == std::end(t) - std::begin(t));
assert(c0.empty());
Expand Down

0 comments on commit 16bf433

Please sign in to comment.