[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196
   test_ctor_under_alloc();
+  test_ctor_with_different_value_type();
 }
----------------
vsapsai wrote:
> Quuxplusone wrote:
> > I suggest that interesting test cases include "array of `int` to vector of `unsigned int`" (trivial, but unimplemented in this patch) and "array of `iostream*` to vector of `ostream*`" (non-trivial because each pointer must be adjusted).
> What is that supposed to test? My `float/int` test is to make sure we have `is_same<_RawSourceTp, _RawDestTp>::value` and don't try to `memcpy` unrelated types. I've chosen `float` and `int` because it is easier for a human to reason about them.
> 
> `int` and `unsigned int` are interested for testing for values that are outside of common range. But in this case you pay more attention to conversion between ints, not to the behaviour of the constructor. That's my interpretation but maybe I've missed some of your intentions.
> My `float/int` test is to make sure we [...] don't try to `memcpy` unrelated types [when it's unsafe to do so].

Right. I suggest two additional tests. The first additional test, `int/unsigned int`, is to verify whether we `memcpy` unrelated types when it //is// safe to do so. The second test, `iostream*/ostream*`, is to verify whether we `memcpy` unrelated types when it is unsafe to `memcpy` them even though they are both of the same "kind" (both pointer types).

These will act as regression tests, to make sure that future changes to the memcpy criteria don't break these cases' behavior (although the first additional test //is// expected to get more efficient).


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

https://reviews.llvm.org/D48342