You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/04/06 13:12:00 UTC
[jira] [Commented] (THRIFT-3783) python code generator dose not
handle struct dependent
[ https://issues.apache.org/jira/browse/THRIFT-3783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16428286#comment-16428286 ]
ASF GitHub Bot commented on THRIFT-3783:
----------------------------------------
jeking3 closed pull request #982: THRIFT-3783: python code generator dose not handle struct dependent
URL: https://github.com/apache/thrift/pull/982
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_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index 1ee0fcb627..7f846ffef1 100644
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -21,12 +21,15 @@
#include <fstream>
#include <iostream>
#include <vector>
+#include <list>
+#include <map>
#include <stdlib.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sstream>
#include <algorithm>
+
#include "t_generator.h"
#include "platform.h"
#include "version.h"
@@ -40,6 +43,58 @@ using std::vector;
static const string endl = "\n"; // avoid ostream << std::endl flushes
+// calculate object dependent graph
+class Graph {
+private:
+ std::vector<int> degree;
+ std::vector<std::vector<int> > edge;
+ std::vector<int> topo;
+ int n;
+ bool circle;
+public:
+ Graph(int n) : n(n) {
+ degree.resize(n);
+ edge.resize(n);
+ topo.resize(n);
+ circle = false;
+ }
+
+ void add_edge(int u, int v) {
+ if (u < 0 || u >= n || v < 0 || v >= n) {
+ throw "node index out of range";
+ }
+ edge[u].push_back(v);
+ degree[v]++;
+ }
+
+ void topological_sort() {
+ int l = 0, r = 0;
+ for (int i = 0; i < n; i++) {
+ if (degree[i] == 0) {
+ topo[r++] = i;
+ }
+ }
+ while (l != r) {
+ int u = topo[l++];
+ for (size_t j = 0; j < edge[u].size(); j++) {
+ int v = edge[u][j];
+ if (--degree[v] == 0) {
+ topo[r++] = v;
+ }
+ }
+ }
+ circle = r != n;
+ }
+
+ bool has_circle() {
+ return circle;
+ }
+
+ std::vector<int> get_order() {
+ return topo;
+ }
+};
+
/**
* Python code generator.
*
@@ -141,6 +196,7 @@ class t_py_generator : public t_generator {
void init_generator();
void close_generator();
+ void construct_object_dependent_graph();
/**
* Program-level generation functions
@@ -266,8 +322,11 @@ class t_py_generator : public t_generator {
}
private:
+ map<t_struct*, int> object_ids_;
+ map<int, std::pair<t_struct*, bool> > pending_objects_;
+ std::list<int> object_order_;
- /**
+ /**
* True if we should generate new-style classes.
*/
bool gen_newstyle_;
@@ -384,6 +443,52 @@ void t_py_generator::init_generator() {
py_autogen_comment() << endl <<
py_imports() << endl <<
"from .ttypes import *" << endl;
+
+ construct_object_dependent_graph();
+}
+
+/**
+ * Construct object dependent graph to determine object generate order
+ * if cycle dependent is detected, generate process will be aborted
+ * object dependent example
+ * struct A {
+ * 1: B b;
+ * }
+ * struct B {
+ * }
+ *
+ * then B should be generated before A in ttypes.py, otherwise runtime error exception will be thrown
+ * in generated python code
+ */
+void t_py_generator::construct_object_dependent_graph() {
+ const std::vector<t_struct*>& objects = program_->get_objects();
+ for (size_t i = 0; i < objects.size(); ++i) {
+ object_ids_[objects[i]] = i;
+ }
+
+ Graph dependent_graph(objects.size());
+ for (size_t i = 0; i < objects.size(); i++) {
+ const vector<t_field*>& members = objects[i]->get_members();
+ for(vector<t_field*>::const_iterator m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+ t_type* type = get_true_type((*m_iter)->get_type());
+ if (!type->is_struct() && !type->is_xception()) {
+ continue;
+ }
+ map<t_struct*, int>::const_iterator it = object_ids_.find((t_struct*)type);
+ if (it != object_ids_.end()) {
+ dependent_graph.add_edge(it->second, i);
+ }
+ }
+ }
+
+ dependent_graph.topological_sort();
+ // check cycle dependent
+ if (dependent_graph.has_circle()) {
+ throw "type error: cycel dependent found in " + program_->get_path();
+ }
+
+ // calculate struct generate order
+ object_order_ = std::list<int>(dependent_graph.get_order().begin(), dependent_graph.get_order().end());
}
/**
@@ -631,7 +736,25 @@ void t_py_generator::generate_xception(t_struct* txception) {
* Generates a python struct
*/
void t_py_generator::generate_py_struct(t_struct* tstruct, bool is_exception) {
- generate_py_struct_definition(f_types_, tstruct, is_exception);
+ map<t_struct*, int>::const_iterator it = object_ids_.find(tstruct);
+ if (it == object_ids_.end()) {
+ // should never happen
+ std::cout << "struct " << tstruct->get_name() << " not found in object_ids dict" << std::endl;
+ return;
+ }
+ pending_objects_[it->second] = std::pair<t_struct*, bool>(tstruct, is_exception);
+ for (;;) {
+ if (object_order_.empty()){
+ break;
+ }
+ map<int, std::pair<t_struct*, bool> >::iterator it = pending_objects_.find(object_order_.front());
+ if (it == pending_objects_.end()) {
+ break;
+ }
+ generate_py_struct_definition(f_types_, it->second.first, it->second.second);
+ object_order_.pop_front();
+ pending_objects_.erase(it);
+ }
}
/**
diff --git a/test/py/object_gen_order/complicated.thrift b/test/py/object_gen_order/complicated.thrift
new file mode 100644
index 0000000000..cfbb117a3f
--- /dev/null
+++ b/test/py/object_gen_order/complicated.thrift
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+namespace py complicated
+struct A {
+ 1: B b;
+}
+struct AA {
+ 1: B b;
+ 2: A a;
+}
+struct B {
+ 1: i32 b;
+}
diff --git a/test/py/object_gen_order/cycle.thrift b/test/py/object_gen_order/cycle.thrift
new file mode 100644
index 0000000000..7c29e44c24
--- /dev/null
+++ b/test/py/object_gen_order/cycle.thrift
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+namespace py cycle
+
+struct A {
+ 1: B b;
+}
+
+struct B {
+ 1: A a;
+}
+
diff --git a/test/py/object_gen_order/runtest.sh b/test/py/object_gen_order/runtest.sh
new file mode 100755
index 0000000000..810ab14d09
--- /dev/null
+++ b/test/py/object_gen_order/runtest.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+rm -rf gen-py
+../../../compiler/cpp/thrift --gen py simple.thrift || exit 1
+../../../compiler/cpp/thrift --gen py complicated.thrift || exit 1
+../../../compiler/cpp/thrift --gen py cycle.thrift && exit 1 # should fail
+PYTHONPATH=./gen-py python -c 'from simple.ttypes import A, B' || exit 1
+PYTHONPATH=./gen-py python -c 'from complicated.ttypes import A, B' || exit 1
+echo 'All tests pass!'
diff --git a/test/py/object_gen_order/simple.thrift b/test/py/object_gen_order/simple.thrift
new file mode 100644
index 0000000000..c40eb69b0a
--- /dev/null
+++ b/test/py/object_gen_order/simple.thrift
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+namespace py simple
+struct A {
+1: B b;
+}
+
+struct B {
+ 1: i32 b;
+}
----------------------------------------------------------------
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:
users@infra.apache.org
> python code generator dose not handle struct dependent
> ------------------------------------------------------
>
> Key: THRIFT-3783
> URL: https://issues.apache.org/jira/browse/THRIFT-3783
> Project: Thrift
> Issue Type: Bug
> Components: Python - Compiler
> Reporter: Huabin
> Priority: Minor
>
> given thrift idl
> {code}
> struct A {
> 1: B b,
> }
> struct B {
> 1: i32 b,
> }
> {code}
> generated ttypes.py
> {code}
> 20 class A:
> 21 """
> 22 Attributes:
> 23 - b
> 24 """
> 25
> 26 thrift_spec = (
> 27 None, # 0
> 28 (1, TType.STRUCT, 'b', (B, B.thrift_spec), None, ), # 1
> 29 )
> 30
> {code}
> import A will cause error since it referenced B, which has not be defined.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)