Skip to content

Commit

Permalink
Fix by-value self-inclusion in C++
Browse files Browse the repository at this point in the history
This IDL is fine for languages where everything is by-reference. But for C++, it tries to embed a class by-value into its self, which cannot succeed, and results in compilation failure. See jaegertracing#35

This has been worked around by patching the IDL-generated code; see jaegertracing/jaeger-client-cpp#45 .

It turns out we can just tell Thrift to include the optional member by-reference. See the comment in https://issues.apache.org/jira/browse/THRIFT-4484  about `cpp.ref`. Apparently that's now spelled `&` in Thrift IDL. I don't know what effect it has for other languages, the documentation is sparse at best, and doesn't mention this at all.

If this doesn't break other languages I think it's the correct fix for the IDL.
  • Loading branch information
ringerc authored Feb 1, 2018
1 parent 6710b74 commit 9b7da60
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion thrift/crossdock/tracetest.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct Downstream {
3: required string host
4: required string port
5: required Transport transport
6: optional Downstream downstream
6: optional Downstream &downstream
}

struct StartTraceRequest {
Expand Down

0 comments on commit 9b7da60

Please sign in to comment.