gemini-code-assist[bot] commented on code in PR #597:
URL: https://github.com/apache/tvm-ffi/pull/597#discussion_r3305070907


##########
tests/cpp/extra/test_structural_equal_hash.cc:
##########
@@ -269,4 +270,29 @@ TEST(StructuralEqualHash, ArraySelfInsertProducesSnapshot) 
{
   EXPECT_EQ(StructuralHash()(arr), StructuralHash()(arr));
 }
 
+// Test the FFI global "ffi.StructuralEqual" — the cheap boolean path that
+// avoids constructing AccessPath mismatch pairs (unlike GetFirstMismatch).
+TEST(StructuralEqualHash, FFIGlobalStructuralEqual) {
+  Function ffi_equal = Function::GetGlobalRequired("ffi.StructuralEqual");
+  Function ffi_mismatch = 
Function::GetGlobalRequired("ffi.GetFirstStructuralMismatch");
+
+  // Equal arrays: both the FFI global and GetFirstMismatch must agree.
+  Array<int> a = {1, 2, 3};
+  Array<int> b = {1, 2, 3};
+  EXPECT_TRUE(ffi_equal(a, b, false, false).cast<bool>());
+  EXPECT_FALSE(ffi_mismatch(a, b, false, 
false).cast<Optional<reflection::AccessPathPair>>());
+
+  // Unequal arrays: FFI global returns false; GetFirstMismatch returns a 
value.
+  Array<int> c = {1, 2, 4};
+  EXPECT_FALSE(ffi_equal(a, c, false, false).cast<bool>());
+  EXPECT_TRUE(ffi_mismatch(a, c, false, 
false).cast<Optional<reflection::AccessPathPair>>());
+
+  // Consistency: bool result from FFI global matches (mismatch is None) for 
equal objects.
+  Array<int> d = {10, 20};
+  Array<int> e = {10, 20};
+  bool equal_result = ffi_equal(d, e, false, false).cast<bool>();
+  bool no_mismatch = !ffi_mismatch(d, e, false, 
false).cast<Optional<reflection::AccessPathPair>>();

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   For consistency with the rest of the test file (which uses `.has_value()` in 
all other cases), and to ensure correct boolean evaluation of the `Optional` 
wrapper, please explicitly call `.has_value()` on the returned 
`Optional<reflection::AccessPathPair>` objects.
   
   ```suggestion
     EXPECT_FALSE(ffi_mismatch(a, b, false, 
false).cast<Optional<reflection::AccessPathPair>>().has_value());
   
     // Unequal arrays: FFI global returns false; GetFirstMismatch returns a 
value.
     Array<int> c = {1, 2, 4};
     EXPECT_FALSE(ffi_equal(a, c, false, false).cast<bool>());
     EXPECT_TRUE(ffi_mismatch(a, c, false, 
false).cast<Optional<reflection::AccessPathPair>>().has_value());
   
     // Consistency: bool result from FFI global matches (mismatch is None) for 
equal objects.
     Array<int> d = {10, 20};
     Array<int> e = {10, 20};
     bool equal_result = ffi_equal(d, e, false, false).cast<bool>();
     bool no_mismatch = !ffi_mismatch(d, e, false, 
false).cast<Optional<reflection::AccessPathPair>>().has_value();
   ```



-- 
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]

Reply via email to