[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16090591#comment-16090591 ] Jens Geyer commented on THRIFT-4253: Confirmed, trying this ends up in a "{{cannot use "test" (type string) as type *string in field value}}". That seems absolutely wrong, and besides fixing it we also should integrate an test into [ConstantsDemo.thrift|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=test/ConstantsDemo.thrift;hb=HEAD] > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16090895#comment-16090895 ] Davin Chia commented on THRIFT-4253: Thanks for the quick reply [~jensg]. This is the first time I am contributing to a large C++ open source project, or writing much C++ to be honest, and it will be good to get some guidance on development workflow. Immediate questions are (Again sorry if they sound newbie, because I am): Where can I check the code out? What is the typical build for C++ projects? I also have a question about testing, but I now know where to do that (ConstantsDemo.thrift). If you spare a few minutes, it will also be nice to have a point in the code I can start from. Thanks. > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16091234#comment-16091234 ] Jens Geyer commented on THRIFT-4253: The Go generator is [this|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/thrift/generate/t_go_generator.cc;hb=HEAD]. I'd probably start looking into [{{generate_const}} or {{render_const_value}}|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/thrift/generate/t_go_generator.cc;hb=HEAD#l114]. > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16095309#comment-16095309 ] ASF GitHub Bot commented on THRIFT-4253: GitHub user davinchia opened a pull request: https://github.com/apache/thrift/pull/1311 Fix for constant assignments to optional fields in Go. As per ticket https://issues.apache.org/jira/browse/THRIFT-4253. As I dug through the code I realised the problem was not only limited to optional strings, and that the generator was not generating optionals for all types. You can merge this pull request into a Git repository by running: $ git pull https://github.com/davinchia/thrift constant-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1311.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1311 commit de3d3a76bdaf2baa3eab9c6fdd8d3b31e7887f55 Author: Davin Chia Ming Juay Date: 2017-07-20T08:08:48Z logging to track path; modified function to pass optional value over commit f038cc0d6504ecbf30bd13a1daa0f8fc8201fad4 Author: Davin Chia Ming Juay Date: 2017-07-20T08:53:24Z fix generation for bool and int constant values commit 7c58a2555203aac67aec3d81c71dd807def4de59 Author: Davin Chia Ming Juay Date: 2017-07-20T09:05:06Z fix for double; make sure int type is precise commit 04f07888c714415a94121a774d26738eda0b21ac Author: Davin Chia Ming Juay Date: 2017-07-20T09:13:30Z fix for string commit acd26f0e9e1c0fc47122b9f2a16573ee84fcc67e Author: Davin Chia Ming Juay Date: 2017-07-20T09:24:30Z remove binary from opt because it is not needed commit 54e97c3fc09737a15380d82d9cd5a9ac049d9095 Author: Davin Chia Ming Juay Date: 2017-07-20T20:23:05Z remove logging > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106111#comment-16106111 ] Jens Geyer commented on THRIFT-4253: Hi, Thanks! Two things: * May I ask you for adding a small test case under {{/lib/go/test}} to verify the changes? * There are some sporadic whitespace changes that should be reverted. Also {{main.cc}} and {{t_program.h}} seem to have some unrelated code formatting changes that probably should be reverted as well. [~calcifer], any further comments from your side? > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106166#comment-16106166 ] Can Celasun commented on THRIFT-4253: - Definitely needs some tests, but otherwise LGTM. [~DavinC] you can ignore Travis failures if they are not related to Go. > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106201#comment-16106201 ] Davin Chia commented on THRIFT-4253: [~jensg] Made the whitespace changes. Will be happy to write some tests. Can I have a brief introduction to the testing workflow? Spent half an hour looking at the code but don't really know where to start. Is {{make cross}} the command to run the test suite? Is there any specific place you want me to insert the test or do I create my own test file? > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (THRIFT-4253) Go generator assigns strings to field in const instead of pointers.
[ https://issues.apache.org/jira/browse/THRIFT-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106243#comment-16106243 ] Jens Geyer commented on THRIFT-4253: You have to run {{make check}} inside the {{/lib/go/test}} dir. This runs these tests by (basically) [invoking {{go test}} on them|https://golang.org/pkg/testing]. New tests need to be added to {{Makefile.am}}. It should be clear by looking at the existing test cases how everything works, just follow the model given by existing test cases. {{make cross}} is for the cross language Test Suite located under {{/test}}. That's a different thing. > Go generator assigns strings to field in const instead of pointers. > --- > > Key: THRIFT-4253 > URL: https://issues.apache.org/jira/browse/THRIFT-4253 > Project: Thrift > Issue Type: Bug > Components: Go - Compiler >Affects Versions: 0.10.0 > Environment: Generated using docker-thrift. >Reporter: Davin Chia > > Given the follow thrift definition: > {code:java} > struct custom { > 1: required string field_a; > 2: optional string field_b; > } > {code} > with the following constant map defined in the same file: > {code:java} > const map custom_map = { > ..., > 42 : { 'field_a':'www.testa.com', 'field_b':'www.testb.com'}, > ..., > } > {code} > Thrift generates the following go struct in the main {code:java}x.go{code} > file: > {code:java} > type Custom struct { > FieldA string `thrift:"field_a,required" db:"field_a" json:"field_a"` > FieldB *string `thrift:"field_b,2" db:"field_b" json:"field_b,omitempty"` > } > {code} > with the corresponding assignments in the {code:java} X-consts.go {code} file: > {code:java} > CUSTOM_MAP = map[int]*Custom { > ..., > 42: &Custom {FieldA: "www.testa.com", FieldB: "www.testb.com"}, > ..., > } > {code} > I assume field_b's pointer type is to allow for null values as per the > optional. The generator should be assigning pointers instead of strings. > I'm not sure how much effort it would take to fix this. I am encountering > this problem at work right and would be very happy to starting working on > this with some guidance from experts around here. -- This message was sent by Atlassian JIRA (v6.4.14#64029)