[oneDPL][ranges][test] + support arbitrary n for ranges tests #1908
Conversation
MikeDvorskiy
commented
Oct 16, 2024
MikeDvorskiy
commented
- [oneDPL][ranges][test] + support arbitrary n for 'data_in' test mode
| template<typename Policy, typename DataGen> | ||
| host_subrange_impl(Policy&&, int n, DataGen gen) | ||
| { | ||
| mem = alloc.allocate(n); |
There was a problem hiding this comment.
allocate() produces uninitialized storage. You have to construct the objects first to avoid UB.
There was a problem hiding this comment.
- strictly speaking - "yes", but the tests use only trivial types so far and now we don't introduce new non-trival types.
- and the same thing was before the changes.
There was a problem hiding this comment.
Let's create an issue or leave a TODO comment in the test in order not to forget about it. You can also add a static_assert which makes sure that the type being tested is trivial.
| ~host_subrange_impl() | ||
| { | ||
| if(mem) | ||
| alloc.deallocate(mem, view.size()); |
There was a problem hiding this comment.
Here you need to destroy the objects first.
There was a problem hiding this comment.
The the comment above.
We use the trivial types that doesn't requires "special" destroying.
Probably, it makes sense to add a static assert to check triviality of type T.
There was a problem hiding this comment.
I agree regarding the static assert.
| Container cont_in(exec, data_in, n_in); | ||
| Container cont_out(exec, data_out, n_out); | ||
| auto src_view = tr_in(std::views::all(cont_in())); | ||
| auto out_view = tr_out(std::views::all(cont_exp())); |
There was a problem hiding this comment.
It looks like there should be cont_out rather than cont_exp, so that out_view matches cont_out.
| Container cont_out(exec, data_out, n_out); | ||
| auto src_view1 = tr_in(std::views::all(cont_in1())); | ||
| auto src_view2 = tr_in(std::views::all(cont_in2())); | ||
| auto expected_view = tr_in(std::views::all(cont_exp())); |
There was a problem hiding this comment.
Should you apply tr_out to cont_exp similarly to cont_out below?
