[libc++] Use std::addressof in std::function::target

[ldionne]

Summary

This guards against hostile overloads of operator&. Thanks to Peter Dimov
for the report.

[• Quuxplusone]

libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/addressof.pass.cpp

21

template<class T> friend void operator&(T) {} to expand its power a bit further.
Or actually, even better, go all out with the traditional ADL-proofing test:

struct Incomplete;
template<class T> struct Holder { T t; };
template<class T> struct Callable {
    int operator()() const { return 1; }
};

int main(int, char**) {
    std::function<int()> f = Callable<Holder<Incomplete>>();
    assert(f() == 1);
    return 0;
}

[Mordante]

LGTM except for a minor issue.

libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/addressof.pass.cpp

23

Can you use test/support/operator_hijacker.h's operator_hijacker instead? If not I would like the operator&() to be deleted.

[ldionne]

Address review comments

[• Quuxplusone]

I don't see the point of having two ADL-proofing tests (robust_against_adl.pass.cpp and the new one), and I wonder why the old robust_against_adl.pass.cpp bothered to test three different cases but you're adding only one new case (like, should there be a matching test for std::function<int()> as well as std::function<void()>?) but anyway this certainly seems code-correct and sufficiently-tested at this point. :)

[ldionne]

In D116380#3212861, @Quuxplusone wrote:

I don't see the point of having two ADL-proofing tests (robust_against_adl.pass.cpp and the new one), and I wonder why the old robust_against_adl.pass.cpp bothered to test three different cases but you're adding only one new case (like, should there be a matching test for std::function<int()> as well as std::function<void()>?) but anyway this certainly seems code-correct and sufficiently-tested at this point. :)

The added test case in addressof.pass.cpp is not checking for ADL proofing, but really for the specific case of using operator& instead of addressof. For example, the ADL proofing test would pass if we used __f_.__target().operator&(), but the operator hijacking one would not. Since we can't reasonably use .operator&() (other stuff would fail), this is mostly academic, but I think it still makes sense to have a test whose *intent* is specifically to test the reported bug.

[ldionne]

Rebase onto main for CI.

[ldionne]

Fix C++03

libcxx/include/__functional/function.h

 #define _LIBCPP___FUNCTIONAL_FUNCTION_H
 
 #include <__config>
 #include <__debug>
 #include <__functional/binary_function.h>
 #include <__functional/invoke.h>
 #include <__functional/unary_function.h>
 #include <__iterator/iterator_traits.h>
+#include <__memory/addressof.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/compressed_pair.h>
 #include <__memory/shared_ptr.h>
 #include <exception>
 #include <memory> // TODO: replace with <__memory/__builtin_new_allocator.h>
 #include <type_traits>
 #include <utility>
 

 
 #ifndef _LIBCPP_NO_RTTI
 
 template<class _Fp, class _Alloc, class _Rp, class ..._ArgTypes>
 const void*
 __func<_Fp, _Alloc, _Rp(_ArgTypes...)>::target(const type_info& __ti) const _NOEXCEPT
 {
     if (__ti == typeid(_Fp))
-        return &__f_.__target();
+        return _VSTD::addressof(__f_.__target());
     return nullptr;
 }
 
 template<class _Fp, class _Alloc, class _Rp, class ..._ArgTypes>
 const std::type_info&
 __func<_Fp, _Alloc, _Rp(_ArgTypes...)>::target_type() const _NOEXCEPT
 {
     return typeid(_Fp);

 
 #ifndef _LIBCPP_NO_RTTI
 
 template<class _Fp, class _Alloc, class _Rp>
 const void*
 __func<_Fp, _Alloc, _Rp()>::target(const type_info& __ti) const
 {
     if (__ti == typeid(_Fp))
-        return &__f_.first();
+        return _VSTD::addressof(__f_.first());
     return (const void*)0;
 }
 
 template<class _Fp, class _Alloc, class _Rp>
 const std::type_info&
 __func<_Fp, _Alloc, _Rp()>::target_type() const
 {
     return typeid(_Fp);

libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/addressof.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
+//
+//===----------------------------------------------------------------------===//
+
+// <functional>
+
+// class function<R(ArgTypes...)>
+
+// This test runs in C++03, but we have deprecated using std::function in C++03.
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
+// Make sure we can use std::function with a type that has a hostile overload
+// of operator&().
+
+#include <functional>
+#include <cassert>
+
+#include "operator_hijacker.h"
+
+struct TrapAddressof : operator_hijacker {
+    int operator()() const { return 1; }
+};
+
+int main(int, char**) {
+    std::function<int()> f = TrapAddressof();
+    assert(f() == 1);
+    return 0;
+}

libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/robust_against_adl.pass.cpp

 // <functional>
 
 #include <functional>
 
 #include "test_macros.h"
 
 struct Incomplete;
 template<class T> struct Holder { T t; };
+
 typedef Holder<Incomplete> *Ptr;
 
+template<class T>
+struct Callable {
+    void operator()() const { }
+};
+
 Ptr no_args() { return nullptr; }
 Ptr one_arg(Ptr p) { return p; }
 Ptr two_args(Ptr p, Ptr) { return p; }
 Ptr three_args(Ptr p, Ptr, Ptr) { return p; }
 Ptr four_args(Ptr p, Ptr, Ptr, Ptr) { return p; }
 
 void one_arg_void(Ptr) { }
 
-int main(int, char**)
-{
+int main(int, char**) {
     Ptr x = nullptr;
     std::function<Ptr()> f(no_args); f();
     std::function<Ptr(Ptr)> g(one_arg); g(x);
     std::function<void(Ptr)> h(one_arg_void); h(x);
+    std::function<void()> i(Callable<Holder<Incomplete>>{});
     return 0;
 }