Copilot commented on code in PR #32:
URL: https://github.com/apache/tvm-ffi/pull/32#discussion_r2364632372
##########
src/ffi/extra/testing.cc:
##########
@@ -87,6 +87,41 @@ class TestObjectDerived : public TestObjectBase {
TVM_FFI_DECLARE_OBJECT_INFO_FINAL("testing.TestObjectDerived",
TestObjectDerived, TestObjectBase);
};
+class TestCxxClassBase : public Object {
+ public:
+ int64_t v_i64;
+ int32_t v_i32;
+
+ TestCxxClassBase(int64_t v_i64, int32_t v_i32) : v_i64(v_i64), v_i32(v_i32)
{}
Review Comment:
The constructor parameter names shadow the member variables. Consider using
different parameter names (e.g., `_v_i64`, `_v_i32`) to avoid confusion and
improve code clarity.
```suggestion
TestCxxClassBase(int64_t _v_i64, int32_t _v_i32) : v_i64(_v_i64),
v_i32(_v_i32) {}
```
##########
src/ffi/extra/testing.cc:
##########
@@ -87,6 +87,41 @@ class TestObjectDerived : public TestObjectBase {
TVM_FFI_DECLARE_OBJECT_INFO_FINAL("testing.TestObjectDerived",
TestObjectDerived, TestObjectBase);
};
+class TestCxxClassBase : public Object {
+ public:
+ int64_t v_i64;
+ int32_t v_i32;
+
+ TestCxxClassBase(int64_t v_i64, int32_t v_i32) : v_i64(v_i64), v_i32(v_i32)
{}
+
+ static constexpr bool _type_mutable = true;
+ TVM_FFI_DECLARE_OBJECT_INFO("testing.TestCxxClassBase", TestCxxClassBase,
Object);
+};
+
+class TestCxxClassDerived : public TestCxxClassBase {
+ public:
+ double v_f64;
+ float v_f32;
+
+ TestCxxClassDerived(int64_t v_i64, int32_t v_i32, double v_f64, float v_f32)
+ : TestCxxClassBase(v_i64, v_i32), v_f64(v_f64), v_f32(v_f32) {}
Review Comment:
The constructor parameter names shadow the member variables. Consider using
different parameter names to avoid confusion and improve code clarity.
```suggestion
TestCxxClassDerived(int64_t i64_param, int32_t i32_param, double
f64_param, float f32_param)
: TestCxxClassBase(i64_param, i32_param), v_f64(f64_param),
v_f32(f32_param) {}
```
##########
src/ffi/extra/testing.cc:
##########
@@ -87,6 +87,41 @@ class TestObjectDerived : public TestObjectBase {
TVM_FFI_DECLARE_OBJECT_INFO_FINAL("testing.TestObjectDerived",
TestObjectDerived, TestObjectBase);
};
+class TestCxxClassBase : public Object {
+ public:
+ int64_t v_i64;
+ int32_t v_i32;
+
+ TestCxxClassBase(int64_t v_i64, int32_t v_i32) : v_i64(v_i64), v_i32(v_i32)
{}
+
+ static constexpr bool _type_mutable = true;
+ TVM_FFI_DECLARE_OBJECT_INFO("testing.TestCxxClassBase", TestCxxClassBase,
Object);
+};
+
+class TestCxxClassDerived : public TestCxxClassBase {
+ public:
+ double v_f64;
+ float v_f32;
+
+ TestCxxClassDerived(int64_t v_i64, int32_t v_i32, double v_f64, float v_f32)
+ : TestCxxClassBase(v_i64, v_i32), v_f64(v_f64), v_f32(v_f32) {}
+
+ TVM_FFI_DECLARE_OBJECT_INFO("testing.TestCxxClassDerived",
TestCxxClassDerived, TestCxxClassBase);
+};
+
+class TestCxxClassDerivedDerived : public TestCxxClassDerived {
+ public:
+ String v_str;
+ bool v_bool;
+
+ TestCxxClassDerivedDerived(int64_t v_i64, int32_t v_i32, double v_f64, float
v_f32, String v_str,
+ bool v_bool)
+ : TestCxxClassDerived(v_i64, v_i32, v_f64, v_f32), v_str(v_str),
v_bool(v_bool) {}
Review Comment:
The constructor parameter names shadow the member variables. Consider using
different parameter names to avoid confusion and improve code clarity.
```suggestion
TestCxxClassDerivedDerived(int64_t v_i64, int32_t v_i32, double v_f64,
float v_f32, String param_str,
bool param_bool)
: TestCxxClassDerived(v_i64, v_i32, v_f64, v_f32), v_str(param_str),
v_bool(param_bool) {}
```
--
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]
