[jira] [Commented] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Dave Watson (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16347488#comment-16347488
 ] 

Dave Watson commented on THRIFT-4484:
-

IIRC cpp needs "cpp.ref = "true"" on the field to make it a pointer type.  Most 
other languages are pointers already.

> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context

[johnboiles]

Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
@jeking3 @dcelasun Travis is green so I think we're almost good to go here. 
Mind taking another look and merging if it looks good?


---

[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16347291#comment-16347291
 ] 

ASF GitHub Bot commented on THRIFT-4448:


Github user johnboiles commented on the issue:

https://github.com/apache/thrift/pull/1459
  
@jeking3 @dcelasun Travis is green so I think we're almost good to go here. 
Mind taking another look and merging if it looks good?


> Golang: do something with context.Context
> -
>
> Key: THRIFT-4448
> URL: https://issues.apache.org/jira/browse/THRIFT-4448
> Project: Thrift
>  Issue Type: Task
>  Components: Go - Library
>Affects Versions: 0.11.0
>Reporter: John Boiles
>Priority: Major
>
> PR Here: https://github.com/apache/thrift/pull/1459
> This patch wires through {{context.Context}} such that it can be used in in 
> {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests 
> to canceled or timed out via the context.
> This patch breaks support for go<1.7 so it's not ready to ship, but I'm 
> hoping to get some direction on this. When does Thrift expect to drop support 
> of go1.7? It looks like the current solution is to duplicate files that need 
> to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but 
> duplication seems unsustainable as the {{context}} package is imported more 
> places.
> Go 1.7 was released 15 August 2016. Given Golang has had significant 
> performance improvements in most dot releases, I suspect most production 
> services stay reasonably up to date. Here at Periscope/Twitter we're on 
> go1.9.1, and we're a fairly large organization.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Reopened] (THRIFT-4454) Large writes/reads may cause range check errors in debug mode

[Jens Geyer (JIRA)]

 [ 
https://issues.apache.org/jira/browse/THRIFT-4454?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jens Geyer reopened THRIFT-4454:


There are a few more places, similar code.

> Large writes/reads may cause range check errors in debug mode
> -
>
> Key: THRIFT-4454
> URL: https://issues.apache.org/jira/browse/THRIFT-4454
> Project: Thrift
>  Issue Type: Bug
>  Components: Delphi - Library
>Affects Versions: 0.11.0
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>Priority: Major
> Fix For: 0.12.0
>
>
> The pipes code contains a few casts using the {{PByteArray}} pointer type. 
> The underlying array {{TByteArray}} is defined as {code}
> type TByteArray = array [0..32767] of Byte;
> {code}
> With range checks enabled, any access to indices > 32767, even correct ones, 
> result in an range check exception. It may be worth noting that the code is 
> otherwise entirely correct (i.e. no buffer overruns). It is really only the 
> type cast that introduces the failing constraint. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Comment Edited] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Craig Ringer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346460#comment-16346460
 ] 

Craig Ringer edited comment on THRIFT-4484 at 1/31/18 11:25 AM:


3On 31 Jan. 2018 16:11, "Jens Geyer (JIRA)"  wrote:
{quote}
The more interesting question is whether it makes sense to do that.

Let's say you have an instance A that points to itself and then gets serialized.

Now you de-serialize the bits and get what ... an A that contains another copy 
of A = *two instances*{quote}

I can't say it makes sense to me how it's modelled right now in Jaeger and
- apparently -Zipkin.

But the non-c++ bindings (or at least Go and Java) deal with it, and it's
used in the wild.

So right now it works and is seemingly legal in the IDL but doesn't work in
the C++ generated bindings.

That's why I think it's a bug. Maybe it was a misfeature in the first
place. But shouldn't it either be disallowed in the IDL or supported by all
backends?



was (Author: ringerc):
On 31 Jan. 2018 16:11, "Jens Geyer (JIRA)"  wrote:

??
The more interesting question is whether it makes sense to do that.

Let's say you have an instance A that points to itself and then gets
serialized.
Now you de-serialize the bits and get what ... an A that contains another
copy of A = *two instances*
??

I can't say it makes sense to me how it's modelled right now in Jaeger and
- apparently -Zipkin.

But the non-c++ bindings (or at least Go and Java) deal with it, and it's
used in the wild.

So right now it works and is seemingly legal in the IDL but doesn't work in
the C++ generated bindings.

That's why I think it's a bug. Maybe it was a misfeature in the first
place. But shouldn't it either be disallowed in the IDL or supported by all
backends?


> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Comment Edited] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Craig Ringer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346460#comment-16346460
 ] 

Craig Ringer edited comment on THRIFT-4484 at 1/31/18 11:24 AM:


On 31 Jan. 2018 16:11, "Jens Geyer (JIRA)"  wrote:

??
The more interesting question is whether it makes sense to do that.

Let's say you have an instance A that points to itself and then gets
serialized.
Now you de-serialize the bits and get what ... an A that contains another
copy of A = *two instances*
??

I can't say it makes sense to me how it's modelled right now in Jaeger and
- apparently -Zipkin.

But the non-c++ bindings (or at least Go and Java) deal with it, and it's
used in the wild.

So right now it works and is seemingly legal in the IDL but doesn't work in
the C++ generated bindings.

That's why I think it's a bug. Maybe it was a misfeature in the first
place. But shouldn't it either be disallowed in the IDL or supported by all
backends?



was (Author: ringerc):
On 31 Jan. 2018 16:11, "Jens Geyer (JIRA)"  wrote:


[ https://issues.apache.org/jira/browse/THRIFT-4484?page=
com.atlassian.jira.plugin.system.issuetabpanels:comment-
tabpanel=16346405#comment-16346405 ]

Jens Geyer edited comment on THRIFT-4484 at 1/31/18 8:08 AM:
-

The more interesting question is whether it makes sense to do that.

Let's say you have an instance A that points to itself and then gets
serialized.
Now you de-serialize the bits and get what ... an A that contains another
copy of A = *two instances*


I can't say it makes sense to me how it's modelled right now in Jaeger and
- apparently -Zipkin.

But the non-c++ bindings (or at least Go and Java) deal with it, and it's
used in the wild.

So right now it works and is seemingly legal in the IDL but doesn't work in
the C++ generated bindings.

That's why I think it's a bug. Maybe it was a misfeature in the first
place. But shouldn't it either be disallowed in the IDL or supported by all
backends?


> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Craig Ringer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346460#comment-16346460
 ] 

Craig Ringer commented on THRIFT-4484:
--

On 31 Jan. 2018 16:11, "Jens Geyer (JIRA)"  wrote:


[ https://issues.apache.org/jira/browse/THRIFT-4484?page=
com.atlassian.jira.plugin.system.issuetabpanels:comment-
tabpanel=16346405#comment-16346405 ]

Jens Geyer edited comment on THRIFT-4484 at 1/31/18 8:08 AM:
-

The more interesting question is whether it makes sense to do that.

Let's say you have an instance A that points to itself and then gets
serialized.
Now you de-serialize the bits and get what ... an A that contains another
copy of A = *two instances*


I can't say it makes sense to me how it's modelled right now in Jaeger and
- apparently -Zipkin.

But the non-c++ bindings (or at least Go and Java) deal with it, and it's
used in the wild.

So right now it works and is seemingly legal in the IDL but doesn't work in
the C++ generated bindings.

That's why I think it's a bug. Maybe it was a misfeature in the first
place. But shouldn't it either be disallowed in the IDL or supported by all
backends?


> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Comment Edited] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Jens Geyer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346405#comment-16346405
 ] 

Jens Geyer edited comment on THRIFT-4484 at 1/31/18 8:08 AM:
-

The more interesting question is whether it makes sense to do that. 

Let's say you have an instance A that points to itself and then gets 
serialized. 
Now you de-serialize the bits and get what ... an A that contains another copy 
of A = *two instances*. 

And that is a well-known limitation of this feature.

I made another proposal a while ago that essentially introduces a new 
{{graph}} container type, THRIFT-2005. That can still be implemented, if 
anyone needs it. It does not interfere with the other feature, as it buildson 
top of {{list}}  but it very likely will need some overhaul to compile.

 


was (Author: jensg):
The more interesting question is whether it makes sense to do that. 

Let's say you have an instance A that points to itself and then gets 
serialized. 
Now you de-serialize the nbits and get what ... an A that contains another copy 
of A = *two instances*. 

And that is a well-known limitation of this feature.

I made another proposal a while ago that essentially introduces a new 
{{graph}} container type, THRIFT-2005. That can still be implemented, if 
anyone needs it. It does not interfere with the other feature, as it buildson 
top of {{list}}  but it very likely will need some overhaul to compile.

 

> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (THRIFT-4484) C++ codegen invalid for optional self-membership

[Jens Geyer (JIRA)]

[ 
https://issues.apache.org/jira/browse/THRIFT-4484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346405#comment-16346405
 ] 

Jens Geyer commented on THRIFT-4484:


The more interesting question is whether it makes sense to do that. 

Let's say you have an instance A that points to itself and then gets 
serialized. 
Now you de-serialize the nbits and get what ... an A that contains another copy 
of A = *two instances*. 

And that is a well-known limitation of this feature.

I made another proposal a while ago that essentially introduces a new 
{{graph}} container type, THRIFT-2005. That can still be implemented, if 
anyone needs it. It does not interfere with the other feature, as it buildson 
top of {{list}}  but it very likely will need some overhaul to compile.

 

> 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;
>   /* ... */
> };
> {code}
> instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)