Support clone of SGObjects' arrays #4239
Conversation
src/shogun/lib/any.h
Outdated
| -> decltype((void)begin[0]->clone()) | ||
| { | ||
| std::transform( | ||
| begin, end, dst, [](auto each) { return each->clone(); }); |
There was a problem hiding this comment.
What if some elements of the array are null?
| @@ -40,6 +40,20 @@ namespace shogun | |||
| namespace any_detail | |||
| { | |||
|
|
|||
| inline CSGObject* copy_object(CSGObject* object) | |||
|
Fine to merge from my side occe CI is happy |
|
Seems CI cannot be happy as this invalidates the cache ... |
| std::copy(begin, end, dst); | ||
| } | ||
|
|
||
| void copy_array(CSGObject** begin, CSGObject** end, CSGObject** dst); |
There was a problem hiding this comment.
There is a problem here:
This will only match if the pointer type is CSGObject but not for any subclass (discovered this in my own tests when writing clone). See below how to reproduce
The thing is: we decided to register the base classes directly, e.g. CKernel, without casting them to CSGObject
I think we will have to use the decltype trick you did for cloning single objects ....
Or maybe a type trait as we have done for put and get?
There was a problem hiding this comment.
Actually already the simple basic_checks test reveals it: double free since the object wasn't cloned:
shogun/tests/unit/base/SGObject_unittest.cc
Line 192 in c526de9
| TEST(Any, clone_array_sgobject) | ||
| { | ||
| int src_len = 3; | ||
| CSGObject** src = new CSGObject*[src_len]; |
There was a problem hiding this comment.
if you make a test where the pointer type is Object**, this will go into the std::copy branch above
No description provided.