Craig Ringer created THRIFT-4484:
------------------------------------
Summary: C++ codegen invalid for optional self-membership
Key: THRIFT-4484
URL: https://issues.apache.org/jira/browse/THRIFT-4484
Project: Thrift
Issue Type: Bug
Components: C++ - Compiler
Affects Versions: 0.11.0
Environment: Thrift 0.10.0 tested, but I don't see a change in 0.11.0.
Fedora 25. gcc 6.4.1. x86_64.
Reporter: Craig Ringer
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.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
