Variable Design. #2586
Variable Design. #2586
Conversation
qingqing01
requested review from
reyoung,
a user,
pkuyym,
JiayiFeng,
hedaoyuan and
QiJune
|
|
||
| privated: | ||
| // A typed data pointer, set the type when calling Reset. | ||
| boost::any value_; |
There was a problem hiding this comment.
I guess we are going to use boost::any for both saving the pointer and the type information (caffe2::TypeMeta)?
I know that boost::any has a method type(), which returns a typeid comparable with typeid(TypeName). But how could we connect the typeid with a deleter?
| Two aspects should be considered when designing `Variable`, the first is interactive interfaces with external data, the second is type maintaining. Here, we use `boost::any` to hold data and its type. | ||
|
|
||
| ```cpp | ||
| Class Variable { |
|
|
||
| // A destroy call. | ||
| template <class T> | ||
| static void Destroy(void* pointer); |
There was a problem hiding this comment.
In C++, there are two concepts related with here -- destruction and delete. Let us not invent a name destroy, but just call it deleter, as in shared_ptr::shared_ptr.
| static void Destroy(void* pointer); | ||
|
|
||
| // A pointer to save the destructor. | ||
| typedef void (*DestroyCall)(void *); |
There was a problem hiding this comment.
As we represent value_ by boost::any, I think we are not going to use void* in type DestroyCall?
How about
std::function<void(boost::any&)> deleter_;
There was a problem hiding this comment.
I understand that we want to use boost::any instead of void*. This seems promising to me, but I am not sure how should we implement the default deleter as C++'s delete, like shared_ptr has a default deleter.
I approve because I guess we need to start programming before we can figure this out. We can update the document at any future time if necessary.
|
I tried to make an implementation in #2587, which works and doesn't rely on boost. |
|
close this PR, since it had been fixed in #2587. |
This design was completed with @hedaoyuan, @pkuyym @reyoung , @QiJune and @Canpio .
Fix #2545