[ https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16349623#comment-16349623 ]
Craig Ringer commented on THRIFT-4484: -------------------------------------- [~davejwatson] Hrm, too soon. {{&}} generates bogus code that tries to reference properties of {{__isset}} members as if they're the main member. I can't find it documented or find it in the tests either. I thought from reading the discussion on {{cpp.ref}} that it was renamed to {{&}} before commit, but maybe not. No idea what's going on now. > 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 > Priority: Minor > > 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. > {code} > struct RecSelf { > 1: i16 item > 2: optional RecSelf > } > {code} > 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: > {code} > 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} > code-generation produces > {code} > 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 */ > }; > {code} > Compilation fails with > {code} > 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 { > ^~~~~~~~~~ > {code} > 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 > {code} > class Downstream { > /* ... */ > std::shared_ptr<Downstream> downstream; > /* ... */ > }; > {code} > instead. -- This message was sent by Atlassian JIRA (v7.6.3#76005)