gemini-code-assist[bot] commented on code in PR #647:
URL: https://github.com/apache/tvm-ffi/pull/647#discussion_r3486159761
##########
include/tvm/ffi/container/tuple.h:
##########
@@ -338,10 +338,8 @@ struct TypeTraits<Tuple<Types...>> : public
ObjectRefTypeTraitsBase<Tuple<Types.
}
};
-namespace details {
template <typename... T, typename... U>
-inline constexpr bool type_contains_v<Tuple<T...>, Tuple<U...>> =
(type_contains_v<T, U> && ...);
-} // namespace details
+inline constexpr bool type_subsumes_v<Tuple<T...>, Tuple<U...>> =
(type_subsumes_v<T, U> && ...);
Review Comment:
The partial specialization of `type_subsumes_v` for `Tuple` uses a fold
expression `(type_subsumes_v<T, U> && ...)` that expands both parameter packs
`T` and `U` together. If the two tuples have different sizes (i.e.,
`sizeof...(T) != sizeof...(U)`), this will result in a hard compilation error
due to mismatched pack lengths, rather than evaluating to `false`.
We can resolve this by introducing a helper trait that checks if the sizes
are equal before performing the fold expansion.
```c
namespace details {
template <typename T, typename U, bool SameSize>
struct tuple_subsumes : std::false_type {};
template <typename... T, typename... U>
struct tuple_subsumes<Tuple<T...>, Tuple<U...>, true>
: std::bool_constant<(type_subsumes_v<T, U> && ...)> {};
} // namespace details
template <typename... T, typename... U>
inline constexpr bool type_subsumes_v<Tuple<T...>, Tuple<U...>> =
details::tuple_subsumes<Tuple<T...>, Tuple<U...>, (sizeof...(T) ==
sizeof...(U))>::value;
```
##########
include/tvm/ffi/type_traits.h:
##########
@@ -118,22 +118,39 @@ inline std::string TypeIndexToTypeKey(int32_t type_index)
{
return std::string(type_info->type_key.data, type_info->type_key.size);
}
-namespace details {
/*!
- * \brief Check whether `Derived` can reuse `Base` storage directly.
+ * \brief Whether TargetType subsumes SourceType for direct storage reuse.
*
- * \tparam Base The base type.
- * \tparam Derived The derived type.
- * \return True if Derived's storage can be used as Base's storage, false
otherwise.
+ * The target type is first and the source type is second. The result is true
+ * exactly when every SourceType value can reuse TargetType storage without
+ * conversion.
+ *
+ * \tparam TargetType The target storage type.
+ * \tparam SourceType The source value type.
*/
-template <typename Base, typename Derived>
-inline constexpr bool type_contains_v =
- std::is_base_of_v<Base, Derived> || std::is_same_v<Base, Derived>;
+template <typename TargetType, typename SourceType>
+inline constexpr bool type_subsumes_v =
+ std::is_base_of_v<TargetType, SourceType> || std::is_same_v<TargetType,
SourceType>;
// Special case for Any, which can store any compatible value directly.
-template <typename Derived>
-inline constexpr bool type_contains_v<Any, Derived> = true;
-} // namespace details
+template <typename SourceType>
+inline constexpr bool type_subsumes_v<Any, SourceType> = true;
+
+/*!
+ * \brief Whether RefType contains every ObjectType instance.
+ *
+ * The containing reference type is first and the contained object type is
+ * second. The default is true exactly when RefType has an exact container type
+ * and ObjectType derives from that container type. Direct specializations can
+ * provide the proof for non-exact reference types.
+ *
+ * \tparam RefType The object reference type.
+ * \tparam ObjectType The object type.
+ */
+template <typename RefType, typename ObjectType>
+inline constexpr bool object_ref_contains_v =
+ RefType::_type_container_is_exact &&
+ std::is_base_of_v<typename RefType::ContainerType, ObjectType>;
Review Comment:
The default implementation of `object_ref_contains_v` directly accesses
`RefType::_type_container_is_exact` and `typename RefType::ContainerType`. If
`RefType` is a primitive type (like `int` or `float`) or any other type that
does not define these members, instantiating `object_ref_contains_v` will
result in a hard compilation error rather than triggering the helpful
`static_assert` in `GetRef`.
We can make `object_ref_contains_v` SFINAE-safe by using a helper trait to
detect the presence of these members first.
```c
namespace details {
template <typename T, typename = void>
struct has_container_type : std::false_type {};
template <typename T>
struct has_container_type<T, std::void_t<typename T::ContainerType,
decltype(T::_type_container_is_exact)>>
: std::true_type {};
template <typename RefType, typename ObjectType, bool HasContainer =
has_container_type<RefType>::value>
struct object_ref_contains_impl : std::false_type {};
template <typename RefType, typename ObjectType>
struct object_ref_contains_impl<RefType, ObjectType, true> {
static constexpr bool value =
RefType::_type_container_is_exact &&
std::is_base_of_v<typename RefType::ContainerType, ObjectType>;
};
} // namespace details
template <typename RefType, typename ObjectType>
inline constexpr bool object_ref_contains_v =
details::object_ref_contains_impl<RefType, ObjectType>::value;
```
--
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]
