Details
-
Bug
-
Status: Open
-
Minor
-
Resolution: Unresolved
-
0.11.0
-
None
-
None
-
Thrift 0.10.0 tested, but I don't see a change in 0.11.0. Fedora 25. gcc 6.4.1. x86_64.
Description
Support was added for self-referential objects in in https://github.com/apache/thrift/pull/84 "Tree/Recursive struct support in thrift".
The tests cover objects that are co-recursive, objects that have lists of themselves, etc. But there's no test for optional self-containment e.g.
struct RecSelf {
1: i16 item
2: optional RecSelf
}
This works fine for languages like Java and Go. But in C++ it generates nonsensical code that cannot compile because it contains a by-value member of its self and a separate isset member.
For example, from opentracing jaeger's IDL:
struct Downstream {
1: required string serviceName
2: required string serverRole
3: required string host
4: required string port
5: required Transport transport
6: optional Downstream downstream
}
code-generation produces
class Downstream { public: /* blah elided blah */ virtual ~Downstream() throw(); std::string serviceName; std::string serverRole; std::string host; std::string port; Transport::type transport; Downstream downstream; _Downstream__isset __isset; /* blah elided blah */ };
Compilation fails with
tracetest_types.h:64:14: error: field ‘downstream’ has incomplete type ‘jaegertracing::crossdock::thrift::Downstream’
Downstream downstream;
^~~~~~~~~~
tracetest_types.h:47:7: note: definition of ‘class jaegertracing::crossdock::thrift::Downstream’ is not complete until the closing brace
class Downstream {
^~~~~~~~~~
I'd argue that the __isset model is not ideal, and a std::expected-like "optional" or "maybe" construct would be a lot better. But presumably there are historical reasons for that.
The simplest correct solution would be to generate
class Downstream { /* ... */ std::shared_ptr<Downstream> downstream; /* ... */ };
instead.
