You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by collinmsn <gi...@git.apache.org> on 2016/04/07 13:35:51 UTC
[GitHub] thrift pull request: THRIFT-3783: python code generator dose not h...
GitHub user collinmsn opened a pull request:
https://github.com/apache/thrift/pull/982
THRIFT-3783: python code generator dose not handle struct dependent
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/collinmsn/thrift master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/982.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #982
----
commit d5de50526d476f234a452f4563531055ce943076
Author: zhenghuabin <zh...@bytedance.com>
Date: 2016-04-07T10:58:01Z
gen py: generate struct types by dependent order
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #982: THRIFT-3783: python code generator dose not handle struct...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/982
Does this need further work or should it be closed?
---
[GitHub] thrift pull request: THRIFT-3783: python code generator dose not h...
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/982#discussion_r58929004
--- Diff: compiler/cpp/src/generate/t_py_generator.cc ---
@@ -384,6 +406,55 @@ 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
--- End diff --
But your test case is perfectly valid Thrift IDL:
```
struct A {
1: B b;
}
struct B {
1: A a;
}
```
One can do this with Thrift. Is there no way to support that in Python? Something like forward-declaration of types maybe?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-3783: python code generator dose not h...
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/982#discussion_r58928719
--- Diff: compiler/cpp/src/generate/t_py_generator.cc ---
@@ -21,12 +21,19 @@
#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 <boost/graph/adjacency_list.hpp>
+#include <boost/graph/topological_sort.hpp>
+#include <boost/graph/depth_first_search.hpp>
+#include <boost/graph/visitors.hpp>
--- End diff --
We don't want to add boost as a dependency when building the Thrift compiler. Sorry.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-3783: python code generator dose not h...
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/982#discussion_r58929379
--- Diff: compiler/cpp/src/generate/t_py_generator.cc ---
@@ -384,6 +406,55 @@ 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;
+ }
+
+ std::vector<DependentGraphEdge> edges;
+ 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()) {
+ edges.push_back(DependentGraphEdge(it->second, i));
+ }
+ }
+ }
+
+ DependentGraph dependent_graph(edges.begin(), edges.end(), objects.size());
+ // check cycle dependent
+ bool has_cycle = false;
+ cycle_detector detector(has_cycle);
+ boost::depth_first_search(dependent_graph, boost::visitor(detector));
+ if (has_cycle) {
+ throw "type error: cycel dependent found in " + program_->get_path();
--- End diff --
Would be nice to know which type(s) are affected.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-3783: python code generator dose not h...
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/982#discussion_r58929507
--- Diff: 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;
+}
--- End diff --
As I said, that's legal code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---