DarkSharpness commented on PR #229:
URL: https://github.com/apache/tvm-ffi/pull/229#issuecomment-3495639713
@tqchen I think this support can remain optional. I agree that for
FFI-exported functions, the only ownership marker that truly makes sense is `T`
— even `const T&` doesn’t, since we cannot hold a direct reference (like Any&)
to a foreign object. In fact, any type passed through FFI must first be
converted to a value.
To clarify, the process is:
1. Convert the input to a value.
2. Pass that value to the inner function according to the function signature.
For `const T&` or `T&&`, C++ automatically extends the lifetime of the value
until the end of the function call. For `T&`, we instead create temporary
objects.
The purpose of this PR is mainly to improve compatibility and ease of use
with native C++ interfaces, which aligns with the goal of PR #228. We complete
the support for all possible signatures `T`, `const T`, `T &`, `const T &`,
`T&&`, `const T&&`. If this behavior feels misleading, we can simply disable
the nonsensical type markers (like T&) and instead provide a clear,
user-friendly error message.
Currently, for `T&` arguments, we get the following error:
```
In file included from
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/function.h:30,
from
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/dtype.h:29,
from
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/container/tensor.h:29,
from
/root/.cache/tvm-ffi/test_sbind_760f1ab214615fe2/main.cpp:1:
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/function_details.h: In
instantiation of ‘void tvm::ffi::details::unpack_call(std::index_sequence<Is
...>, const std::string*, const F&, const tvm::ffi::AnyView*, int32_t,
tvm::ffi::Any*) [with R = int; long unsigned int ...Is = {0}; F = int(int&);
std::index_sequence<Is ...> = std::integer_sequence<long unsigned int, 0>;
std::string = std::__cxx11::basic_string<char>; int32_t = int]’:
/root/.cache/tvm-ffi/test_sbind_760f1ab214615fe2/main.cpp:17:1: required
from here
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/function_details.h:207:15:
error: cannot bind non-const lvalue reference of type ‘int&’ to an rvalue of
type ‘int’
207 | *rv = R(f(ArgValueWithContext(args, Is, optional_name,
f_sig)...));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/tvm-ffi/python/tvm_ffi/../../include/tvm/ffi/function_details.h:155:18:
note: after user-defined conversion:
‘tvm::ffi::details::ArgValueWithContext::operator Type() [with Type = int]’
155 | TVM_FFI_INLINE operator Type() { //
NOLINT(google-explicit-constructor)
| ^~~~~~~~
ninja: build stopped: subcommand failed.
```
Another minor point of PR is that, for types with template constructors,
current `ArgValue` will fail. We need to change non-template ArgValue +
template operator into template ArgValue + non-template operator. [Online
example](https://godbolt.org/z/18z5MbzTY). A real-world example is
`std::optional` (yes, we met this problem in PR #228 ).
--
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]
