-
Notifications
You must be signed in to change notification settings - Fork 50
feat: add array __contains__ support #377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| #include <gtest/gtest.h> | ||
| #include <tvm/ffi/container/array.h> | ||
| #include <tvm/ffi/function.h> | ||
| #include <tvm/ffi/string.h> | ||
|
|
||
| #include "./testing_object.h" | ||
|
|
||
|
|
@@ -292,4 +293,51 @@ TEST(Array, Upcast) { | |
| static_assert(details::type_contains_v<Any, Array<float>>); | ||
| } | ||
|
|
||
| TEST(Array, Contains) { | ||
| Array<int> arr = {1, 2, 3, 4, 5}; | ||
| AnyEqual eq; | ||
|
|
||
| // Test element is present | ||
| bool found = false; | ||
| for (const auto& elem : *arr.GetArrayObj()) { | ||
| if (eq(elem, Any(3))) { | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| EXPECT_TRUE(found); | ||
|
|
||
| // Test element is not present | ||
| found = false; | ||
| for (const auto& elem : *arr.GetArrayObj()) { | ||
| if (eq(elem, Any(10))) { | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| EXPECT_FALSE(found); | ||
|
|
||
| // Test empty array | ||
| Array<int> empty_arr; | ||
| found = false; | ||
| for (const auto& elem : *empty_arr.GetArrayObj()) { | ||
| if (eq(elem, Any(1))) { | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| EXPECT_FALSE(found); | ||
|
|
||
| // Test with strings | ||
| Array<String> str_arr = {String("hello"), String("world")}; | ||
| found = false; | ||
| for (const auto& elem : *str_arr.GetArrayObj()) { | ||
| if (eq(elem, Any(String("world")))) { | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| EXPECT_TRUE(found); | ||
| } | ||
|
Comment on lines
296
to
313
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To make this a more effective integration test, you should fetch the registered Additionally, the test can be made more concise and readable by removing the repetitive TEST(Array, Contains) {
Function f = Function::Get("ffi.ArrayContains");
ASSERT_TRUE(f.defined());
Array<int> arr = {1, 2, 3, 4, 5};
EXPECT_TRUE(f(arr, 3));
EXPECT_TRUE(f(arr, 1));
EXPECT_TRUE(f(arr, 5));
EXPECT_FALSE(f(arr, 10));
EXPECT_FALSE(f(arr, 0));
Array<int> empty_arr;
EXPECT_FALSE(f(empty_arr, 1));
Array<String> str_arr = {String("hello"), String("world")};
EXPECT_TRUE(f(str_arr, String("hello")));
EXPECT_TRUE(f(str_arr, String("world")));
EXPECT_FALSE(f(str_arr, String("foo")));
} |
||
|
|
||
| } // namespace | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of
ffi.ArrayContainsis correct, but it can be made more concise and idiomatic by usingstd::any_offrom the<algorithm>header. This improves readability and aligns with modern C++ practices. The<algorithm>header is already available through existing includes.