From dev-return-51339-archive-asf-public=cust-asf.ponee.io@thrift.apache.org Wed Jan 31 12:26:04 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id D76CE180662 for ; Wed, 31 Jan 2018 12:26:04 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id C72D5160C25; Wed, 31 Jan 2018 11:26:04 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id EA26F160C35 for ; Wed, 31 Jan 2018 12:26:03 +0100 (CET) Received: (qmail 22248 invoked by uid 500); 31 Jan 2018 11:26:03 -0000 Mailing-List: contact dev-help@thrift.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@thrift.apache.org Delivered-To: mailing list dev@thrift.apache.org Received: (qmail 22235 invoked by uid 99); 31 Jan 2018 11:26:03 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 31 Jan 2018 11:26:03 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 73683C0326 for ; Wed, 31 Jan 2018 11:26:02 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -109.511 X-Spam-Level: X-Spam-Status: No, score=-109.511 tagged_above=-999 required=6.31 tests=[ENV_AND_HDR_SPF_MATCH=-0.5, KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_DEF_SPF_WL=-7.5, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id fGCIz3AesvKT for ; Wed, 31 Jan 2018 11:26:01 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 022655FB04 for ; Wed, 31 Jan 2018 11:26:01 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 71B8AE01D8 for ; Wed, 31 Jan 2018 11:26:00 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id 299F1240FA for ; Wed, 31 Jan 2018 11:26:00 +0000 (UTC) Date: Wed, 31 Jan 2018 11:26:00 +0000 (UTC) From: "Craig Ringer (JIRA)" To: dev@thrift.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Comment Edited] (THRIFT-4484) C++ codegen invalid for optional self-membership MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/THRIFT-4484?page=3Dcom.atlassia= n.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D163= 46460#comment-16346460 ]=20 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 serial= ized. Now you de-serialize the bits and get what ... an A that contains another c= opy of A =3D *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 =3D *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]=C2=A0"Tree/Recursive struct support in thrift". > =C2=A0 > 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=20 > } > {code} > This works fine for languages like Java and Go. But in C++ it generates n= onsensical code that cannot compile because it contains a by-value member o= f 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: > =20 > /* 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 =E2=80=98downstream=E2=80=99 has in= complete type =E2=80=98jaegertracing::crossdock::thrift::Downstream=E2=80= =99 > Downstream downstream; > ^~~~~~~~~~ > tracetest_types.h:47:7: note: definition of =E2=80=98class jaegertracing:= :crossdock::thrift::Downstream=E2=80=99 is not complete until the closing b= race > 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 presumabl= y 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)