gemini-code-assist[bot] commented on code in PR #224:
URL: https://github.com/apache/tvm-ffi/pull/224#discussion_r2491878103
##########
src/ffi/extra/env_context.cc:
##########
@@ -139,8 +139,10 @@ int TVMFFIEnvTensorAlloc(DLTensor* prototype,
TVMFFIObjectHandle* out) {
}
DLManagedTensorVersioned* dlpack_tensor = nullptr;
int ret = (*dlpack_alloc)(prototype, &dlpack_tensor, nullptr,
TVMFFIEnvTensorAllocSetError);
- TVM_FFI_ICHECK(dlpack_tensor != nullptr);
if (ret != 0) return ret;
+ // need to first check ret so we can properly propagate the error from
caller side.
+ TVM_FFI_CHECK(dlpack_tensor != nullptr, RuntimeError)
+ << "TVMFFIEnvTensorAlloc: failed to allocate a tensor from the
allocator";
Review Comment:
There's a small typo in the error message. It should be "failed to allocate
a tensor..." instead of "failed to a tensor...".
```suggestion
<< "TVMFFIEnvTensorAlloc: failed to allocate a tensor from the
allocator";
```
##########
tests/cpp/extra/test_c_env_api.cc:
##########
@@ -74,12 +74,14 @@ TEST(CEnvAPI, TVMFFIEnvTensorAlloc) {
TEST(CEnvAPI, TVMFFIEnvTensorAllocError) {
auto old_allocator = TVMFFIEnvGetDLPackManagedTensorAllocator();
TVMFFIEnvSetDLPackManagedTensorAllocator(TestDLPackManagedTensorAllocatorError,
0, nullptr);
- EXPECT_THROW(
- {
- Tensor::FromEnvAlloc(TVMFFIEnvTensorAlloc, {1, 2, 3},
DLDataType({kDLFloat, 32, 1}),
- DLDevice({kDLCPU, 0}));
- },
- tvm::ffi::Error);
+ try {
+ Tensor::FromEnvAlloc(TVMFFIEnvTensorAlloc, {1, 2, 3},
DLDataType({kDLFloat, 32, 1}),
+ DLDevice({kDLCPU, 0}));
Review Comment:
The test will silently pass if `Tensor::FromEnvAlloc` does not throw an
exception. It's better to explicitly fail the test in that case to make it more
robust.
```c
Tensor::FromEnvAlloc(TVMFFIEnvTensorAlloc, {1, 2, 3},
DLDataType({kDLFloat, 32, 1}),
DLDevice({kDLCPU, 0}));
FAIL() << "Expected tvm::ffi::Error to be thrown.";
```
--
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]
