[libcxx-commits] [PATCH] D130631: [libc++] Implement P0288R9 (move_only_function)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__functional/move_only_function_impl.h:39-54
+#ifndef _LIBCPP_MOVE_ONLY_FUNCTION_CV
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_CV
+#endif
+
+#ifndef _LIBCPP_MOVE_ONLY_FUNCTION_REF
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_REF
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS _LIBCPP_MOVE_ONLY_FUNCTION_CV&
----------------
ldionne wrote:
> Is there a reason why you allow these macros to *not* be defined? I would simply document the macros that this file needs in a comment and also have a `#ifndef #error #endif` for each of them.
There are a two reasons:
1. It's a lot harder to define `_LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS`
2. Having defaults for them allows editors to parse the file properly, instead of generating errors all over the place.


================
Comment at: libcxx/include/__utility/pointer_int_pair.h:59
+public:
+  _LIBCPP_HIDE_FROM_ABI __pointer_int_pair(nullptr_t) {}
+  _LIBCPP_HIDE_FROM_ABI __pointer_int_pair(_Pointer __ptr_value, _IntType __int_value)
----------------
ldionne wrote:
> `constexpr`? That will make it constinit-able.
Doesn't apply anymore.


================
Comment at: libcxx/include/__utility/pointer_int_pair.h:72
+
+  _LIBCPP_HIDE_FROM_ABI _IntType __get_value() const { return (__value_ & __int_mask) >> __extra_bits; }
+
----------------
ldionne wrote:
> This could be `constexpr`?
I guess so, but I don't think it makes a lot of sense to mark `constexpr` since nothing else is `constexpr`, or is there some benefit?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130631/new/

https://reviews.llvm.org/D130631