tqchen commented on code in PR #17103: URL: https://github.com/apache/tvm/pull/17103#discussion_r1644479468
########## include/tvm/runtime/packed_func.h: ########## @@ -833,35 +859,17 @@ class TVMRetValue : public TVMPODValue_ { using TVMPODValue_::AsObjectRef; using TVMPODValue_::IsObjectRef; - TVMRetValue(const TVMRetValue& other) : TVMPODValue_() { this->Assign(other); } // conversion operators - operator std::string() const { - if (type_code_ == kTVMDataType) { - return DLDataType2String(operator DLDataType()); - } else if (type_code_ == kTVMBytes) { - return *ptr<std::string>(); - } - TVM_CHECK_TYPE_CODE(type_code_, kTVMStr); - return *ptr<std::string>(); - } - operator DLDataType() const { - if (type_code_ == kTVMStr) { - return String2DLDataType(operator std::string()); - } - TVM_CHECK_TYPE_CODE(type_code_, kTVMDataType); - return value_.v_type; - } - operator DataType() const { return DataType(operator DLDataType()); } template <typename FType> operator TypedPackedFunc<FType>() const { return TypedPackedFunc<FType>(operator PackedFunc()); } // Assign operators TVMRetValue& operator=(TVMRetValue&& other) { - this->Clear(); - value_ = other.value_; - type_code_ = other.type_code_; - other.type_code_ = kTVMNullptr; + std::swap(value_, other.value_); + std::swap(type_code_, other.type_code_); + std::swap(f_deleter_, other.f_deleter_); + std::swap(f_deleter_arg_, other.f_deleter_arg_); Review Comment: I think we are doing a bit too much special casing for this case. Instead, a simpler approach that might work is to leverage the Object system, which might align with our future directions of better FFI surrounding the object system ########## include/tvm/runtime/packed_func.h: ########## @@ -833,35 +859,17 @@ class TVMRetValue : public TVMPODValue_ { using TVMPODValue_::AsObjectRef; using TVMPODValue_::IsObjectRef; - TVMRetValue(const TVMRetValue& other) : TVMPODValue_() { this->Assign(other); } // conversion operators - operator std::string() const { - if (type_code_ == kTVMDataType) { - return DLDataType2String(operator DLDataType()); - } else if (type_code_ == kTVMBytes) { - return *ptr<std::string>(); - } - TVM_CHECK_TYPE_CODE(type_code_, kTVMStr); - return *ptr<std::string>(); - } - operator DLDataType() const { - if (type_code_ == kTVMStr) { - return String2DLDataType(operator std::string()); - } - TVM_CHECK_TYPE_CODE(type_code_, kTVMDataType); - return value_.v_type; - } - operator DataType() const { return DataType(operator DLDataType()); } template <typename FType> operator TypedPackedFunc<FType>() const { return TypedPackedFunc<FType>(operator PackedFunc()); } // Assign operators TVMRetValue& operator=(TVMRetValue&& other) { - this->Clear(); - value_ = other.value_; - type_code_ = other.type_code_; - other.type_code_ = kTVMNullptr; + std::swap(value_, other.value_); + std::swap(type_code_, other.type_code_); + std::swap(f_deleter_, other.f_deleter_); + std::swap(f_deleter_arg_, other.f_deleter_arg_); Review Comment: I think we are doing too much special casing for this case. Instead, a simpler approach that might work is to leverage the Object system, which might align with our future directions of better FFI surrounding the object system -- 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: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org