This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 79697974 [FEAT][FFI] Accept flexible callback return types in C++
StructuralMap (#725)
79697974 is described below
commit 796979744f8ec4dc7c72238f493be7acd00f89e9
Author: Tianqi Chen <[email protected]>
AuthorDate: Tue Sep 1 15:28:22 2026 -0400
[FEAT][FFI] Accept flexible callback return types in C++ StructuralMap
(#725)
Add flexible callback-return support to C++ `StructuralMap` through two
additive `Expected<T>` conversions.
- Preserve the existing `Expected(T)`, `Expected(Error)`, and
`Expected(Unexpected<E>)` constructors unchanged.
- Accept a bare `U` when `U` is implicitly convertible to `T`, using the
C++17-compatible implicit subset of `std::expected`'s value-constructor
rules.
- Convert `Expected<U>` to `Expected<T>`; when `T` subsumes `U`, move
the shared `Any data_` representation directly, and otherwise convert
success values while propagating errors.
- Document that StructuralMap callbacks may return a replacement value,
`Expected<T>`, `Error`/`Unexpected`, or throw `Error`, while retaining
the diagnostic for unsupported return types.
StructuralWalk and the Rust bindings are unchanged.
---
include/tvm/ffi/expected.h | 63 +++++++++++++++++++++++++++++++
include/tvm/ffi/extra/structural_mutate.h | 24 ++++++++----
tests/cpp/extra/test_structural_mutate.cc | 58 ++++++++++++++++++++++++++++
tests/cpp/test_expected.cc | 29 ++++++++++++++
4 files changed, 166 insertions(+), 8 deletions(-)
diff --git a/include/tvm/ffi/expected.h b/include/tvm/ffi/expected.h
index 14412ac9..4d31fdce 100644
--- a/include/tvm/ffi/expected.h
+++ b/include/tvm/ffi/expected.h
@@ -64,10 +64,25 @@ template <typename E>
Unexpected(E) -> Unexpected<E>;
#endif
+template <typename T>
+class Expected;
+
namespace details {
struct ExpectedUnsafe;
+template <typename T>
+inline constexpr bool is_expected_v = false;
+
+template <typename T>
+inline constexpr bool is_expected_v<Expected<T>> = true;
+
+template <typename T>
+inline constexpr bool is_unexpected_v = false;
+
+template <typename E>
+inline constexpr bool is_unexpected_v<Unexpected<E>> = true;
+
} // namespace details
/*!
@@ -111,6 +126,52 @@ class Expected {
// NOLINTNEXTLINE(google-explicit-constructor,runtime/explicit)
Expected(T value) : data_(Any(std::move(value))) {}
+ /*!
+ * \brief Implicit constructor from a different success value type.
+ * \tparam U Source type implicitly convertible to ``T``.
+ * \param value The success value to convert.
+ */
+ // Excludes Error, Unexpected, and Expected deliberately: Any subsumes all
three, so without
+ // these an Expected<Any> built from an error would store it as a success
value. The Expected
+ // exclusion also keeps this overload disjoint from Expected(Expected<U>)
instead of relying on
+ // partial ordering to choose between two paths that must agree.
+ //
+ // std::expected admits constructible sources and uses C++20 explicit(bool)
to separate its
+ // implicit subset. Under C++17, convertibility keeps exactly that implicit
subset and drops only
+ // explicit-only conversions; is_constructible plus explicit(bool) can
extend it after an upgrade.
+ template <typename U, typename =
std::enable_if_t<!details::is_expected_v<std::decay_t<U>> &&
+
!details::is_unexpected_v<std::decay_t<U>> &&
+ !std::is_base_of_v<Error,
std::decay_t<U>> &&
+ std::is_convertible_v<U,
T>>>
+ // NOLINTNEXTLINE(google-explicit-constructor,runtime/explicit)
+ Expected(U&& value) : data_(Any(T(std::forward<U>(value)))) {}
+
+ /*!
+ * \brief Implicit converting constructor from another Expected success type.
+ * \tparam U Source success type whose storage is subsumed by or implicitly
convertible to ``T``.
+ * \param other The Expected value to convert.
+ */
+ // Subsumption belongs only here: this source already contains a
materialized U or Error whose
+ // representation may be reused. Applying type_subsumes_v<Any, U> to the
bare-value constructor
+ // would accept every U, including types that cannot be materialized as Any,
and fail in its body.
+ // Taking by value gives a local to move from, copying an lvalue source and
moving an rvalue. The
+ // implicit copy constructor still wins for Expected<T> itself by the
non-template tiebreaker.
+ template <typename U,
+ typename = std::enable_if_t<!std::is_void_v<U> &&
+ (type_subsumes_v<T, U> ||
std::is_convertible_v<U, T>)>>
+ // NOLINTNEXTLINE(google-explicit-constructor,runtime/explicit)
+ Expected(Expected<U> other) {
+ if constexpr (type_subsumes_v<T, U>) {
+ // data_ holds a T or an Error. Subsumption proves the source
representation already
+ // satisfies that invariant, so the Any moves without inspecting its
state. Do not make
+ // this unconditional: value() reads back through
MoveFromAnyAfterCheck<T>, whose check is
+ // the success/error state, not the type.
+ data_ = std::move(other.data_);
+ } else {
+ data_ = other.is_err() ? Any(std::move(other).error()) :
Any(T(std::move(other).value()));
+ }
+ }
+
/*!
* \brief Implicit constructor from an error.
* \param error The error value.
@@ -199,6 +260,8 @@ class Expected {
}
private:
+ template <typename>
+ friend class Expected;
Expected() = default;
friend struct details::ExpectedUnsafe;
diff --git a/include/tvm/ffi/extra/structural_mutate.h
b/include/tvm/ffi/extra/structural_mutate.h
index 908bb5e8..c7916b79 100644
--- a/include/tvm/ffi/extra/structural_mutate.h
+++ b/include/tvm/ffi/extra/structural_mutate.h
@@ -812,17 +812,20 @@ struct StructuralMapCallbackChain {
/*!
* \brief Invoke a matched callback with optional def-region context.
- * \tparam Callback Callable whose result is convertible to
``Expected<Any>``.
+ * \tparam Callback Callable returning a value implicitly convertible to
``Expected<Any>``.
* \tparam Value Type of the converted value passed to the callback.
* \param callback The matched callback.
* \param value The converted value passed to the callback.
* \param kind The active def-region kind.
- * \return The callback result converted to ``Expected<Any>``.
+ * \return The callback result normalized to ``Expected<Any>``.
*/
template <typename Callback, typename Value>
TVM_FFI_INLINE static Expected<Any> InvokeCallbackLink(Callback& callback,
Value&& value,
TVMFFIDefRegionKind
kind) {
using FuncInfo = FunctionInfo<std::decay_t<Callback>>;
+ static_assert(std::is_convertible_v<typename FuncInfo::RetType,
Expected<Any>>,
+ "StructuralMap callbacks must return a replacement value,
Error, Unexpected, "
+ "or Expected<U> implicitly convertible to Expected<Any>");
if constexpr (FuncInfo::num_args == 1) {
return callback(std::forward<Value>(value));
} else {
@@ -843,8 +846,9 @@ struct StructuralMapCallbackChain {
* argument is borrowed and must not be retained after the callback returns.
*
* Each callback should follow map semantics: it must not mutate the input in
place and should
- * return ``Expected<Any>`` containing either the unchanged input or its
replacement. An ``Error``
- * stops the mapping. In pre-order, an unchanged input or uniquely owned
replacement may
+ * return a bare Any-convertible replacement or ``Expected<U>`` where ``U`` is
Any-convertible.
+ * A callback may instead return an error value or throw ``Error`` to stop the
mapping. In
+ * pre-order, an unchanged input or uniquely owned replacement may
* continue through ``MaybeInplaceMutate``; a shared replacement uses
``Mutate``. In post-order,
* the callback runs after the node's optional in-place mutation. In-place
mutation is available
* only through an explicit ``__s_maybe_inplace_mutate__`` hook.
@@ -877,10 +881,12 @@ struct StructuralMapCallbackChain {
* \tparam Callbacks Callback types whose first parameters select matching
values.
* \param root The borrowed root value to map.
* \param callbacks Callbacks tested in declaration order. Each accepts
``(value)`` or
- * ``(value, def_region_kind)`` and should return ``Expected<Any>``.
+ * ``(value, def_region_kind)`` and returns a bare Any-convertible
replacement,
+ * ``Expected<U>`` where ``U`` is Any-convertible, or an error value.
* \return The mapped owning value, or an Error if mapping or a callback fails.
*
- * \note Return type of each callback should be ``Expected<Any>``.
+ * \note Returning ``Expected<U>`` expresses errors as values; throwing
``Error`` is also
+ * supported and is converted to the error state.
*/
template <WalkOrder order, typename... Callbacks>
Expected<Any> StructuralMapExpected(AnyView root, Callbacks&&... callbacks)
noexcept {
@@ -902,11 +908,13 @@ Expected<Any> StructuralMapExpected(AnyView root,
Callbacks&&... callbacks) noex
* \tparam Callbacks Callback types whose first parameters select matching
values.
* \param root The borrowed root value to map.
* \param callbacks Callbacks tested in declaration order. Each accepts
``(value)`` or
- * ``(value, def_region_kind)`` and should return ``Expected<Any>``.
+ * ``(value, def_region_kind)`` and returns a bare Any-convertible
replacement,
+ * ``Expected<U>`` where ``U`` is Any-convertible, or an error value.
* \return The mapped owning value.
* \throws Error if mapping or a callback fails.
*
- * \note Return type of each callback should be ``Expected<Any>``.
+ * \note Returning ``Expected<U>`` expresses errors as values; throwing
``Error`` is also
+ * supported and is rethrown by this interface.
*/
template <WalkOrder order, typename... Callbacks>
Any StructuralMap(AnyView root, Callbacks&&... callbacks) {
diff --git a/tests/cpp/extra/test_structural_mutate.cc
b/tests/cpp/extra/test_structural_mutate.cc
index 2066d074..d2feb251 100644
--- a/tests/cpp/extra/test_structural_mutate.cc
+++ b/tests/cpp/extra/test_structural_mutate.cc
@@ -24,6 +24,7 @@
#include <cstdint>
#include <string>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -157,6 +158,63 @@ TEST(StructuralMap, PreOrderRecursivelyMapsCallbackResult)
{
EXPECT_EQ(mapped_value[0].cast<int64_t>(), 11);
}
+TEST(StructuralMap, AcceptsExpectedCallbackReturnTypes) {
+ static_assert(std::is_convertible_v<Expected<TVar>, Expected<Any>>,
+ "Expected<T> should implicitly convert to Expected<Any>");
+ static_assert(std::is_convertible_v<TVar, Expected<Any>>,
+ "Bare values convertible to Any should convert to
Expected<Any>");
+ TVar root("n");
+
+ TVar bare = StructuralMap<WalkOrder::kPostOrder>(root, [](const TVar&) ->
TVar {
+ return TVar("bare");
+ }).cast<TVar>();
+ EXPECT_EQ(bare->name, "bare");
+
+ // Any already converted directly to Expected<Any> before Expected<U>
support was added.
+ TVar any = StructuralMap<WalkOrder::kPostOrder>(root, [](const TVar&) -> Any
{
+ return Any(TVar("any"));
+ }).cast<TVar>();
+ EXPECT_EQ(any->name, "any");
+
+ TVar expected_object =
+ StructuralMap<WalkOrder::kPostOrder>(root, [](const TVar&) ->
Expected<TVar> {
+ return TVar("expected-object");
+ }).cast<TVar>();
+ EXPECT_EQ(expected_object->name, "expected-object");
+
+ TVar expected_any = StructuralMap<WalkOrder::kPostOrder>(root, [](const
TVar&) -> Expected<Any> {
+ return Any(TVar("expected-any"));
+ }).cast<TVar>();
+ EXPECT_EQ(expected_any->name, "expected-any");
+
+ auto check_error = [](auto callback, const char* expected_message) {
+ Expected<Any> result =
+ StructuralMapExpected<WalkOrder::kPostOrder>(TVar("n"),
std::move(callback));
+ ASSERT_TRUE(result.is_err());
+ EXPECT_EQ(result.error().kind(), "ValueError");
+ EXPECT_EQ(result.error().message(), expected_message);
+ };
+ check_error(
+ [](const TVar&) -> Expected<TVar> {
+ return Error("ValueError", "expected object error", "");
+ },
+ "expected object error");
+ check_error(
+ [](const TVar&) -> Expected<Any> { return Error("ValueError", "expected
any error", ""); },
+ "expected any error");
+ check_error([](const TVar&) -> Any { return Any(Error("ValueError", "any
error", "")); },
+ "any error");
+ check_error([](const TVar&) -> Error { return Error("ValueError", "direct
error", ""); },
+ "direct error");
+ check_error(
+ [](const TVar&) -> Unexpected<Error> {
+ return Unexpected(Error("ValueError", "unexpected error", ""));
+ },
+ "unexpected error");
+ check_error([](const TVar&) -> Expected<TVar> { throw Error("ValueError",
"thrown error", ""); },
+ "thrown error");
+}
+
template <WalkOrder order>
void CheckRepeatedVarRemap() {
TVar var("n");
diff --git a/tests/cpp/test_expected.cc b/tests/cpp/test_expected.cc
index d4d2ac26..3e365359 100644
--- a/tests/cpp/test_expected.cc
+++ b/tests/cpp/test_expected.cc
@@ -95,6 +95,35 @@ TEST(Expected, ObjectRefType) {
EXPECT_EQ(result.value()->value, 123);
}
+TEST(Expected, ImplicitConvertingConstructor) {
+ static_assert(std::is_convertible_v<String, Expected<Any>>);
+ static_assert(!std::is_convertible_v<String, Expected<int>>);
+ static_assert(std::is_assignable_v<Expected<Any>&, String>);
+ static_assert(std::is_convertible_v<Expected<int>, Expected<double>>);
+ static_assert(std::is_convertible_v<Expected<String>, Expected<Any>>);
+ static_assert(!std::is_convertible_v<Expected<void>, Expected<Any>>);
+
+ // Non-subsuming success types require value conversion.
+ Expected<double> success = Expected<int>(42);
+ ASSERT_TRUE(success.is_ok());
+ EXPECT_EQ(success.value(), 42.0);
+
+ Expected<double> failure = Expected<int>(Error("ValueError", "conversion
failed", ""));
+ ASSERT_TRUE(failure.is_err());
+ EXPECT_EQ(failure.error().kind(), "ValueError");
+ EXPECT_EQ(failure.error().message(), "conversion failed");
+
+ // Any subsumes String storage, so both success and error states move
without inspection.
+ Expected<Any> subsumed_success = Expected<String>(String("hello"));
+ ASSERT_TRUE(subsumed_success.is_ok());
+ EXPECT_EQ(subsumed_success.value().cast<String>(), "hello");
+
+ Expected<Any> subsumed_failure = Expected<String>(Error("TypeError",
"subsumed error", ""));
+ ASSERT_TRUE(subsumed_failure.is_err());
+ EXPECT_EQ(subsumed_failure.error().kind(), "TypeError");
+ EXPECT_EQ(subsumed_failure.error().message(), "subsumed error");
+}
+
// Test with String type
TEST(Expected, StringType) {
Expected<String> result = String("hello");
