[PATCH] D96629: [llvm][TextAPI] add equality operator for InterfaceFile

[Steven Wu via Phabricator via llvm-commits]

steven_wu added a comment.

Good in general. Suggestions in line.



================
Comment at: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h:441
+                                   KeyInfoT, BucketT> &RHS) {
+  if (LHS.size() != RHS.size()) {
+    return false;
----------------
you don't need `{}` for one line of body.


================
Comment at: llvm/lib/TextAPI/MachO/InterfaceFile.cpp:131
+bool InterfaceFile::operator==(const InterfaceFile &O) const {
+  if (Targets != O.Targets) {
+    return false;
----------------
Again, omit `{}`


================
Comment at: llvm/unittests/TextAPI/TextStubV3Tests.cpp:918
+  EXPECT_FALSE(FileA == FileB);
+  // Set CurrentVersion back to equal.
+  FileB.setCurrentVersion(PackedVersion(1, 2, 3));
----------------
Rather than spell out all the transform and reset back to equal, I will just create a wrapper that takes a function that transform one of the input, make sure they are different, transform the other, make sure they are equal now. It would be a lot cleaner and easy to read.

Same as below which you recreates the TBDFile from inputs again and again, I will create a wrapper to avoid duplicated code. Same as TBDv4 tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96629/new/

https://reviews.llvm.org/D96629