junrushao commented on code in PR #676:
URL: https://github.com/apache/tvm-ffi/pull/676#discussion_r3615048746
##########
python/tvm_ffi/dataclasses/gen_abi_cpp.py:
##########
@@ -319,19 +319,35 @@ def __init__(self, emitted_infos: list[TypeInfo]) -> None:
def _lower_object(self, schema: TypeSchema) -> _Carrier:
type_index = schema.origin_type_index
if type_index == _OBJECT_TYPE_INDEX or schema.origin == "Object":
- return _Carrier("::tvm::ffi::ObjectPtr<::tvm::ffi::Object>", 8, 8)
+ return _Carrier("::tvm::ffi::Arc<::tvm::ffi::Object>", 8, 8)
builtin = self.builtins.get(schema.origin_type_index)
if builtin is not None:
if builtin.value_type is None:
raise ValueError(
f"Static TVM-FFI type {schema.origin!r} has no supported
C++ value wrapper"
)
+ if builtin.value_type.startswith("::tvm::ffi::ObjectPtr<"):
+ return _Carrier(f"::tvm::ffi::Arc<{builtin.object_type}>", 8,
8)
return _Carrier(builtin.value_type, builtin.size,
builtin.alignment)
if schema.origin_type_index < _DYNAMIC_OBJECT_TYPE_INDEX_BEGIN:
raise ValueError(f"Schema {schema!r} is not a registered object
type")
info = _lookup_or_register_type_info_from_type_key(schema.origin)
self.dependencies[info.type_index] = info
object_type = _cpp_name(info.type_key).qualified
+ return _Carrier(f"::tvm::ffi::Arc<{object_type}>", 8, 8)
+
+ def _lower_nullable_object(self, schema: TypeSchema) -> _Carrier:
+ type_index = schema.origin_type_index
+ if type_index == _OBJECT_TYPE_INDEX or schema.origin == "Object":
+ object_type = "::tvm::ffi::Object"
+ elif (builtin := self.builtins.get(type_index)) is not None:
+ object_type = builtin.object_type
+ else:
+ if type_index < _DYNAMIC_OBJECT_TYPE_INDEX_BEGIN:
+ raise ValueError(f"Schema {schema!r} is not a registered
object type")
+ info = _lookup_or_register_type_info_from_type_key(schema.origin)
+ self.dependencies[info.type_index] = info
+ object_type = _cpp_name(info.type_key).qualified
return _Carrier(f"::tvm::ffi::ObjectPtr<{object_type}>", 8, 8)
Review Comment:
Done — factored the dynamic-object lookup into
`_get_dynamic_object_cpp_type` and reused it from both lowering paths.
##########
include/tvm/ffi/any.h:
##########
@@ -871,6 +871,44 @@ struct AnyEqual {
}
};
+// Defer these definitions until any.h so null Arc values can be rejected with
+// TypeError while object.h stays below the error layer.
+//! \cond Doxygen_Suppress
+template <typename TObject>
+TVM_FFI_INLINE void
+TypeTraits<Arc<TObject>, std::enable_if_t<is_object_subclass_v<TObject> &&
+ std::is_same_v<TObject,
std::remove_cv_t<TObject>>>>::
+ CopyToAnyView(const Arc<TObject>& src, TVMFFIAny* result) {
+ const ObjectPtr<TObject>& ptr = src;
+ if (TVM_FFI_PREDICT_FALSE(ptr == nullptr)) {
Review Comment:
Done — removed the dedicated Arc conversion definitions and runtime null
checks; the traits now rely on the Arc invariant.
##########
include/tvm/ffi/any.h:
##########
@@ -871,6 +871,44 @@ struct AnyEqual {
}
};
+// Defer these definitions until any.h so null Arc values can be rejected with
+// TypeError while object.h stays below the error layer.
+//! \cond Doxygen_Suppress
+template <typename TObject>
+TVM_FFI_INLINE void
+TypeTraits<Arc<TObject>, std::enable_if_t<is_object_subclass_v<TObject> &&
+ std::is_same_v<TObject,
std::remove_cv_t<TObject>>>>::
+ CopyToAnyView(const Arc<TObject>& src, TVMFFIAny* result) {
+ const ObjectPtr<TObject>& ptr = src;
+ if (TVM_FFI_PREDICT_FALSE(ptr == nullptr)) {
+ TVM_FFI_THROW(TypeError) << "Cannot convert null Arc to expected object
type `"
+ << TObject::_type_key << "`";
+ }
+ TVMFFIObject* obj_ptr =
details::ObjectUnsafe::TVMFFIObjectPtrFromObjectPtr(ptr);
+ result->type_index = obj_ptr->type_index;
+ result->zero_padding = 0;
+ TVM_FFI_CLEAR_PTR_PADDING_IN_FFI_ANY(result);
+ result->v_obj = obj_ptr;
+}
+
+template <typename TObject>
+TVM_FFI_INLINE void
+TypeTraits<Arc<TObject>, std::enable_if_t<is_object_subclass_v<TObject> &&
+ std::is_same_v<TObject,
std::remove_cv_t<TObject>>>>::
+ MoveToAny(Arc<TObject> src, TVMFFIAny* result) {
+ ObjectPtr<TObject>& ptr = src;
+ if (TVM_FFI_PREDICT_FALSE(ptr == nullptr)) {
+ TVM_FFI_THROW(TypeError) << "Cannot convert null Arc to expected object
type `"
Review Comment:
Done — removed the move-path runtime check as well; it now delegates through
the existing `ObjectPtr` traits.
##########
include/tvm/ffi/object.h:
##########
@@ -53,6 +53,12 @@ namespace ffi {
*/
struct UnsafeInit {};
+template <typename T>
+class Arc;
+
+template <typename T, typename... Args>
+Arc<T> make_arc(Args&&... args);
Review Comment:
Done — removed the namespace-scope declaration entirely, as suggested in the
follow-up; the Arc friend declaration covers `make_arc`.
##########
include/tvm/ffi/object.h:
##########
@@ -1388,6 +1480,51 @@ struct TypeTraits<ObjectPtr<TObject>,
}
};
+/*!
+ * \brief Type traits for a non-null owning pointer to an unqualified Object
subclass.
+ * \tparam TObject The unqualified Object subclass.
+ */
+template <typename TObject>
+struct TypeTraits<Arc<TObject>,
+ std::enable_if_t<is_object_subclass_v<TObject> &&
+ std::is_same_v<TObject,
std::remove_cv_t<TObject>>>>
+ : public TypeTraitsBase {
+ static constexpr int32_t field_static_type_index = TypeIndex::kTVMFFIObject;
+
+ // Defined in any.h so the null-rejection path can use the error layer.
+ TVM_FFI_INLINE static void CopyToAnyView(const Arc<TObject>& src, TVMFFIAny*
result);
Review Comment:
Done — `TypeTraits<Arc<T>>::CopyToAnyView` and `MoveToAny` now delegate
directly to `TypeTraits<ObjectPtr<T>>`, relying on the Arc invariant.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
