[pigeon] Optimize data class equality and hashing in Dart, Kotlin, java, and Swift, adds equality in other languages by tarrinneal · Pull Request #11140 · flutter/packages

tarrinneal · 2026-02-28T00:41:20Z

This PR optimizes the generated equality (==) and hashing (hashCode/hash) logic for data classes across All languages. These have been bothering me for a while, and I wanted to break out a new pr before the NI code is in review.

gemini-code-assist

Code Review

This pull request introduces significant performance optimizations for data class equality and hashing in Dart, Kotlin, and Swift by avoiding the creation of intermediate lists. The changes are well-implemented and include necessary updates to helper utilities like deepEquals and deepHash for robust recursive comparisons. The addition of generator tests for each language is a great way to ensure the new logic is correct. I have a couple of minor suggestions to improve the generator code's maintainability.

tarrinneal · 2026-03-04T06:06:18Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a significant and valuable improvement by optimizing and standardizing data class equality and hashing across all supported languages. The new pigeonDeepEquals and pigeonDeepHashCode (or equivalent) helper functions provide more robust and consistent behavior, especially for complex data types and collections. I've identified a few minor inconsistencies in the handling of signed zero for double values in the C++, GObject, and Swift generators. Addressing these will help fully achieve the goal of cross-platform consistency.

_{Note: Security Review did not run due to the size of the PR.}

packages/pigeon/lib/src/cpp/cpp_generator.dart

packages/pigeon/lib/src/gobject/gobject_generator.dart

packages/pigeon/lib/src/swift/swift_generator.dart

gemini-code-assist

Code Review

This pull request introduces significant improvements to the equality and hashing logic for data classes generated by Pigeon. It adds custom deepEquals and deepHash helper functions for various languages (Java, Kotlin, Swift, Objective-C, C++, Dart) to ensure consistent behavior across platforms, especially for complex types like lists, maps, and nested objects. The data class equals and hashCode (or equivalent) methods are updated to use these new helpers, making them more efficient and correct. Equality support is also added for languages that were missing it (C++, Objective-C). The changes are extensive, touching both the generated code in examples and the generator logic itself. Overall, this is a very valuable change for improving the robustness of data classes in Pigeon. I've found two minor inconsistencies in the hashing logic for maps in Dart and Swift compared to other platforms, which could affect cross-platform hash-based collection behavior.

packages/pigeon/lib/src/dart/dart_generator.dart

packages/pigeon/lib/src/swift/swift_generator.dart

tarrinneal · 2026-03-05T01:15:23Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces and optimizes data class equality (==) and hashing (hashCode/hash) across all supported languages, adding helper functions for deep equality and hashing of collections and nested objects to achieve more consistent and efficient implementations. However, a critical security vulnerability exists where the equality/hashing contract is violated for signed zero values (0.0 and -0.0) in the Objective-C and GObject generators; while equality checks consider them equal, their hashing logic produces different results, potentially leading to subtle bugs and logic bypasses in hash-based collections. Furthermore, other general issues have been found concerning the handling of special floating-point values, including NaN, in some of the new logic and tests.

packages/pigeon/lib/src/gobject/gobject_generator.dart

packages/pigeon/platform_tests/shared_test_plugin_code/test/equality_test.dart

packages/pigeon/platform_tests/test_plugin/example/ios/RunnerTests/AllDatatypesTests.swift

packages/pigeon/lib/src/objc/objc_generator.dart

packages/pigeon/lib/src/gobject/gobject_generator.dart

...lugin/android/src/test/java/com/example/alternate_language_test_plugin/AllDatatypesTest.java

…s all platforms

tarrinneal · 2026-03-05T09:11:33Z

@stuartmorgan-g this should be ready for review

gemini-code-assist

Code Review

This pull request introduces a significant and wide-ranging change to optimize and standardize equality and hashing for data classes across all supported languages. It adds new helper functions for deep equality and hashing, which is a great step towards cross-platform consistency. However, I've found a critical logic bug in the new map equality implementation across several language generators (Dart, Kotlin, Swift, Objective-C, C++) that could lead to incorrect results. Additionally, the C++ map hashing implementation is inconsistent with other platforms, which undermines the goal of uniform hashing. These issues need to be addressed before merging.

_{Note: Security Review did not run due to the size of the PR.}

gemini-code-assist · 2026-03-05T09:16:08Z

packages/pigeon/lib/src/cpp/cpp_generator.dart

+  size_t result = 0;
+  for (const auto& kv : v) {
+    result = result * 31 + PigeonInternalDeepHash(kv.first);
+    result = result * 31 + PigeonInternalDeepHash(kv.second);
+  }
+  return result;


The hashing implementation for std::map is inconsistent with the other languages. This implementation uses an order-dependent algorithm (result = result * 31 + ...), while other platforms (Dart, Java, Kotlin, Swift, GObject/C) use an order-independent XOR sum of entry hashes (hash(key) ^ hash(value)).

While std::map is ordered, this inconsistency will lead to different hash codes for the same map data across platforms, which defeats a primary goal of this PR. To ensure consistent hashing, please use the XOR sum approach.

Suggested change

size_t result = 0;

for (const auto& kv : v) {

result = result * 31 + PigeonInternalDeepHash(kv.first);

result = result * 31 + PigeonInternalDeepHash(kv.second);

}

return result;

size_t result = 0;

for (const auto& kv : v) {

result += PigeonInternalDeepHash(kv.first) ^ PigeonInternalDeepHash(kv.second);

}

return result;

I wasn't expecting the hashes from the platforms to match each other, just an equivalent class on the same platform

gemini-code-assist · 2026-03-05T09:16:08Z

packages/pigeon/lib/src/dart/dart_generator.dart

+        if (_deepEquals(entryA.key, entryB.key)) {
+          if (_deepEquals(entryA.value, entryB.value)) {
+            found = true;
+            break;
+          } else {
+            return false;
+          }
+        }


There's a logic error in the _deepEquals implementation for maps. If a key matches but the value doesn't, it immediately returns false. This is incorrect if another entry in the second map has an equivalent key and a matching value (e.g., if keys are objects that are equal by value but not identical). This could lead to incorrectly reporting two equivalent maps as unequal.

This same issue exists in the Kotlin, Swift, Objective-C, and C++ generators. The else { return false; } block should be removed to allow the loop to continue searching for another matching key-value pair, similar to the implementation in the Java and GObject generators.

Suggested change

if (_deepEquals(entryA.key, entryB.key)) {

if (_deepEquals(entryA.value, entryB.value)) {

found = true;

break;

} else {

return false;

}

}

if (_deepEquals(entryA.key, entryB.key)) {

if (_deepEquals(entryA.value, entryB.value)) {

found = true;

break;

}

}

There shouldn't be matching keys with different values in a map in any of these languages, especially since we're forcing the hashes to match based on contents rather than identity.

packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java

stuartmorgan-g · 2026-03-05T16:33:12Z

packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java

+      return pigeonDeepEquals(name, that.name)
+          && pigeonDeepEquals(description, that.description)
+          && pigeonDeepEquals(code, that.code)
+          && pigeonDeepEquals(data, that.data);


Capturing for posterity, this has the same inefficiency as the previous iteration, where we are feeding known-type values through a big type-inspection chain instead of dispatching to specific-type equality checkers.

Not something we need to address now since we still mostly expect these to be used in tests where efficiency doesn't matter, but something to keep in mind later if it performance ever comes up.

This doesn't actually, any non-collections well return first before any type checking. Even collections are only checking for collection types.

You're probably thinking about the codec, which I intend to change so known types bypass the type checking soon(tm)

This doesn't actually, any non-collections well return first before any type checking.

I'm not following. The first line in this statement is calling pigeonDeepEquals with two strings. We know at compile time they are strings. But we are calling pigeonDeepEquals for them which will do runtime checks for whether they are:

byte arrays

int arrays

long arrays

double arrays

Lists

Maps

Doubles

Floats

and only then, after all of that, check that the strings are equal.

packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java

packages/pigeon/lib/src/cpp/cpp_generator.dart

packages/pigeon/lib/src/gobject/gobject_generator.dart

packages/pigeon/platform_tests/test_plugin/windows/CMakeLists.txt

packages/pigeon/test/cpp_generator_test.dart

tarrinneal · 2026-03-06T05:40:16Z

@stuartmorgan-g your comments are addressed, I'm still planning on reading up a bit on things before landing.

stuartmorgan-g

Minor notes (mostly orthodontia), but otherwise LGTM.

stuartmorgan-g · 2026-03-09T17:00:19Z

packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java

+      return pigeonDeepEquals(name, that.name)
+          && pigeonDeepEquals(description, that.description)
+          && pigeonDeepEquals(code, that.code)
+          && pigeonDeepEquals(data, that.data);


This doesn't actually, any non-collections well return first before any type checking.

I'm not following. The first line in this statement is calling pigeonDeepEquals with two strings. We know at compile time they are strings. But we are calling pigeonDeepEquals for them which will do runtime checks for whether they are:

byte arrays

int arrays

long arrays

double arrays

Lists

Maps

Doubles

Floats

and only then, after all of that, check that the strings are equal.

packages/pigeon/platform_tests/test_plugin/windows/CMakeLists.txt

stuartmorgan-g · 2026-03-09T17:11:53Z

packages/pigeon/example/app/android/app/src/main/java/io/flutter/plugins/Messages.java

+    if (a instanceof double[] && b instanceof double[]) {
+      double[] da = (double[]) a;
+      double[] db = (double[]) b;
+      if (da.length != db.length) return false;


Needs braces.

stuartmorgan-g · 2026-03-09T17:14:27Z

packages/pigeon/example/app/linux/messages.g.cc

+
+#include <cmath>
+static guint G_GNUC_UNUSED flpigeon_hash_double(double v) {
+  if (std::isnan(v)) return (guint)0x7FF80000;


Needs braces

stuartmorgan-g · 2026-03-09T17:14:36Z

packages/pigeon/example/app/linux/messages.g.cc


+#include <string.h>
+
+#include <cmath>


Nit: There should be a blank line after this.

stuartmorgan-g · 2026-03-09T17:20:10Z

packages/pigeon/example/app/linux/messages.g.cc

+      return result;
+    }
+    default:
+      return (guint)fl_value_get_type(value);


static_cast

stuartmorgan-g · 2026-03-09T17:27:53Z

packages/pigeon/example/app/windows/runner/messages.g.cpp

+bool PigeonInternalDeepEquals(const std::unique_ptr<T>& a,
+                              const std::unique_ptr<T>& b);
+
+inline bool PigeonInternalDeepEquals(const ::flutter::EncodableValue& a,


Why is this one inlined? That's a bunch of code to inline.

stuartmorgan-g · 2026-03-09T17:29:18Z

packages/pigeon/example/app/windows/runner/messages.g.cpp

+  }
+  if (!a || !b) {
+    return false;
+  }


You might also want to check for pointer equality (a.get() == b.get()) before checking deep equality, since if the pointers match you don't need to check the contents.

stuartmorgan-g · 2026-03-09T18:03:59Z

packages/pigeon/example/app/windows/runner/messages.g.cpp

+template <typename T>
+size_t PigeonInternalDeepHash(const std::unique_ptr<T>& v);
+
+inline size_t PigeonInternalDeepHash(const ::flutter::EncodableValue& v);


Same here; inlining this seems questionable.

stuartmorgan-g · 2026-03-09T18:05:03Z

packages/pigeon/example/app/windows/runner/messages.g.cpp

+  size_t result = 0;
+  for (const auto& kv : v) {
+    result = result * 31 + PigeonInternalDeepHash(kv.first);
+    result = result * 31 + PigeonInternalDeepHash(kv.second);


Why doesn't the concern about map key order from the other comment thread apply for C++?

Improve efficiency in some methods

82065a8

tarrinneal requested a review from stuartmorgan-g February 28, 2026 00:41

github-actions bot added p: pigeon platform-android platform-ios platform-macos labels Feb 28, 2026

gemini-code-assist bot reviewed Feb 28, 2026

View reviewed changes

tarrinneal added 7 commits February 27, 2026 18:22

analyze this

9861c00

format

11daedd

less repeating, and less listing

e197fc0

allow individual class types to not have colliding hashes

cbf4096

improve deep equals for non-collections

935aa47

allowing for NaN or other cases where identical is better

4e9541f

more tests, add other languages, unify behavior across platforms

f30b44c

github-actions bot added the platform-windows label Mar 3, 2026

first pass at gobject

08a79f3

github-actions bot added the platform-linux label Mar 3, 2026

tarrinneal added 7 commits March 3, 2026 12:09

Merge branch 'main' of https://github.com/flutter/packages into effic

8cb51ac

analyze

ddc41c2

delete random extra files

4ad3636

fix kotlin generator change that broke unit test

0a08ef5

fix broken gobject and cpp code

5855b23

more bugs on languages I can't compile locally

f99e64a

last few issues I hope

101fb0f

tarrinneal changed the title ~~[pigeon] Optimize data class equality and hashing in Dart, Kotlin, and Swift~~ [pigeon] Optimize data class equality and hashing in Dart, Kotlin, and Swift, adds equality in other languages Mar 4, 2026

tarrinneal added 4 commits March 3, 2026 20:47

not being able to run things locally sucks

7309d17

bigobj

24d0f23

consolidate tests

21ad393

revert project files

94a58c8

gemini-code-assist bot reviewed Mar 4, 2026

View reviewed changes

packages/pigeon/lib/src/dart/dart_generator.dart Show resolved Hide resolved

packages/pigeon/lib/src/swift/swift_generator.dart Show resolved Hide resolved

tarrinneal added 3 commits March 4, 2026 11:10

More consistency across languages

3fdd987

revert -0.0 changes

b920f50

gen example

7df481e

gemini-code-assist bot reviewed Mar 5, 2026

View reviewed changes

tarrinneal added 4 commits March 4, 2026 17:33

Merge branch 'main' of https://github.com/flutter/packages into effic

25021e7

memecmp

4cbfdcd

re-normalize -0.0 in hash methods to create consistent behavior acros…

8c75a36

…s all platforms

finish tests

655229c

tarrinneal marked this pull request as draft March 5, 2026 02:54

tarrinneal added 5 commits March 4, 2026 19:02

lints and reverting of accidentally pushed files

80d8bbc

hopeful test fix

a57dd6f

delete file that shouldn't ever have existed

4ded545

more tests and windows method rework

8698cb5

bug fixes and more robust map==

7355a96

tarrinneal changed the title ~~[pigeon] Optimize data class equality and hashing in Dart, Kotlin, and Swift, adds equality in other languages~~ [pigeon] Optimize data class equality and hashing in Dart, Kotlin, java, and Swift, adds equality in other languages Mar 5, 2026

tarrinneal added 2 commits March 4, 2026 23:15

fix linix

45d8207

one last linux fix

97b8e8c

tarrinneal marked this pull request as ready for review March 5, 2026 09:11

gemini-code-assist bot reviewed Mar 5, 2026

View reviewed changes

stuartmorgan-g requested changes Mar 5, 2026

View reviewed changes

nits and nats

38cffb2

stuartmorgan-g reviewed Mar 9, 2026

View reviewed changes


		#include <string.h>

		#include <cmath>

Conversation

tarrinneal commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

tarrinneal commented Mar 4, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

tarrinneal commented Mar 5, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarrinneal commented Mar 5, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarrinneal commented Mar 6, 2026

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tarrinneal commented Feb 28, 2026 •

edited

Loading