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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
