Kathryn-cat commented on code in PR #649: URL: https://github.com/apache/tvm-ffi/pull/649#discussion_r3741820329
########## include/tvm/ffi/extra/structural_mutate.h: ########## @@ -0,0 +1,904 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/*! + * \file tvm/ffi/extra/structural_mutate.h + * \brief Structural mutation API with optional in-place optimization. + */ +#ifndef TVM_FFI_EXTRA_STRUCTURAL_MUTATE_H_ +#define TVM_FFI_EXTRA_STRUCTURAL_MUTATE_H_ + +#include <tvm/ffi/any.h> +#include <tvm/ffi/c_api.h> +#include <tvm/ffi/cast.h> +#include <tvm/ffi/container/array.h> +#include <tvm/ffi/container/map.h> +#include <tvm/ffi/container/tuple.h> +#include <tvm/ffi/container/variant.h> +#include <tvm/ffi/expected.h> +#include <tvm/ffi/extra/structural_visit.h> +#include <tvm/ffi/extra/visit_error_context.h> +#include <tvm/ffi/function.h> +#include <tvm/ffi/function_details.h> +#include <tvm/ffi/optional.h> +#include <tvm/ffi/reflection/accessor.h> + +#include <cstddef> +#include <exception> +#include <optional> +#include <string> +#include <tuple> +#include <type_traits> +#include <utility> + +namespace tvm { +namespace ffi { + +class StructuralMutatorObj; + +/*! + * \brief ABI callback type for structural mutation. + * + * \param mutator The active structural mutator. + * \param value The borrowed value to transform. + * \return Raw ``TVMFFIAny`` containing the transformed value or an Error. + */ +using FStructuralMutate = TVMFFIAny (*)(StructuralMutatorObj* mutator, AnyView value) noexcept; + +/*! + * \brief ABI callback type for looking up an identity substitution. + * + * \param mutator The active structural mutator. + * \param var The borrowed variable identity to look up. + * \return Raw ``TVMFFIAny`` containing the owning mapped value, FFI None when no mapping exists, + * or an Error. + */ +using FStructuralVarRemapGet = TVMFFIAny (*)(StructuralMutatorObj* mutator, AnyView var) noexcept; + +/*! + * \brief ABI callback type for recording an identity substitution. + * + * \param mutator The active structural mutator. + * \param var The borrowed variable identity to bind. + * \param mapped_value The borrowed replacement value. + * \return Raw ``TVMFFIAny`` containing FFI None on success or an Error. + */ +using FStructuralVarRemapSet = TVMFFIAny (*)(StructuralMutatorObj* mutator, AnyView var, + AnyView mapped_value) noexcept; + +namespace details { + +// Copy and structurally mutate the reflected fields of an object-backed value. +TVM_FFI_INLINE static Expected<Any> MutateReflectedFieldsExpected(StructuralMutatorObj* mutator, + AnyView value) noexcept; + +} // namespace details + +/*! + * \brief VTable ABI for \ref StructuralMutator dispatch. + */ +struct StructuralMutatorVTable { + /*! + * \brief Mutate a value without modifying the source in place. + * + * \param mutator The active structural mutator. + * \param value The borrowed value to mutate. + * \return Raw ``TVMFFIAny`` carrying the transformed value or Error. + */ + FStructuralMutate mutate = nullptr; + /*! + * \brief Mutate a value, permitting an in-place implementation when it is safe. + * + * \param mutator The active structural mutator. + * \param value The borrowed value to transform. + * \return Raw ``TVMFFIAny`` carrying the mutated value or Error. + * + * The returned value may refer to the same object as \p value when the implementation mutates + * that object in place. + */ + FStructuralMutate maybe_inplace_mutate = nullptr; + /*! + * \brief Look up the replacement for a variable identity. + * + * \param mutator The active structural mutator. + * \param var The borrowed variable identity to look up. + * \return Raw ``TVMFFIAny`` carrying the owning replacement, FFI None on a miss, or Error. + */ + FStructuralVarRemapGet var_remap_get = nullptr; + /*! + * \brief Record the replacement for a variable identity. + * + * \param mutator The active structural mutator. + * \param var The borrowed variable identity to bind. + * \param mapped_value The borrowed replacement value. + * \return Raw ``TVMFFIAny`` carrying None or Error. + */ + FStructuralVarRemapSet var_remap_set = nullptr; +}; + +/*! + * \brief Object node of a structural mutator. + */ +class StructuralMutatorObj : public Object { + public: + /*! + * \brief Mutate a value through the mutator vtable. + * + * \param value The value to mutate. + * \return The mutated owning value. + * \throws Error if mutation fails. + * + * This entry point never intentionally mutates \p value in place. Recursive transformations + * also use \ref Mutate. + */ + TVM_FFI_INLINE Any Mutate(AnyView value) { return MutateExpected(value).value(); } + + /*! + * \brief Exception-free form of \ref Mutate. + * + * \param value The value to mutate. + * \return The mutated owning value, or an Error if mutation failed. + */ + TVM_FFI_INLINE Expected<Any> MutateExpected(AnyView value) noexcept { + return details::ExpectedUnsafe::MoveFromTVMFFIAny<Any>((*vtable_->mutate)(this, value)); + } + + /*! + * \brief Mutate a value, permitting an in-place implementation when it is safe. + * + * \param value The borrowed value to transform. + * \return The transformed owning value. + * \throws Error if transformation fails. + * + * The returned value may refer to the same object as \p value. Callers must use the return value + * as the result of the transformation rather than assuming that the input object was reused. + */ + TVM_FFI_INLINE Any MaybeInplaceMutate(AnyView value) { + return MaybeInplaceMutateExpected(value).value(); + } + + /*! + * \brief Exception-free form of \ref MaybeInplaceMutate. + * + * \param value The borrowed value to transform. + * \return The transformed owning value, or an Error if transformation failed. + */ + TVM_FFI_INLINE Expected<Any> MaybeInplaceMutateExpected(AnyView value) noexcept { + return details::ExpectedUnsafe::MoveFromTVMFFIAny<Any>( + (*vtable_->maybe_inplace_mutate)(this, value)); + } + + /*! + * \brief Mutate a value, in-place mutate only when it is uniquely owned. + * + * \param value The borrowed value to transform. + * \return The transformed owning value, or an Error if transformation failed. + */ + TVM_FFI_INLINE Expected<Any> MaybeInplaceMutateIfUniqueExpected(AnyView value) noexcept { + const Object* obj = value.as<Object>(); + if (obj == nullptr || obj->unique()) { + return MaybeInplaceMutateExpected(value); + } + return MutateExpected(value); + } + + /*! + * \brief Apply the default structural mutation with copy-on-write behavior. + * + * \param value The value to mutate. + * \return The mutated value, or an Error if hook dispatch, copying, or field mutation failed. + * + * \note A registered ``__s_mutate__`` hook is dispatched before the reflected fallback and is + * responsible for variable-remap lookup and insertion when it represents a FreeVar + * identity. Automatic FreeVar remapping applies only to the reflected fallback. + */ + TVM_FFI_INLINE Expected<Any> DefaultMutateExpected(AnyView value) noexcept { + int32_t type_index = value.type_index(); + static reflection::TypeAttrColumn column(reflection::type_attr::kStructuralMutate); + AnyView attr = column[type_index]; + if (attr.type_index() != TypeIndex::kTVMFFINone) { + if (attr.type_index() == TypeIndex::kTVMFFIOpaquePtr) { + auto* hook = reinterpret_cast<FStructuralMutate>(attr.cast<void*>()); + return details::ExpectedUnsafe::MoveFromTVMFFIAny<Any>((*hook)(this, value)); + } + if (attr.type_index() == TypeIndex::kTVMFFIFunction) { + return attr.cast<Function>().CallExpected<Any>(this, value); + } + return Unexpected(Error("TypeError", + std::string(reflection::type_attr::kStructuralMutate) + + " must be an opaque function pointer or ffi.Function", + "")); + } + if (type_index < TypeIndex::kTVMFFIStaticObjectBegin) { + return Any(value); + } + + const TVMFFITypeInfo* type_info = TVMFFIGetTypeInfo(type_index); + bool is_free_var = type_info->metadata != nullptr && + type_info->metadata->structural_eq_hash_kind == kTVMFFISEqHashKindFreeVar; + if (is_free_var) { + Expected<Any> mapped_value = VarRemapGetExpected(value); + if (TVM_FFI_PREDICT_FALSE(mapped_value.is_err())) { + return Unexpected(std::move(mapped_value).error()); + } + if (details::ExpectedUnsafe::GetData(mapped_value).type_index() != TypeIndex::kTVMFFINone) { + return mapped_value; + } + } + + Expected<Any> result = details::MutateReflectedFieldsExpected(this, value); + if (TVM_FFI_PREDICT_FALSE(result.is_err()) || !is_free_var) { + return result; + } + + Expected<void> set_result = + VarRemapSetExpected(value, details::ExpectedUnsafe::GetData(result)); + if (TVM_FFI_PREDICT_FALSE(set_result.is_err())) { + return Unexpected(std::move(set_result).error()); + } + return result; + } + + /*! + * \brief Apply custom maybe-in-place mutation, or fall back to non-in-place mutation. + * + * \param value The borrowed value to transform. + * \return The transformed owning value, or an Error if transformation failed. In-place changes + * completed before an Error are not rolled back. + * + * \note In-place mutation is explicitly opt-in. A registered + * ``__s_maybe_inplace_mutate__`` hook may rely on its input being safe to mutate and owns + * any variable-remap handling. When the hook is absent, this method calls + * \ref DefaultMutateExpected. + */ + TVM_FFI_INLINE Expected<Any> DefaultMaybeInplaceMutateExpected(AnyView value) noexcept { + int32_t type_index = value.type_index(); + static reflection::TypeAttrColumn maybe_inplace_mutate_column( + reflection::type_attr::kStructuralMaybeInplaceMutate); + AnyView maybe_inplace_mutate_attr = maybe_inplace_mutate_column[type_index]; + if (maybe_inplace_mutate_attr.type_index() == TypeIndex::kTVMFFIOpaquePtr) { + auto* hook = reinterpret_cast<FStructuralMutate>(maybe_inplace_mutate_attr.cast<void*>()); + return details::ExpectedUnsafe::MoveFromTVMFFIAny<Any>((*hook)(this, value)); + } + if (maybe_inplace_mutate_attr.type_index() == TypeIndex::kTVMFFIFunction) { + return maybe_inplace_mutate_attr.cast<Function>().CallExpected<Any>(this, value); + } + if (maybe_inplace_mutate_attr.type_index() == TypeIndex::kTVMFFINone) { + return DefaultMutateExpected(value); + } + return Unexpected(Error("TypeError", + std::string(reflection::type_attr::kStructuralMaybeInplaceMutate) + + " must be an opaque function pointer or ffi.Function", + "")); + } + + /*! + * \brief Look up the replacement recorded for a variable identity. + * + * \param var The borrowed variable identity to look up. + * \return The owning replacement, FFI None if no replacement exists, or an Error if lookup + * fails. + * + * \note The variable identity must have + * ``kTVMFFISEqHashKindFreeVar`` structural-equality metadata. + */ + TVM_FFI_INLINE Expected<Any> VarRemapGetExpected(AnyView var) noexcept { + return details::ExpectedUnsafe::MoveFromTVMFFIAny<Any>((*vtable_->var_remap_get)(this, var)); + } + + /*! + * \brief Record the replacement for a variable identity. + * + * \param var The borrowed variable identity to bind. + * \param mapped_value The borrowed replacement value. + * \return Successful completion, or an Error if the binding is invalid or cannot be stored. + * + * \note The variable identity must have + * ``kTVMFFISEqHashKindFreeVar`` structural-equality metadata. + */ + TVM_FFI_INLINE Expected<void> VarRemapSetExpected(AnyView var, AnyView mapped_value) noexcept { + return details::ExpectedUnsafe::MoveFromTVMFFIAny<void>( + (*vtable_->var_remap_set)(this, var, mapped_value)); + } + + /*! + * \brief Return the current def-region context. + * \return The active def-region kind. + */ + TVM_FFI_INLINE TVMFFIDefRegionKind def_region_kind() const { return def_region_mode_; } + + /*! + * \brief Temporarily switch the def-region context while invoking \p callback. + * + * \param kind The def-region kind to set during the callback. + * \param callback A nullary callable that performs recursive transformation. + * \return The value returned by \p callback. + */ + template <typename Callback> + TVM_FFI_INLINE auto WithDefRegionKind(TVMFFIDefRegionKind kind, Callback&& callback) + -> decltype(std::forward<Callback>(callback)()) { + class Scope { + public: + Scope(StructuralMutatorObj* mutator, TVMFFIDefRegionKind kind) + : mutator_(mutator), old_kind_(mutator->def_region_mode_) { + mutator_->def_region_mode_ = kind; + } + ~Scope() { mutator_->def_region_mode_ = old_kind_; } + Scope(const Scope&) = delete; + Scope& operator=(const Scope&) = delete; + + private: + StructuralMutatorObj* mutator_; + TVMFFIDefRegionKind old_kind_; + }; + Scope scope(this, kind); + return std::forward<Callback>(callback)(); + } + + /// \cond Doxygen_Suppress + static constexpr const bool _type_mutable = true; + TVM_FFI_DECLARE_OBJECT_INFO("ffi.StructuralMutator", StructuralMutatorObj, Object); + /// \endcond + + protected: + /*! + * \brief Construct a structural mutator from an immutable dispatch vtable. + * \param vtable The non-null dispatch table for this mutator. It must outlive this object. + */ + explicit StructuralMutatorObj(const StructuralMutatorVTable* vtable) : vtable_(vtable) {} + + /*! + * \brief Non-owning pointer to the required ABI dispatch table. + */ + const StructuralMutatorVTable* vtable_ = nullptr; + + /*! + * \brief Current def-region context for def-region-aware structural transformation. + */ + TVMFFIDefRegionKind def_region_mode_ = kTVMFFIDefRegionKindNone; +}; + +/*! + * \brief ObjectRef wrapper for \ref StructuralMutatorObj. + * + * \sa StructuralMutatorObj + */ +class StructuralMutator : public ObjectRef { + public: + /*! + * \brief Construct from an existing mutator object pointer. + * \param n The object pointer to wrap. + */ + explicit StructuralMutator(ObjectPtr<StructuralMutatorObj> n) : ObjectRef(std::move(n)) {} + + /// \cond Doxygen_Suppress + TVM_FFI_DEFINE_OBJECT_REF_METHODS_NOTNULLABLE(StructuralMutator, ObjectRef, StructuralMutatorObj); + /// \endcond +}; + +namespace details { + +/*! + * \brief Mutate the reflected structural fields of an object-backed value. + * + * \param mutator The active structural mutator. + * \param value The object-backed value to mutate. + * \return The original value when no field changes, a transformed shallow copy otherwise, or an + * Error if copying or mutation failed. + */ +TVM_FFI_INLINE static Expected<Any> MutateReflectedFieldsExpected(StructuralMutatorObj* mutator, + AnyView value) noexcept { + const Object* obj = value.as<Object>(); + int32_t type_index = obj->type_index(); + + static reflection::TypeAttrColumn column(reflection::type_attr::kShallowCopy); + AnyView attr = column[type_index]; + if (TVM_FFI_PREDICT_FALSE(attr.type_index() != TypeIndex::kTVMFFIFunction)) { + return Unexpected( + Error("TypeError", + std::string(reflection::type_attr::kShallowCopy) + " must be an ffi.Function", "")); Review Comment: Thanks @Seven-Streams for spotting the issue! The core issue is that TVM-FFI has two representations for String and Bytes: * Length ≤ 7: `kTVMFFISmallStr` / `kTVMFFISmallBytes` * Length ≥ 8: `kTVMFFIStr` / `kTVMFFIBytes` In your example, `"12345678"` is a heap `StringObj`, which goes through `MutateReflectedFieldsExpected`, and the function immediately requests `__ffi_shallow_copy__`. `StringObj` does not define that attribute, so the function fails. ### My proposal Since `String` and `Bytes` are logically atomic, they should be treated like POD for structural traversal, but they remain object-backed for reference counting. I have updated this check. Please take care of Rust side as well. -- 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]
