[libc++] Use addressof in forward_list.

[Mordante]

Summary

This addresses the usage of operator& in <forward_list>.

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

[• Quuxplusone]

libcxx/include/forward_list

183

Consider adding #include <__memory/addressof.h> here.

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_pred.addressof.compile.pass.cpp

22 ↗

(On Diff #382769)

The comment on line 11 is talking about forward_list&& x, but here you're passing lvalue l. Which is correct? And should there be two test cases, one for lvalues and one for rvalues?

[Mordante]

libcxx/include/forward_list

183

We already include <memory> at line 188 which should provide addressof.

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_pred.addressof.compile.pass.cpp

22 ↗

(On Diff #382769)

I copy pasted this from the existing test, which has the same issue. Having an rvalue in the comment and using an lvalue. It seems there are no tests for rvalues.

Looking at the implementation of forward_list all 4 merge functions were affected by this issue.

void merge(forward_list& x);
void merge(forward_list&& x);
template<class Compare> void merge(forward_list& x, Compare comp);
template<class Compare> void merge(forward_list&& x, Compare comp);

I'll fix this test and add the 3 missing addressof compile tests.
The missing runtime tests will be done in a separate patch.

[• Quuxplusone]

libcxx/include/forward_list

183

Yes, and certainly it's included transitively from a lot of places. Just trying to sneak in some granularization. :) No worries.

[Mordante]

Addresses review comments.

[Mordante]

Rebase to fix CI.

libcxx/include/forward_list

template <class T, class Allocator, class Predicate>
     typename forward_list<T, Allocator>::size_type
     erase_if(forward_list<T, Allocator>& c, Predicate pred);    // C++20
 
 }  // std
 
 */
 
 #include <__config>
 #include <__utility/forward.h>
 #include <algorithm>
 #include <initializer_list>
 #include <iterator>
 #include <limits>
 #include <memory>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
forward_list<_Tp, _Alloc>::unique(_BinaryPredicate __binary_pred)
     return (__remove_return_type) __count_removed;
 }
 
 template <class _Tp, class _Alloc>
 template <class _Compare>
 void
 forward_list<_Tp, _Alloc>::merge(forward_list& __x, _Compare __comp)
 {
-    if (this != &__x)
+    if (this != _VSTD::addressof(__x))
     {
         base::__before_begin()->__next_ = __merge(base::__before_begin()->__next_,
                                                     __x.__before_begin()->__next_,
                                                     __comp);
         __x.__before_begin()->__next_ = nullptr;
     }
 }
 

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.addressof.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <forward_list>
+
+// template <class Compare> void merge(forward_list& x);
+
+// Validate whether the operation properly guards against ADL-hijacking operator&
+
+#include <forward_list>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::forward_list<operator_hijacker> lo;
+  std::forward_list<operator_hijacker> l;
+  lo.merge(l);
+}

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.addressof.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <forward_list>
+
+// template <class Compare> void merge(forward_list& x, Compare comp);
+
+// Validate whether the operation properly guards against ADL-hijacking operator&
+
+#include <forward_list>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::forward_list<operator_hijacker> lo;
+  std::forward_list<operator_hijacker> l;
+  lo.merge(l, std::less<operator_hijacker>());
+}

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.addressof.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <forward_list>
+
+// template <class Compare> void merge(forward_list&& x);
+
+// Validate whether the operation properly guards against ADL-hijacking operator&
+
+#include <forward_list>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::forward_list<operator_hijacker> lo;
+  std::forward_list<operator_hijacker> l;
+  lo.merge(std::move(l));
+}

libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.addressof.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <forward_list>
+
+// template <class Compare> void merge(forward_list&& x, Compare comp);
+
+// Validate whether the operation properly guards against ADL-hijacking operator&
+
+#include <forward_list>
+
+#include "test_macros.h"
+#include "operator_hijacker.h"
+
+void test() {
+  std::forward_list<operator_hijacker> lo;
+  std::forward_list<operator_hijacker> l;
+  lo.merge(std::move(l), std::less<operator_hijacker>());
+}