[libcxx-commits] [PATCH] D151953: [libc++] Fix std::copy and std::move for ranges with potentially overlapping tail padding

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__utility/is_pointer_in_range.h:40
     const _Tp* __begin, const _Tp* __end, const _Up* __ptr) {
-  static_assert(!is_function<_Tp>::value && !is_function<_Up>::value,
+  static_assert(!is_function<_Tp*>::value && !is_function<_Up*>::value,
                 "__is_pointer_in_range should not be called with function pointers");
----------------
This is never true since you're passing `_Tp*`. `is_function<_Tp>` was correct, I think.

Edit: Actually the overload won't match if passed a function, so let's remove this altogether. You can probably remove some includes too.


================
Comment at: libcxx/include/__utility/is_pointer_in_range.h:42-43
                 "__is_pointer_in_range should not be called with function pointers");
-  static_assert(!is_member_pointer<_Tp>::value && !is_member_pointer<_Up>::value,
+  static_assert(!is_member_pointer<_Tp*>::value && !is_member_pointer<_Up*>::value,
                 "__is_pointer_in_range should not be called with member pointers");
 
----------------
If `T` is a ptr to member, this function doesn't match so I don't think it's too useful to have this `static_assert` at all: https://godbolt.org/z/8z8KPn9e5


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp:53
+
+    { // Make sure that padding bits aren't copied
+      Derived src(1, 2, 3);
----------------
Since this test case and the one below it are not dependent on any template params, let's move them out of this function. It confused me at first. Probably applies to the other test files as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151953