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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai 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();
 }
----------------
Quuxplusone wrote:
> 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).
Added test for `int32_t/uint32_t`. Size is mentioned explicitly to avoid surprises with testing on different architectures. Tested that such initialization isn't using `memcpy` and still slow.

Do you know ways to verify pointer adjustment for `iostream*/ostream*` using their public API? Because I found a way to do that with accessing `vector::data()` and mucking with pointers. But I don't like this approach because it seems brittle and specific to vector implementation. I'd rather use stable public API. Currently I don't plan to invest much effort into `iostream*/ostream*` test because I agree it is nice to have but `is_same` part is already covered.


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

https://reviews.llvm.org/D48342