junrushao1994 commented on a change in pull request #10032:
URL: https://github.com/apache/tvm/pull/10032#discussion_r790328468



##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -65,36 +132,22 @@ class TVMArgsSetter;
  *  It is the unified function function type of TVM.
  *  It corresponds to TVMFunctionHandle in C runtime API.
  */
-class PackedFunc {
+class PackedFunc : public ObjectRef {
  public:
-  /*!
-   * \brief The internal std::function
-   * \param args The arguments to the function.
-   * \param rv The return value.
-   *
-   * \code
-   *   // Example code on how to implemented FType
-   *   void MyPackedFunc(TVMArgs args, TVMRetValue* rv) {
-   *     // automatically convert arguments to desired type.
-   *     int a0 = args[0];
-   *     float a1 = args[1];
-   *     ...
-   *     // automatically assign values to rv
-   *     std::string my_return_value = "x";
-   *     *rv = my_return_value;
-   *   }
-   * \endcode
-   */
-  using FType = std::function<void(TVMArgs args, TVMRetValue* rv)>;
-  /*! \brief default constructor */
-  PackedFunc() {}
   /*! \brief constructor from null */
-  PackedFunc(std::nullptr_t null) {}  // NOLINT(*)
+  PackedFunc(std::nullptr_t null): ObjectRef(nullptr) {}  // NOLINT(*)
   /*!
-   * \brief constructing a packed function from a std::function.
-   * \param body the internal container of packed function.
+   * \brief constructing a packed function from a type-erased callable type.
+   * \param data the internal container of packed function.
    */
-  explicit PackedFunc(FType body) : body_(body) {}
+  template <typename TCallable,
+            typename = std::enable_if_t<
+                std::is_convertible<TCallable, std::function<void(TVMArgs, 
TVMRetValue*)>>::value &&
+                !std::is_base_of<TCallable, PackedFunc>::value>>
+  explicit PackedFunc(TCallable data) {
+    using ObjType = PackedFuncSubObj<TCallable>;
+    data_ = make_object<ObjType>(std::forward<TCallable>(data));

Review comment:
       Would love to think more carefully if we need to implement copy/move 
constructor/operator as rule of five. Would you like to provide some pointers 
here? @tqchen 
   
   Update: no need to do so after a second thought




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to