gemini-code-assist[bot] commented on code in PR #376:
URL: https://github.com/apache/tvm-ffi/pull/376#discussion_r2656978758
##########
tests/cpp/test_array.cc:
##########
@@ -292,4 +292,29 @@ TEST(Array, Upcast) {
static_assert(details::type_contains_v<Any, Array<float>>);
}
+TEST(Array, NegativeIndexThrows) {
+ Array<int> arr = {1, 2, 3};
+ EXPECT_THROW(
+ {
+ try {
+ [[maybe_unused]] int val = arr[-1];
+ } catch (const Error& error) {
+ EXPECT_EQ(error.kind(), "IndexError");
+ throw;
+ }
+ },
+ ::tvm::ffi::Error);
+
+ EXPECT_THROW(
+ {
+ try {
+ arr.Set(-1, 42);
+ } catch (const Error& error) {
+ EXPECT_EQ(error.kind(), "IndexError");
+ throw;
+ }
+ },
+ ::tvm::ffi::Error);
+}
Review Comment:
The current test case for negative indices exercises `Array<T>::operator[]`
and `Array<T>::Set`. These methods have their own bounds checking, which
already included negative index validation. Therefore, this test passes even
without the fixes in `ArrayObj` and doesn't verify the changes made in this PR.
To ensure the fix is correctly tested, the test should directly call the
modified methods on `ArrayObj`. The suggested change below targets
`ArrayObj::at` (which calls `operator[]`) and `ArrayObj::SetItem` to properly
validate the new bounds checks.
```c
TEST(Array, NegativeIndexThrows) {
Array<int> arr = {1, 2, 3};
// Directly test ArrayObj methods, which are the ones modified in this PR.
// The Array<T> wrapper methods already had negative index checks.
ArrayObj* arr_obj = arr.GetArrayObj();
// Test ArrayObj::at (which calls operator[])
EXPECT_THROW(
{
try {
[[maybe_unused]] const auto& val = arr_obj->at(-1);
} catch (const Error& error) {
EXPECT_EQ(error.kind(), "IndexError");
throw;
}
},
::tvm::ffi::Error);
// Test ArrayObj::SetItem
EXPECT_THROW(
{
try {
arr_obj->SetItem(-1, Any(42));
} catch (const Error& error) {
EXPECT_EQ(error.kind(), "IndexError");
throw;
}
},
::tvm::ffi::Error);
}
```
--
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]
