gemini-code-assist[bot] commented on code in PR #393:
URL: https://github.com/apache/tvm-ffi/pull/393#discussion_r2676189912
##########
tests/cpp/test_reflection.cc:
##########
@@ -294,4 +294,31 @@ TEST(Reflection, AccessPath) {
auto root_parent = root->GetParent();
EXPECT_FALSE(root_parent.has_value());
}
+
+struct TestObjWithAny : public Object {
+ Any value;
+ TestObjWithAny(Any value) : value(std::move(value)) {}
+ static constexpr bool _type_mutable = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_FINAL("test.TestObjWithAny", TestObjWithAny,
Object);
+};
+
+TVM_FFI_STATIC_INIT_BLOCK() {
+ namespace refl = tvm::ffi::reflection;
+ refl::ObjectDef<TestObjWithAny>().def(refl::init<Any>()).def_ro("value",
&TestObjWithAny::value);
+}
+
+TEST(Reflection, InitWithAny) {
+ Function init = reflection::GetMethod("test.TestObjWithAny", "__ffi_init__");
+ Any obj1 = init(42);
+ EXPECT_TRUE(obj1.as<TestObjWithAny>() != nullptr);
+ EXPECT_EQ(obj1.as<TestObjWithAny>()->value.cast<int>(), 42);
+
+ Any obj2 = init(3.14);
+ EXPECT_TRUE(obj2.as<TestObjWithAny>() != nullptr);
Review Comment:
Similar to the previous check, using `ASSERT_TRUE` is safer here to prevent
a null pointer dereference if the cast fails.
```suggestion
ASSERT_TRUE(obj2.as<TestObjWithAny>() != nullptr);
```
##########
tests/cpp/test_reflection.cc:
##########
@@ -294,4 +294,31 @@ TEST(Reflection, AccessPath) {
auto root_parent = root->GetParent();
EXPECT_FALSE(root_parent.has_value());
}
+
+struct TestObjWithAny : public Object {
+ Any value;
+ TestObjWithAny(Any value) : value(std::move(value)) {}
+ static constexpr bool _type_mutable = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_FINAL("test.TestObjWithAny", TestObjWithAny,
Object);
+};
+
+TVM_FFI_STATIC_INIT_BLOCK() {
+ namespace refl = tvm::ffi::reflection;
+ refl::ObjectDef<TestObjWithAny>().def(refl::init<Any>()).def_ro("value",
&TestObjWithAny::value);
+}
+
+TEST(Reflection, InitWithAny) {
+ Function init = reflection::GetMethod("test.TestObjWithAny", "__ffi_init__");
+ Any obj1 = init(42);
+ EXPECT_TRUE(obj1.as<TestObjWithAny>() != nullptr);
+ EXPECT_EQ(obj1.as<TestObjWithAny>()->value.cast<int>(), 42);
+
+ Any obj2 = init(3.14);
+ EXPECT_TRUE(obj2.as<TestObjWithAny>() != nullptr);
+ EXPECT_EQ(obj2.as<TestObjWithAny>()->value.cast<double>(), 3.14);
+
+ Any obj3 = init(String("hello"));
+ EXPECT_TRUE(obj3.as<TestObjWithAny>() != nullptr);
Review Comment:
Using `ASSERT_TRUE` here as well will make the test more robust by
preventing execution from continuing with a null pointer if the cast fails.
```suggestion
ASSERT_TRUE(obj3.as<TestObjWithAny>() != nullptr);
```
##########
tests/cpp/test_reflection.cc:
##########
@@ -294,4 +294,31 @@ TEST(Reflection, AccessPath) {
auto root_parent = root->GetParent();
EXPECT_FALSE(root_parent.has_value());
}
+
+struct TestObjWithAny : public Object {
+ Any value;
+ TestObjWithAny(Any value) : value(std::move(value)) {}
+ static constexpr bool _type_mutable = true;
+ TVM_FFI_DECLARE_OBJECT_INFO_FINAL("test.TestObjWithAny", TestObjWithAny,
Object);
+};
+
+TVM_FFI_STATIC_INIT_BLOCK() {
+ namespace refl = tvm::ffi::reflection;
+ refl::ObjectDef<TestObjWithAny>().def(refl::init<Any>()).def_ro("value",
&TestObjWithAny::value);
+}
+
+TEST(Reflection, InitWithAny) {
+ Function init = reflection::GetMethod("test.TestObjWithAny", "__ffi_init__");
+ Any obj1 = init(42);
+ EXPECT_TRUE(obj1.as<TestObjWithAny>() != nullptr);
Review Comment:
Using `ASSERT_TRUE` instead of `EXPECT_TRUE` is more robust for this check.
If the cast fails and returns a null pointer, `ASSERT_TRUE` will halt the test
immediately, preventing a potential crash from dereferencing a null pointer on
the next line.
As a minor improvement, you could also store the result of
`obj1.as<TestObjWithAny>()` in a variable to avoid calling it twice.
```suggestion
ASSERT_TRUE(obj1.as<TestObjWithAny>() != nullptr);
```
--
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]
