fishy commented on a change in pull request #2307:
URL: https://github.com/apache/thrift/pull/2307#discussion_r568820431



##########
File path: compiler/cpp/src/thrift/generate/t_go_generator.cc
##########
@@ -3463,6 +3543,109 @@ void 
t_go_generator::generate_serialize_list_element(ostream& out, t_list* tlist
   generate_serialize_field(out, &efield, prefix);
 }
 
+/**
+ * Compares any type
+ */
+void t_go_generator::generate_go_equals(ostream& out, t_type* ori_type, string 
tgt, string src) {
+
+  t_type* ttype = get_true_type(ori_type);
+  // Do nothing for void types
+  if (ttype->is_void()) {
+    throw "compiler error: cannot generate equals for void type: " + tgt;
+  }
+
+  if (ttype->is_struct() || ttype->is_xception()) {
+    generate_go_equals_struct(out, ttype, tgt, src);
+  } else if (ttype->is_container()) {
+    generate_go_equals_container(out, ttype, tgt, src);
+  } else if (ttype->is_base_type() || ttype->is_enum()) {
+    out << indent() << "if ";
+    if (ttype->is_base_type()) {
+      t_base_type::t_base tbase = ((t_base_type*)ttype)->get_base();
+      switch (tbase) {
+      case t_base_type::TYPE_VOID:
+        throw "compiler error: cannot equals void: " + tgt;
+        break;
+
+      case t_base_type::TYPE_STRING:
+        if (ttype->is_binary()) {
+          out << "bytes.Compare(" << tgt << ", " << src << ") != 0";
+        } else {
+          out << tgt << " != " << src;
+        }
+        break;
+
+      case t_base_type::TYPE_BOOL:
+      case t_base_type::TYPE_I8:
+      case t_base_type::TYPE_I16:
+      case t_base_type::TYPE_I32:
+      case t_base_type::TYPE_I64:
+      case t_base_type::TYPE_DOUBLE:
+        out << tgt << " != " << src;
+        break;
+
+      default:
+        throw "compiler error: no Go name for base type " + 
t_base_type::t_base_name(tbase);
+      }
+    } else if (ttype->is_enum()) {
+      out << tgt << " != " << src;
+    }
+
+    out << " { return false }" << endl;
+  } else {
+    throw "compiler error: Invalid type in generate_go_equals '" + 
ttype->get_name() + "' for '"
+        + tgt + "'";
+  }
+}
+
+/**
+ * Compares the members of a struct
+ */
+void t_go_generator::generate_go_equals_struct(ostream& out,
+                                               t_type* ttype,
+                                               string tgt,
+                                               string src) {
+  (void)ttype;

Review comment:
       what does this line do?

##########
File path: compiler/cpp/src/thrift/generate/t_go_generator.cc
##########
@@ -3463,6 +3543,109 @@ void 
t_go_generator::generate_serialize_list_element(ostream& out, t_list* tlist
   generate_serialize_field(out, &efield, prefix);
 }
 
+/**
+ * Compares any type
+ */
+void t_go_generator::generate_go_equals(ostream& out, t_type* ori_type, string 
tgt, string src) {
+
+  t_type* ttype = get_true_type(ori_type);
+  // Do nothing for void types
+  if (ttype->is_void()) {
+    throw "compiler error: cannot generate equals for void type: " + tgt;
+  }
+
+  if (ttype->is_struct() || ttype->is_xception()) {
+    generate_go_equals_struct(out, ttype, tgt, src);
+  } else if (ttype->is_container()) {
+    generate_go_equals_container(out, ttype, tgt, src);
+  } else if (ttype->is_base_type() || ttype->is_enum()) {
+    out << indent() << "if ";
+    if (ttype->is_base_type()) {
+      t_base_type::t_base tbase = ((t_base_type*)ttype)->get_base();
+      switch (tbase) {
+      case t_base_type::TYPE_VOID:
+        throw "compiler error: cannot equals void: " + tgt;
+        break;
+
+      case t_base_type::TYPE_STRING:
+        if (ttype->is_binary()) {
+          out << "bytes.Compare(" << tgt << ", " << src << ") != 0";
+        } else {
+          out << tgt << " != " << src;
+        }
+        break;
+
+      case t_base_type::TYPE_BOOL:
+      case t_base_type::TYPE_I8:
+      case t_base_type::TYPE_I16:
+      case t_base_type::TYPE_I32:
+      case t_base_type::TYPE_I64:
+      case t_base_type::TYPE_DOUBLE:
+        out << tgt << " != " << src;
+        break;
+
+      default:
+        throw "compiler error: no Go name for base type " + 
t_base_type::t_base_name(tbase);
+      }
+    } else if (ttype->is_enum()) {
+      out << tgt << " != " << src;
+    }
+
+    out << " { return false }" << endl;
+  } else {
+    throw "compiler error: Invalid type in generate_go_equals '" + 
ttype->get_name() + "' for '"
+        + tgt + "'";
+  }
+}
+
+/**
+ * Compares the members of a struct
+ */
+void t_go_generator::generate_go_equals_struct(ostream& out,
+                                               t_type* ttype,
+                                               string tgt,
+                                               string src) {
+  (void)ttype;
+  out << indent() << "if !" << tgt << "." << equals_method_name_ << "(" << src
+      << ") { return false }" << endl;
+}
+
+/**
+ * Compares any container type
+ */
+void t_go_generator::generate_go_equals_container(ostream& out,
+                                                  t_type* ttype,
+                                                  string tgt,
+                                                  string src) {
+  out << indent() << "if len(" << tgt << ") != len(" << src << ") { return 
false }" << endl;
+  if (ttype->is_map()) {
+    t_map* tmap = (t_map*)ttype;
+    out << indent() << "for k, _tgt := range " << tgt << " {" << endl;
+    indent_up();
+    string element_source = tmp("_src");
+    out << indent() << element_source << " := " << src << "[k]" << endl;
+    generate_go_equals(out, tmap->get_val_type(), "_tgt", element_source);
+    indent_down();
+    indent(out) << "}" << endl;
+  } else if (ttype->is_list() || ttype->is_set()) {

Review comment:
       should this just be `} else {`, or maybe add another `} else { throw 
"unsupported container" }` at the end? currently if this is a container but 
it's not a map/list/set (I'm not sure if we have other container types) this 
function only checks for length and then does nothing.

##########
File path: lib/go/test/tests/equals_test.go
##########
@@ -0,0 +1,219 @@
+package tests
+
+import (
+       "equalstest"
+       "strconv"
+       "testing"
+)
+
+func TestEquals(t *testing.T) {
+       // test basic field
+       basicTgt, basicSrc := genBasicFoo(), genBasicFoo()
+       if !basicTgt.Equals(basicSrc) {
+               t.Error("BasicEqualsFoo.Equals() test failed")
+       }
+       basicSrc.EnumFoo = equalstest.EnumFoo_e2
+       if basicTgt.Equals(basicSrc) {
+               t.Error("BasicEqualsFoo.Equals() test failed")
+       }
+       basicSrc = genBasicFoo()
+       basicSrc.OptBoolFoo = nil
+       if basicTgt.Equals(basicSrc) || basicSrc.Equals(basicTgt) {
+               t.Error("BasicEqualsFoo.Equals() test failed")
+       }
+       if !(&equalstest.BasicEqualsFoo{}).Equals(&equalstest.BasicEqualsFoo{}) 
{
+               t.Error("BasicEqualsFoo.Equals() test failed")
+       }
+       // test struct field
+       structTgt, structSrc := genStructFoo(), genStructFoo()
+       if !structTgt.Equals(structSrc) {
+               t.Error("StructEqualsFoo.Equals() test failed")
+       }
+       structSrc.OptStructFoo.EnumFoo = equalstest.EnumFoo_e2
+       if structTgt.Equals(structSrc) {
+               t.Error("StructEqualsFoo.Equals() test failed")
+       }
+       structSrc = genStructFoo()
+       structSrc.OptStructFoo = nil
+       if structTgt.Equals(structSrc) || structSrc.Equals(structTgt) {
+               t.Error("StructEqualsFoo.Equals() test failed")
+       }
+       if 
!(&equalstest.StructEqualsFoo{}).Equals(&equalstest.StructEqualsFoo{}) {
+               t.Error("StructEqualsFoo.Equals() test failed")
+       }
+       // test list field
+       listTgt, listSrc := genListFoo(), genListFoo()
+       if !listTgt.Equals(listSrc) {
+               t.Error("ListEqualsFoo.Equals() test failed")
+       }
+       listSrc.OptI64StringMapListFoo[0][1] = "0"
+       if listTgt.Equals(listSrc) {
+               t.Error("ListEqualsFoo.Equals() test failed")
+       }
+       listSrc = genListFoo()
+       listSrc.OptI64StringMapListFoo = nil
+       if listTgt.Equals(listSrc) || listSrc.Equals(listTgt) {
+               t.Error("ListEqualsFoo.Equals() test failed")
+       }
+       if !(&equalstest.ListEqualsFoo{}).Equals(&equalstest.ListEqualsFoo{}) {
+               t.Error("ListEqualsFoo.Equals() test failed")
+       }
+       // test set field
+       setTgt, setSrc := genSetFoo(), genSetFoo()
+       if !setTgt.Equals(setSrc) {
+               t.Error("SetEqualsFoo.Equals() test failed")
+       }
+       setSrc.OptI64StringMapSetFoo[0][1] = "0"
+       if setTgt.Equals(setSrc) {
+               t.Error("SetEqualsFoo.Equals() test failed")
+       }
+       setSrc = genSetFoo()
+       setSrc.OptI64StringMapSetFoo = nil
+       if setTgt.Equals(setSrc) || setSrc.Equals(setTgt) {
+               t.Error("SetEqualsFoo.Equals() test failed")
+       }
+       if !(&equalstest.SetEqualsFoo{}).Equals(&equalstest.SetEqualsFoo{}) {
+               t.Error("SetEqualsFoo.Equals() test failed")
+       }
+       // test map field
+       mapTgt, mapSrc := genMapFoo(), genMapFoo()
+       if !mapTgt.Equals(mapSrc) {
+               t.Error("MapEqualsFoo.Equals() test failed")
+       }
+       mapSrc.OptI64I64StringMapMapFoo[1][1] = "0"
+       if mapTgt.Equals(mapSrc) {
+               t.Error("MapEqualsFoo.Equals() test failed")
+       }
+       mapSrc = genMapFoo()
+       mapSrc.OptI64I64StringMapMapFoo = nil
+       if mapTgt.Equals(mapSrc) || mapSrc.Equals(mapTgt) {
+               t.Error("MapEqualsFoo.Equals() test failed")
+       }
+       if !(&equalstest.MapEqualsFoo{}).Equals(&equalstest.MapEqualsFoo{}) {
+               t.Error("MapEqualsFoo.Equals() test failed")
+       }
+}
+
+func genBasicFoo() *equalstest.BasicEqualsFoo {
+       return &equalstest.BasicEqualsFoo{
+               BoolFoo:      true,
+               OptBoolFoo:   &(&struct{ x bool }{true}).x,
+               I8Foo:        1,
+               OptI8Foo:     &(&struct{ x int8 }{1}).x,
+               I16Foo:       2,
+               OptI16Foo:    &(&struct{ x int16 }{2}).x,
+               I32Foo:       3,
+               OptI32Foo:    &(&struct{ x int32 }{3}).x,
+               I64Foo:       4,
+               OptI64Foo:    &(&struct{ x int64 }{4}).x,
+               DoubleFoo:    5,
+               OptDoubleFoo: &(&struct{ x float64 }{5}).x,
+               StrFoo:       "6",
+               OptStrFoo:    &(&struct{ x string }{"6"}).x,
+               BinFoo:       []byte("7"),
+               OptBinFoo:    []byte("7"),
+               EnumFoo:      equalstest.EnumFoo_e1,
+               OptEnumFoo:   &(&struct{ x equalstest.EnumFoo 
}{equalstest.EnumFoo_e1}).x,
+       }

Review comment:
       the newly added typedef'd fields are not added here (and other `gen*` 
functions)

##########
File path: test/go/src/bin/testequals/main.go
##########
@@ -0,0 +1,179 @@
+package main

Review comment:
       hmm why do we need to add this directory/file?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to