thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Craig Ringer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (THRIFT-2471) Make cpp.ref annotation language agnostic
Date Fri, 02 Feb 2018 01:49:00 GMT

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

Craig Ringer commented on THRIFT-2471:
--------------------------------------

I think a mistake may have been made in this commit, because {{cpp.ref}} still exists in the
tree but only apparently for the Go compiler:

{code}
CHANGES:    * [THRIFT-2471] - Make cpp.ref annotation language agnostic
compiler/cpp/src/thrift/generate/t_go_generator.cc:  if (tfield->annotations_.count("cpp.ref")
!= 0) {
lib/go/test/RefAnnotationFieldsTest.thrift: 1: optional string s = "DEFAULT" (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 2: optional i64 i = 42 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 3: optional bool b = false (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 4: optional string s2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 5: optional i64 i2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 6: optional bool b2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 7: optional structA aa (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 9: optional list<i64> l (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 10: optional list<i64> l2 = [1, 2] (cpp.ref
= ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 11: optional map<i64, i64> m (cpp.ref =
""),
lib/go/test/RefAnnotationFieldsTest.thrift: 12: optional map<i64, i64> m2 = {1:2, 3:4}
(cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 13: optional binary bin (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 14: optional binary bin2 = "asdf" (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 15: required string ref_s = "DEFAULT" (cpp.ref
= ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 16: required i64 ref_i = 42 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 17: required bool ref_b = false (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 18: required string ref_s2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 19: required i64 ref_i2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 20: required bool ref_b2 (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 21: required structA ref_aa (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 22: required list<i64> ref_l (cpp.ref =
""),
lib/go/test/RefAnnotationFieldsTest.thrift: 23: required list<i64> ref_l2 = [1, 2] (cpp.ref
= ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 24: required map<i64, i64> ref_m (cpp.ref
= ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 25: required map<i64, i64> ref_m2 = {1:2,
3:4} (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 26: required binary ref_bin (cpp.ref = ""),
lib/go/test/RefAnnotationFieldsTest.thrift: 27: required binary ref_bin2 = "asdf" (cpp.ref
= ""),
{code}

and a {{git grep '\<ref\>'}} doesn't reveal anything that looks relevant; in particular
nothing at all in the c++ codegen.

Looks to me like support in Go was missed when converting this to the {{&}} form.

There's also another oversight where it produces bad code for {{optional}} members, but I'll
raise a new bug for that.

> Make cpp.ref annotation language agnostic
> -----------------------------------------
>
>                 Key: THRIFT-2471
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2471
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (General)
>            Reporter: Jens Geyer
>            Assignee: Jens Geyer
>            Priority: Major
>             Fix For: 0.9.2
>
>         Attachments: boost_1.40.0__to__1.54.0-patch
>
>
> The proposal is to make the new {{cpp.ref}} annotation introduced with THRIFT-2421 language
agnostic instead of a C++ specialty only. 
> The annotation changes inlined nested structs into pointers to structs. This behaviour
is potentially relevant with all languages using value semantics for nested structs etc.



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

Mime
View raw message