[ 
https://issues.apache.org/jira/browse/THRIFT-3595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16428283#comment-16428283
 ] 

ASF GitHub Bot commented on THRIFT-3595:
----------------------------------------

jeking3 closed pull request #831: THRIFT-3595 Perl: Unify implementation of set 
as a hashref
URL: https://github.com/apache/thrift/pull/831
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/compiler/cpp/src/generate/t_perl_generator.cc 
b/compiler/cpp/src/generate/t_perl_generator.cc
index 3f247e01d2..b0c0283320 100644
--- a/compiler/cpp/src/generate/t_perl_generator.cc
+++ b/compiler/cpp/src/generate/t_perl_generator.cc
@@ -382,22 +382,23 @@ string t_perl_generator::render_const_value(t_type* type, 
t_const_value* value)
     }
 
     out << "}";
-  } else if (type->is_list() || type->is_set()) {
-    t_type* etype;
-    if (type->is_list()) {
-      etype = ((t_list*)type)->get_elem_type();
-    } else {
-      etype = ((t_set*)type)->get_elem_type();
+  } else if (type->is_set()) {
+    t_type* etype = ((t_set*)type)->get_elem_type();
+    out << "{" << endl;
+    const vector<t_const_value*>& val = value->get_list();
+    vector<t_const_value*>::const_iterator v_iter;
+    for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
+      out << render_const_value(etype, *v_iter);
+      out << " => 1," << endl;
     }
+    out << "}";
+  } else if (type->is_list()) {
+    t_type* etype = ((t_list*)type)->get_elem_type();
     out << "[" << endl;
     const vector<t_const_value*>& val = value->get_list();
     vector<t_const_value*>::const_iterator v_iter;
     for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
-
       out << render_const_value(etype, *v_iter);
-      if (type->is_set()) {
-        out << " => 1";
-      }
       out << "," << endl;
     }
     out << "]";
@@ -1426,7 +1427,7 @@ void 
t_perl_generator::generate_serialize_container(ofstream& out, t_type* ttype
   } else if (ttype->is_set()) {
     indent(out) << "$xfer += $output->writeSetBegin("
                 << type_to_enum(((t_set*)ttype)->get_elem_type()) << ", "
-                << "scalar(@{$" << prefix << "}));" << endl;
+                << "scalar(keys %{$" << prefix << "}));" << endl;
 
   } else if (ttype->is_list()) {
 
@@ -1449,7 +1450,7 @@ void 
t_perl_generator::generate_serialize_container(ofstream& out, t_type* ttype
 
   } else if (ttype->is_set()) {
     string iter = tmp("iter");
-    indent(out) << "foreach my $" << iter << " (@{$" << prefix << "})" << endl;
+    indent(out) << "foreach my $" << iter << " (keys %{$" << prefix << "})" << 
endl;
     scope_up(out);
     generate_serialize_set_element(out, (t_set*)ttype, iter);
     scope_down(out);
diff --git a/test/perl/TestClient.pl b/test/perl/TestClient.pl
index 7ec32bf39e..03ad574490 100755
--- a/test/perl/TestClient.pl
+++ b/test/perl/TestClient.pl
@@ -250,16 +250,16 @@ sub usage {
 #
 # SET TEST
 #
-my $setout = [];
+my $setout = {};
 for (my $i = -2; $i < 3; ++$i) {
-    push(@$setout, $i);
+    $setout->{$i} = 1;
 }
 
-print("testSet({".join(",",@$setout)."})");
+print("testSet({".join(",",keys %$setout)."})");
 
 my $setin = $testClient->testSet($setout);
 
-print(" = {".join(",",@$setout)."}\n");
+print(" = {".join(",",keys %$setin)."}\n");
 
 #
 # LIST TEST
diff --git a/test/perl/TestServer.pl b/test/perl/TestServer.pl
index 4e0cc79bac..c3dcb18ef1 100644
--- a/test/perl/TestServer.pl
+++ b/test/perl/TestServer.pl
@@ -269,21 +269,8 @@ ()
 sub testSet() {
   my $self = shift;
   my $thing = shift;
-  my @arr;
-  my $result = \@arr;
-  print("testSet({");
-  my $first = 1;
-  foreach my $key (keys %$thing) {
-    if ($first) {
-        $first = 0;
-    } else {
-        print(", ");
-    }
-    print("$key");
-    push($result, $key);
-  }
-  print("})\n");
-  return $result;
+  printf "testSet({%s})\n", join(', ', keys %$thing);
+  return $thing;
 }
 
 sub testList() {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Perl Bindings: Set serialization/deserialization differs
> --------------------------------------------------------
>
>                 Key: THRIFT-3595
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3595
>             Project: Thrift
>          Issue Type: Bug
>          Components: Perl - Compiler
>    Affects Versions: 0.9.1, 0.9.2, 0.9.3
>            Reporter: Adam Millerchip
>            Assignee: James E. King, III
>            Priority: Major
>
> In the Perl bindings, a Thrift Set is dezerialized as a hashref, but the 
> serialization code expects an arrayref. This causes the code to die when 
> called if attempting to serialize a previously dezerialized Set.
> Additionally, it looks like there is a typo in the test that is testing this 
> feature:
> https://github.com/apache/thrift/blob/49f4dc0cd8c87213a0f80ae1daba2d094a358ea7/test/perl/TestClient.pl#L262
> If you change that {{@$setout}} to {{@$setin}}, the test fails.
> It doesn't make much sense to implement a Set in Perl as array, because 
> arrays allow duplicate entries and are ordered.
> I've written a change for the serialization that correctly expects a hashref: 
> https://github.com/apache/thrift/pull/831



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to