Jens,
Um I'd have C++ in already and be working on Python, but I can't
run the CI tests in docker, and there's a failure that shouldn't be there
(a bug with code I didn't touch, in languages I didn't touch) . I tried to
follow the instructions and failed, then sent email, and, well, no
Hi Chet,
expecting your pull request :-)
Have fun,
JensG
-Ursprüngliche Nachricht-
From: Chet Murthy
Sent: Saturday, December 2, 2017 7:39 PM
To: dev@thrift.apache.org
Subject: Re: Thrift unions don't work as documented
Jens,
I think I agree with you on all points. Let me re
I should have said that there were changes, but nothing that looked linear
in the number of fields. But like I said, I no longer know any relevant
assembly languages
On Sat, Dec 2, 2017 at 10:39 AM, Chet Murthy wrote:
>
> [Just out of curiousity, I wrote a program with 64 bit fields, and l
I should have added that, the actual code for "erase bits and set new one"
would be something like:
this->__sset = _t_const_value__isset() ;
this->__isset.map_val = true;
It seems to me that a decent optimizer ought to be able to collapse all
that into a single store. But I'm
is true, the union choice here is simply incorrect. If it works
> today, it relies on undefined behavuiour and that certainly can't be a good
> thing as any bug fix can potentially break it. That relates exactly to what
> I wrote earlier:
>
> > If unions are used *as they are suppose
xactly to what
I wrote earlier:
> If unions are used *as they are supposed to be*, this
> should be a nonbreaker. If they arent, it is wrong anyway.
So let's fix it.
Have fun,
JensG
-Ursprüngliche Nachricht-
From: Chet Murthy
Sent: Saturday, December 2, 2017 12:28 AM
To
Jens,
To add a little more detail:
I could easily provide a series of patches:
(1) fix plugin.thirft and the code in compiler/cpp so that it uses unions
correctly.
(2) then for each language, provide a fix that enforces in __set_
and TProtocol::{write,read} that unions obey the "exactly one fie
Jens,
YES! That's why I sent the email, rather than diving into code! *grin*
(1) the current implementation in C++ and Ocaml treats a union as a struct
with all-optional fields.
(2) this means that if code sets two fields of a union, both fields get
sent on-the-wire, and the receiving end dese
the code.
Have fun,
JensG
-Ursprüngliche Nachricht-
From: Chet Murthy
Sent: Friday, December 1, 2017 9:37 AM
To: dev@thrift.apache.org
Subject: Thrift unions don't work as documented
I dug into this question b/c I noticed that in plugin.thrift, the
t_const_value seemed to not be behaving as a
I dug into this question b/c I noticed that in plugin.thrift, the
t_const_value seemed to not be behaving as a union should. To wit, it
seems that in the case of a type like
struct Foo {
1: required Enum E x = E.A
}
(for some enum E), the t_const_value that represents the initializer, ends
up
10 matches
Mail list logo
