You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Allen George <al...@gmail.com> on 2016/11/04 01:59:41 UTC
What's the right approach to dealing with reserved words in thrift structs?
The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword
"type" in my target language. What's the right approach do dealing
with reserved keywords during code generation? Obviously I'd prefer
that "type" wasn't used at all, but...should I turn it into another
name? Should I simply fail instead?
Thanks,
Allen
Terminal Musings: http://www.allengeorge.com/
Raft in Java: https://github.com/allengeorge/libraft/
Twitter: https://twitter.com/allenageorge/
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Allen George <al...@gmail.com>.
Thanks guys - much appreciated.
I'll take the steps suggested:
1. Add "type" as a reserved word to thrift.ll
2. Change the test to use __type (or something similar)
I've already done "2" - and gone far enough to know that maps of f64s
are definitely a pain point in Rust that I'll have to figure out.
Thanks!
Allen
Terminal Musings: http://www.allengeorge.com/
Raft in Java: https://github.com/allengeorge/libraft/
Twitter: https://twitter.com/allenageorge/
On Fri, Nov 4, 2016 at 5:04 PM, Randy Abernethy
<ra...@gmail.com> wrote:
> Hey Allen,
>
> Also excited to hear about the Rust work! Agree with Jake, on the pain
> level associated with adding type as a keyword. However, type is probably a
> pretty good word to add to the keyword list if we were to add one.
>
> Similar/Identical keywords:
>
> - Rust, Go, Delphi, Scala: type
> - C#, ECMAScript 6 typeof
> - C++ typedef, typeid, typename
>
>
> Obviously some languages avoid the problem through context (like Go and
> Delphi). Perhaps a reasonable approach might be to deprecate it in IDL
> today, change the tests to use __type or some such and soldier on with the
> Rust addition?
>
> -Randy
>
> On Fri, Nov 4, 2016 at 1:34 PM, Jake Farrell <jf...@apache.org> wrote:
>
>> Hey Allen
>> Really cool to hear you are working on Rust support, know that others would
>> appreciate having it available. Currently "type" is not a listed reserved
>> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
>> that way you would have to add it to the reserved list and modify the test
>> to no longer support "type" as an identifier.
>>
>> This would cause a lot of pain for others as "type" is a pretty commonly
>> used term and would break exiting expected behavior for anyone using the
>> identifier "type" in their idl today.
>>
>> -Jake
>>
>>
>>
>> On Fri, Nov 4, 2016 at 4:07 PM, Allen George <al...@gmail.com>
>> wrote:
>>
>> > Hi Jim,
>> >
>> > FWIW, the Thrift file I'm talking about is "TestThrift.thrift" that's
>> > used as part of the cross-platform tests. And, you're right: I didn't
>> > specify my language. I'm *trying* to implement Thrift support in Rust.
>> > I've got part of the code-gen working, and part of the binary protocol
>> > as well, but there's obviously still a lot of work to do. "type" is
>> > used to define a typedef (aka. type alias) in Rust, which means that
>> > the "Bonk" struct (the first struct there) doesn't work anymore.
>> >
>> > Allen
>> > Terminal Musings: http://www.allengeorge.com/
>> > Raft in Java: https://github.com/allengeorge/libraft/
>> > Twitter: https://twitter.com/allenageorge/
>> >
>> >
>> > On Fri, Nov 4, 2016 at 12:35 PM, Jim King <Ji...@simplivity.com>
>> wrote:
>> > > The use of "type" as a field name seems suspect to me in a test thrift
>> > file for the reason you described.
>> > >
>> > > This is similar to the use of "what" that was in some example thrift
>> > files exception structures before we added what() to the C++ library,
>> > breaking those examples. Fortunately one can rename the field if using
>> > BinaryProtocol because it only cares about field ordinals, not names, and
>> > remain backwards compatible.
>> > >
>> > > Perhaps we need to update the list of reserved keywords and enforce
>> them?
>> > > Another possibility is to change the code generator to generate a field
>> > in a structure that has a different name like "__type", however would the
>> > definition of "getType()" then fail in your language?
>> > >
>> > > By the way, what is your target language? You didn't specify.
>> > >
>> > > - Jim
>> > >
>> > > -----Original Message-----
>> > > From: Allen George [mailto:allen.george@gmail.com]
>> > > Sent: Thursday, November 03, 2016 10:00 PM
>> > > To: dev@thrift.apache.org
>> > > Subject: What's the right approach to dealing with reserved words in
>> > thrift structs?
>> > >
>> > > The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword "type"
>> > in my target language. What's the right approach do dealing with reserved
>> > keywords during code generation? Obviously I'd prefer that "type" wasn't
>> > used at all, but...should I turn it into another name? Should I simply
>> fail
>> > instead?
>> > >
>> > > Thanks,
>> > > Allen
>> > >
>> > > Terminal Musings: http://www.allengeorge.com/ Raft in Java:
>> > https://github.com/allengeorge/libraft/
>> > > Twitter: https://twitter.com/allenageorge/
>> > > ------------------------------------------------------------
>> > ---------------------------
>> > > PRIVACY STATEMENT:
>> > > This message is a PRIVATE communication. This message and all
>> > attachments are a private communication sent by SimpliVity and are
>> > considered to be confidential or protected by privilege. If you are not
>> the
>> > intended recipient, you are hereby notified that any disclosure, copying,
>> > distribution or use of the information contained in or attached to this
>> > message is strictly prohibited. Please notify the sender of the delivery
>> > error by replying to this message, and then delete it from your system.
>> > > ------------------------------------------------------------
>> > ---------------------------
>> >
>>
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Randy Abernethy <ra...@gmail.com>.
Hey Allen,
Also excited to hear about the Rust work! Agree with Jake, on the pain
level associated with adding type as a keyword. However, type is probably a
pretty good word to add to the keyword list if we were to add one.
Similar/Identical keywords:
- Rust, Go, Delphi, Scala: type
- C#, ECMAScript 6 typeof
- C++ typedef, typeid, typename
Obviously some languages avoid the problem through context (like Go and
Delphi). Perhaps a reasonable approach might be to deprecate it in IDL
today, change the tests to use __type or some such and soldier on with the
Rust addition?
-Randy
On Fri, Nov 4, 2016 at 1:34 PM, Jake Farrell <jf...@apache.org> wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others would
> appreciate having it available. Currently "type" is not a listed reserved
> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> that way you would have to add it to the reserved list and modify the test
> to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty commonly
> used term and would break exiting expected behavior for anyone using the
> identifier "type" in their idl today.
>
> -Jake
>
>
>
> On Fri, Nov 4, 2016 at 4:07 PM, Allen George <al...@gmail.com>
> wrote:
>
> > Hi Jim,
> >
> > FWIW, the Thrift file I'm talking about is "TestThrift.thrift" that's
> > used as part of the cross-platform tests. And, you're right: I didn't
> > specify my language. I'm *trying* to implement Thrift support in Rust.
> > I've got part of the code-gen working, and part of the binary protocol
> > as well, but there's obviously still a lot of work to do. "type" is
> > used to define a typedef (aka. type alias) in Rust, which means that
> > the "Bonk" struct (the first struct there) doesn't work anymore.
> >
> > Allen
> > Terminal Musings: http://www.allengeorge.com/
> > Raft in Java: https://github.com/allengeorge/libraft/
> > Twitter: https://twitter.com/allenageorge/
> >
> >
> > On Fri, Nov 4, 2016 at 12:35 PM, Jim King <Ji...@simplivity.com>
> wrote:
> > > The use of "type" as a field name seems suspect to me in a test thrift
> > file for the reason you described.
> > >
> > > This is similar to the use of "what" that was in some example thrift
> > files exception structures before we added what() to the C++ library,
> > breaking those examples. Fortunately one can rename the field if using
> > BinaryProtocol because it only cares about field ordinals, not names, and
> > remain backwards compatible.
> > >
> > > Perhaps we need to update the list of reserved keywords and enforce
> them?
> > > Another possibility is to change the code generator to generate a field
> > in a structure that has a different name like "__type", however would the
> > definition of "getType()" then fail in your language?
> > >
> > > By the way, what is your target language? You didn't specify.
> > >
> > > - Jim
> > >
> > > -----Original Message-----
> > > From: Allen George [mailto:allen.george@gmail.com]
> > > Sent: Thursday, November 03, 2016 10:00 PM
> > > To: dev@thrift.apache.org
> > > Subject: What's the right approach to dealing with reserved words in
> > thrift structs?
> > >
> > > The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword "type"
> > in my target language. What's the right approach do dealing with reserved
> > keywords during code generation? Obviously I'd prefer that "type" wasn't
> > used at all, but...should I turn it into another name? Should I simply
> fail
> > instead?
> > >
> > > Thanks,
> > > Allen
> > >
> > > Terminal Musings: http://www.allengeorge.com/ Raft in Java:
> > https://github.com/allengeorge/libraft/
> > > Twitter: https://twitter.com/allenageorge/
> > > ------------------------------------------------------------
> > ---------------------------
> > > PRIVACY STATEMENT:
> > > This message is a PRIVATE communication. This message and all
> > attachments are a private communication sent by SimpliVity and are
> > considered to be confidential or protected by privilege. If you are not
> the
> > intended recipient, you are hereby notified that any disclosure, copying,
> > distribution or use of the information contained in or attached to this
> > message is strictly prohibited. Please notify the sender of the delivery
> > error by replying to this message, and then delete it from your system.
> > > ------------------------------------------------------------
> > ---------------------------
> >
>
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Randy Abernethy <ra...@gmail.com>.
I agree with Jens, type should ultimately be reserved. If you make "type"
deprecated first, it will give everyone a window of time before it becomes
officially reserved (a version or two). Removing type from the cross test
is a no brainer and shoul dbe all you need to get Rust integrated with the
trunk in the mean time.
On Sat, Nov 5, 2016 at 6:12 PM, Allen George <al...@gmail.com> wrote:
> While (obviously!) adding "type" to the reserved keyword list would be the
> easiest for me, I'm happy to change my code-generation to deal with
> Rust-reserved keywords if that you guys determine is the best way forward.
> As for when it'll show up in the main Thrift repo...that is my intent :)
> There's a lot of work to do, but I want to complete it - if only so I can
> create my command-line tools in Rust at work ;)
> Allen
>
>
>
>
>
>
> On Sat, Nov 5, 2016 at 6:16 AM -0400, "Jens Geyer" <je...@hotmail.com>
> wrote:
>
>
>
>
>
>
>
>
>
>
>
> Re the OPs question: "type" should very likely really be added to the
> global
> list. So +1 from me.
>
>
> -----Ursprüngliche Nachricht-----
> From: Jens Geyer
> Sent: Saturday, November 5, 2016 10:35 AM
> To: dev@thrift.apache.org
> Subject: Re: What's the right approach to dealing with reserved words in
> thrift structs?
>
> Hi *,
>
> I agree with what Ben wrote below (I have written sth. similar in a ticket
> just a few days ago). Bottom line from my viewpoint:
>
> (a) Blowing the global list of reserved keywords to cover every exotic case
> seems not be useful. The list should countain only those identifiers that
> are frequently used in a number of languages, any other keyword specifics
> is
> up to the language generator. I am not going to suggest a specific number
> here, as this is to some extent a case-by-case decision, and a moving
> target
> because we will continue to add languages and language versions over time.
>
> (b) As noted below, forbidding formerly valid keywords introduces the risk
> of breaking existing code for *all* target languages, not just the ones
> affected. So from that viewpoint, having a language-special treatment that
> makes it work seems generally more useful than just plain forbid them. I'm
> personally very hesitant to add anything to the global list at all, just
> because of that single reason.
>
> (c) We already have certain generator-specific implementations for this,
> which is great. However, what the Thrift generators lacks is a common
> concept, which could streamline the generator implementations and make it
> easy to add other useful features, such as the warnings mentioned by Ben.
>
> BTW, greets to the Rusties. Nice to hear again from you! Any news regarding
> merging the stuff into Apache Thrift? ;-)
>
>
> Have fun,
> JensG
>
>
> -----Ursprüngliche Nachricht-----
> From: BCG
> Sent: Saturday, November 5, 2016 6:59 AM
> To: dev@thrift.apache.org
> Subject: Re: What's the right approach to dealing with reserved words in
> thrift structs?
>
> On 11/04/2016 04:34 PM, Jake Farrell wrote:
> > Hey Allen
> > Really cool to hear you are working on Rust support, know that others
> > would
> > appreciate having it available. Currently "type" is not a listed reserved
> > word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> > that way you would have to add it to the reserved list and modify the
> test
> > to no longer support "type" as an identifier.
> >
> > This would cause a lot of pain for others as "type" is a pretty commonly
> > used term and would break exiting expected behavior for anyone using the
> > identifier "type" in their idl today.
> >
> > -Jake
> >
> I've run into the same problem this week working on THRIFT-3301...
> specifically, there are many Java keywords that can be valid Thrift
> identifiers... for example, most/all of the following cause compilation
> errors in generated Java code, but only a handful are in the reserved
> words list:
>
> http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
>
> Seems like Rust has some crossover with this list, as well as some of
> its own keywords that are not reserved in Thrift:
>
> https://doc.rust-lang.org/grammar.html#keywords
>
> It seems to me like the list of reserved words in Thrift should be kept
> small except for words that are extremely common across languages. Even
> if it might be justified to add some of the keywords to the reserved
> list, it seems impractical to have a policy of adding all keywords for
> all languages. Furthermore, even aside from keywords, some languages
> may have unique problems with specific identifiers (for example,
> generating code for a struct with a field named 'prototype' will
> probably be a problem in Javascript). IMO the responsibility should
> fall on the language-specific generator to work around such issues...
> for example the Java generator could be modified to prefix identifiers
> that might be problematic with underscores or $ in order to avoid
> problems. I assume that a Rust generator could do something similar for
> Rust-specific keywords, as could any other language I presume.
>
> One thing I thought of however that might be a useful feature would be
> to provide a means for generators to register a list of words that would
> need to be munged, cause the generator to error out, or might just be
> inconvenient for users of that language. The parser could then issue a
> warning if it comes across such a keyword, so at least the designer of
> the IDL might realize early on that another name might be a better
> choice. It could be something like:
>
> [WARNING:something.thrift:42] The identifier 'final' may be problematic
> for the following language(s): Java, Rust
>
> Just my two cents.
>
> Thanks
>
> Ben
>
>
>
>
>
>
>
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Allen George <al...@gmail.com>.
While (obviously!) adding "type" to the reserved keyword list would be the easiest for me, I'm happy to change my code-generation to deal with Rust-reserved keywords if that you guys determine is the best way forward.
As for when it'll show up in the main Thrift repo...that is my intent :) There's a lot of work to do, but I want to complete it - if only so I can create my command-line tools in Rust at work ;)
Allen
On Sat, Nov 5, 2016 at 6:16 AM -0400, "Jens Geyer" <je...@hotmail.com> wrote:
Re the OPs question: "type" should very likely really be added to the global
list. So +1 from me.
-----Ursprüngliche Nachricht-----
From: Jens Geyer
Sent: Saturday, November 5, 2016 10:35 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in
thrift structs?
Hi *,
I agree with what Ben wrote below (I have written sth. similar in a ticket
just a few days ago). Bottom line from my viewpoint:
(a) Blowing the global list of reserved keywords to cover every exotic case
seems not be useful. The list should countain only those identifiers that
are frequently used in a number of languages, any other keyword specifics is
up to the language generator. I am not going to suggest a specific number
here, as this is to some extent a case-by-case decision, and a moving target
because we will continue to add languages and language versions over time.
(b) As noted below, forbidding formerly valid keywords introduces the risk
of breaking existing code for *all* target languages, not just the ones
affected. So from that viewpoint, having a language-special treatment that
makes it work seems generally more useful than just plain forbid them. I'm
personally very hesitant to add anything to the global list at all, just
because of that single reason.
(c) We already have certain generator-specific implementations for this,
which is great. However, what the Thrift generators lacks is a common
concept, which could streamline the generator implementations and make it
easy to add other useful features, such as the warnings mentioned by Ben.
BTW, greets to the Rusties. Nice to hear again from you! Any news regarding
merging the stuff into Apache Thrift? ;-)
Have fun,
JensG
-----Ursprüngliche Nachricht-----
From: BCG
Sent: Saturday, November 5, 2016 6:59 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in
thrift structs?
On 11/04/2016 04:34 PM, Jake Farrell wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others
> would
> appreciate having it available. Currently "type" is not a listed reserved
> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> that way you would have to add it to the reserved list and modify the test
> to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty commonly
> used term and would break exiting expected behavior for anyone using the
> identifier "type" in their idl today.
>
> -Jake
>
I've run into the same problem this week working on THRIFT-3301...
specifically, there are many Java keywords that can be valid Thrift
identifiers... for example, most/all of the following cause compilation
errors in generated Java code, but only a handful are in the reserved
words list:
http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
Seems like Rust has some crossover with this list, as well as some of
its own keywords that are not reserved in Thrift:
https://doc.rust-lang.org/grammar.html#keywords
It seems to me like the list of reserved words in Thrift should be kept
small except for words that are extremely common across languages. Even
if it might be justified to add some of the keywords to the reserved
list, it seems impractical to have a policy of adding all keywords for
all languages. Furthermore, even aside from keywords, some languages
may have unique problems with specific identifiers (for example,
generating code for a struct with a field named 'prototype' will
probably be a problem in Javascript). IMO the responsibility should
fall on the language-specific generator to work around such issues...
for example the Java generator could be modified to prefix identifiers
that might be problematic with underscores or $ in order to avoid
problems. I assume that a Rust generator could do something similar for
Rust-specific keywords, as could any other language I presume.
One thing I thought of however that might be a useful feature would be
to provide a means for generators to register a list of words that would
need to be munged, cause the generator to error out, or might just be
inconvenient for users of that language. The parser could then issue a
warning if it comes across such a keyword, so at least the designer of
the IDL might realize early on that another name might be a better
choice. It could be something like:
[WARNING:something.thrift:42] The identifier 'final' may be problematic
for the following language(s): Java, Rust
Just my two cents.
Thanks
Ben
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Jens Geyer <je...@hotmail.com>.
Re the OPs question: "type" should very likely really be added to the global
list. So +1 from me.
-----Ursprüngliche Nachricht-----
From: Jens Geyer
Sent: Saturday, November 5, 2016 10:35 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in
thrift structs?
Hi *,
I agree with what Ben wrote below (I have written sth. similar in a ticket
just a few days ago). Bottom line from my viewpoint:
(a) Blowing the global list of reserved keywords to cover every exotic case
seems not be useful. The list should countain only those identifiers that
are frequently used in a number of languages, any other keyword specifics is
up to the language generator. I am not going to suggest a specific number
here, as this is to some extent a case-by-case decision, and a moving target
because we will continue to add languages and language versions over time.
(b) As noted below, forbidding formerly valid keywords introduces the risk
of breaking existing code for *all* target languages, not just the ones
affected. So from that viewpoint, having a language-special treatment that
makes it work seems generally more useful than just plain forbid them. I'm
personally very hesitant to add anything to the global list at all, just
because of that single reason.
(c) We already have certain generator-specific implementations for this,
which is great. However, what the Thrift generators lacks is a common
concept, which could streamline the generator implementations and make it
easy to add other useful features, such as the warnings mentioned by Ben.
BTW, greets to the Rusties. Nice to hear again from you! Any news regarding
merging the stuff into Apache Thrift? ;-)
Have fun,
JensG
-----Ursprüngliche Nachricht-----
From: BCG
Sent: Saturday, November 5, 2016 6:59 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in
thrift structs?
On 11/04/2016 04:34 PM, Jake Farrell wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others
> would
> appreciate having it available. Currently "type" is not a listed reserved
> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> that way you would have to add it to the reserved list and modify the test
> to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty commonly
> used term and would break exiting expected behavior for anyone using the
> identifier "type" in their idl today.
>
> -Jake
>
I've run into the same problem this week working on THRIFT-3301...
specifically, there are many Java keywords that can be valid Thrift
identifiers... for example, most/all of the following cause compilation
errors in generated Java code, but only a handful are in the reserved
words list:
http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
Seems like Rust has some crossover with this list, as well as some of
its own keywords that are not reserved in Thrift:
https://doc.rust-lang.org/grammar.html#keywords
It seems to me like the list of reserved words in Thrift should be kept
small except for words that are extremely common across languages. Even
if it might be justified to add some of the keywords to the reserved
list, it seems impractical to have a policy of adding all keywords for
all languages. Furthermore, even aside from keywords, some languages
may have unique problems with specific identifiers (for example,
generating code for a struct with a field named 'prototype' will
probably be a problem in Javascript). IMO the responsibility should
fall on the language-specific generator to work around such issues...
for example the Java generator could be modified to prefix identifiers
that might be problematic with underscores or $ in order to avoid
problems. I assume that a Rust generator could do something similar for
Rust-specific keywords, as could any other language I presume.
One thing I thought of however that might be a useful feature would be
to provide a means for generators to register a list of words that would
need to be munged, cause the generator to error out, or might just be
inconvenient for users of that language. The parser could then issue a
warning if it comes across such a keyword, so at least the designer of
the IDL might realize early on that another name might be a better
choice. It could be something like:
[WARNING:something.thrift:42] The identifier 'final' may be problematic
for the following language(s): Java, Rust
Just my two cents.
Thanks
Ben
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Jens Geyer <je...@hotmail.com>.
Hi *,
I agree with what Ben wrote below (I have written sth. similar in a ticket
just a few days ago). Bottom line from my viewpoint:
(a) Blowing the global list of reserved keywords to cover every exotic case
seems not be useful. The list should countain only those identifiers that
are frequently used in a number of languages, any other keyword specifics is
up to the language generator. I am not going to suggest a specific number
here, as this is to some extent a case-by-case decision, and a moving target
because we will continue to add languages and language versions over time.
(b) As noted below, forbidding formerly valid keywords introduces the risk
of breaking existing code for *all* target languages, not just the ones
affected. So from that viewpoint, having a language-special treatment that
makes it work seems generally more useful than just plain forbid them. I'm
personally very hesitant to add anything to the global list at all, just
because of that single reason.
(c) We already have certain generator-specific implementations for this,
which is great. However, what the Thrift generators lacks is a common
concept, which could streamline the generator implementations and make it
easy to add other useful features, such as the warnings mentioned by Ben.
BTW, greets to the Rusties. Nice to hear again from you! Any news regarding
merging the stuff into Apache Thrift? ;-)
Have fun,
JensG
-----Ursprüngliche Nachricht-----
From: BCG
Sent: Saturday, November 5, 2016 6:59 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in
thrift structs?
On 11/04/2016 04:34 PM, Jake Farrell wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others
> would
> appreciate having it available. Currently "type" is not a listed reserved
> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> that way you would have to add it to the reserved list and modify the test
> to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty commonly
> used term and would break exiting expected behavior for anyone using the
> identifier "type" in their idl today.
>
> -Jake
>
I've run into the same problem this week working on THRIFT-3301...
specifically, there are many Java keywords that can be valid Thrift
identifiers... for example, most/all of the following cause compilation
errors in generated Java code, but only a handful are in the reserved
words list:
http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
Seems like Rust has some crossover with this list, as well as some of
its own keywords that are not reserved in Thrift:
https://doc.rust-lang.org/grammar.html#keywords
It seems to me like the list of reserved words in Thrift should be kept
small except for words that are extremely common across languages. Even
if it might be justified to add some of the keywords to the reserved
list, it seems impractical to have a policy of adding all keywords for
all languages. Furthermore, even aside from keywords, some languages
may have unique problems with specific identifiers (for example,
generating code for a struct with a field named 'prototype' will
probably be a problem in Javascript). IMO the responsibility should
fall on the language-specific generator to work around such issues...
for example the Java generator could be modified to prefix identifiers
that might be problematic with underscores or $ in order to avoid
problems. I assume that a Rust generator could do something similar for
Rust-specific keywords, as could any other language I presume.
One thing I thought of however that might be a useful feature would be
to provide a means for generators to register a list of words that would
need to be munged, cause the generator to error out, or might just be
inconvenient for users of that language. The parser could then issue a
warning if it comes across such a keyword, so at least the designer of
the IDL might realize early on that another name might be a better
choice. It could be something like:
[WARNING:something.thrift:42] The identifier 'final' may be problematic
for the following language(s): Java, Rust
Just my two cents.
Thanks
Ben
RE: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Jim King <Ji...@simplivity.com>.
I like the notion of making it a warning (that can be disabled), and the suggested text looks great.
-----Original Message-----
From: BCG [mailto:bgould@hushmail.com]
Sent: Saturday, November 05, 2016 1:59 AM
To: dev@thrift.apache.org
Subject: Re: What's the right approach to dealing with reserved words in thrift structs?
On 11/04/2016 04:34 PM, Jake Farrell wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others
> would appreciate having it available. Currently "type" is not a listed
> reserved word in the compiler/cpp/src/thrift/thriftl.ll, but due to
> rust using it that way you would have to add it to the reserved list
> and modify the test to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty
> commonly used term and would break exiting expected behavior for
> anyone using the identifier "type" in their idl today.
>
> -Jake
>
I've run into the same problem this week working on THRIFT-3301...
specifically, there are many Java keywords that can be valid Thrift identifiers... for example, most/all of the following cause compilation errors in generated Java code, but only a handful are in the reserved words list:
http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
Seems like Rust has some crossover with this list, as well as some of its own keywords that are not reserved in Thrift:
https://doc.rust-lang.org/grammar.html#keywords
It seems to me like the list of reserved words in Thrift should be kept small except for words that are extremely common across languages. Even if it might be justified to add some of the keywords to the reserved list, it seems impractical to have a policy of adding all keywords for all languages. Furthermore, even aside from keywords, some languages may have unique problems with specific identifiers (for example, generating code for a struct with a field named 'prototype' will probably be a problem in Javascript). IMO the responsibility should fall on the language-specific generator to work around such issues...
for example the Java generator could be modified to prefix identifiers that might be problematic with underscores or $ in order to avoid problems. I assume that a Rust generator could do something similar for Rust-specific keywords, as could any other language I presume.
One thing I thought of however that might be a useful feature would be to provide a means for generators to register a list of words that would need to be munged, cause the generator to error out, or might just be inconvenient for users of that language. The parser could then issue a warning if it comes across such a keyword, so at least the designer of the IDL might realize early on that another name might be a better choice. It could be something like:
[WARNING:something.thrift:42] The identifier 'final' may be problematic for the following language(s): Java, Rust
Just my two cents.
Thanks
Ben
---------------------------------------------------------------------------------------
PRIVACY STATEMENT:
This message is a PRIVATE communication. This message and all attachments are a private communication sent by SimpliVity and are considered to be confidential or protected by privilege. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or use of the information contained in or attached to this message is strictly prohibited. Please notify the sender of the delivery error by replying to this message, and then delete it from your system.
---------------------------------------------------------------------------------------
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by BCG <bg...@hushmail.com>.
On 11/04/2016 04:34 PM, Jake Farrell wrote:
> Hey Allen
> Really cool to hear you are working on Rust support, know that others would
> appreciate having it available. Currently "type" is not a listed reserved
> word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
> that way you would have to add it to the reserved list and modify the test
> to no longer support "type" as an identifier.
>
> This would cause a lot of pain for others as "type" is a pretty commonly
> used term and would break exiting expected behavior for anyone using the
> identifier "type" in their idl today.
>
> -Jake
>
I've run into the same problem this week working on THRIFT-3301...
specifically, there are many Java keywords that can be valid Thrift
identifiers... for example, most/all of the following cause compilation
errors in generated Java code, but only a handful are in the reserved
words list:
http://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
Seems like Rust has some crossover with this list, as well as some of
its own keywords that are not reserved in Thrift:
https://doc.rust-lang.org/grammar.html#keywords
It seems to me like the list of reserved words in Thrift should be kept
small except for words that are extremely common across languages. Even
if it might be justified to add some of the keywords to the reserved
list, it seems impractical to have a policy of adding all keywords for
all languages. Furthermore, even aside from keywords, some languages
may have unique problems with specific identifiers (for example,
generating code for a struct with a field named 'prototype' will
probably be a problem in Javascript). IMO the responsibility should
fall on the language-specific generator to work around such issues...
for example the Java generator could be modified to prefix identifiers
that might be problematic with underscores or $ in order to avoid
problems. I assume that a Rust generator could do something similar for
Rust-specific keywords, as could any other language I presume.
One thing I thought of however that might be a useful feature would be
to provide a means for generators to register a list of words that would
need to be munged, cause the generator to error out, or might just be
inconvenient for users of that language. The parser could then issue a
warning if it comes across such a keyword, so at least the designer of
the IDL might realize early on that another name might be a better
choice. It could be something like:
[WARNING:something.thrift:42] The identifier 'final' may be problematic
for the following language(s): Java, Rust
Just my two cents.
Thanks
Ben
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Jake Farrell <jf...@apache.org>.
Hey Allen
Really cool to hear you are working on Rust support, know that others would
appreciate having it available. Currently "type" is not a listed reserved
word in the compiler/cpp/src/thrift/thriftl.ll, but due to rust using it
that way you would have to add it to the reserved list and modify the test
to no longer support "type" as an identifier.
This would cause a lot of pain for others as "type" is a pretty commonly
used term and would break exiting expected behavior for anyone using the
identifier "type" in their idl today.
-Jake
On Fri, Nov 4, 2016 at 4:07 PM, Allen George <al...@gmail.com> wrote:
> Hi Jim,
>
> FWIW, the Thrift file I'm talking about is "TestThrift.thrift" that's
> used as part of the cross-platform tests. And, you're right: I didn't
> specify my language. I'm *trying* to implement Thrift support in Rust.
> I've got part of the code-gen working, and part of the binary protocol
> as well, but there's obviously still a lot of work to do. "type" is
> used to define a typedef (aka. type alias) in Rust, which means that
> the "Bonk" struct (the first struct there) doesn't work anymore.
>
> Allen
> Terminal Musings: http://www.allengeorge.com/
> Raft in Java: https://github.com/allengeorge/libraft/
> Twitter: https://twitter.com/allenageorge/
>
>
> On Fri, Nov 4, 2016 at 12:35 PM, Jim King <Ji...@simplivity.com> wrote:
> > The use of "type" as a field name seems suspect to me in a test thrift
> file for the reason you described.
> >
> > This is similar to the use of "what" that was in some example thrift
> files exception structures before we added what() to the C++ library,
> breaking those examples. Fortunately one can rename the field if using
> BinaryProtocol because it only cares about field ordinals, not names, and
> remain backwards compatible.
> >
> > Perhaps we need to update the list of reserved keywords and enforce them?
> > Another possibility is to change the code generator to generate a field
> in a structure that has a different name like "__type", however would the
> definition of "getType()" then fail in your language?
> >
> > By the way, what is your target language? You didn't specify.
> >
> > - Jim
> >
> > -----Original Message-----
> > From: Allen George [mailto:allen.george@gmail.com]
> > Sent: Thursday, November 03, 2016 10:00 PM
> > To: dev@thrift.apache.org
> > Subject: What's the right approach to dealing with reserved words in
> thrift structs?
> >
> > The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword "type"
> in my target language. What's the right approach do dealing with reserved
> keywords during code generation? Obviously I'd prefer that "type" wasn't
> used at all, but...should I turn it into another name? Should I simply fail
> instead?
> >
> > Thanks,
> > Allen
> >
> > Terminal Musings: http://www.allengeorge.com/ Raft in Java:
> https://github.com/allengeorge/libraft/
> > Twitter: https://twitter.com/allenageorge/
> > ------------------------------------------------------------
> ---------------------------
> > PRIVACY STATEMENT:
> > This message is a PRIVATE communication. This message and all
> attachments are a private communication sent by SimpliVity and are
> considered to be confidential or protected by privilege. If you are not the
> intended recipient, you are hereby notified that any disclosure, copying,
> distribution or use of the information contained in or attached to this
> message is strictly prohibited. Please notify the sender of the delivery
> error by replying to this message, and then delete it from your system.
> > ------------------------------------------------------------
> ---------------------------
>
Re: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Allen George <al...@gmail.com>.
Hi Jim,
FWIW, the Thrift file I'm talking about is "TestThrift.thrift" that's
used as part of the cross-platform tests. And, you're right: I didn't
specify my language. I'm *trying* to implement Thrift support in Rust.
I've got part of the code-gen working, and part of the binary protocol
as well, but there's obviously still a lot of work to do. "type" is
used to define a typedef (aka. type alias) in Rust, which means that
the "Bonk" struct (the first struct there) doesn't work anymore.
Allen
Terminal Musings: http://www.allengeorge.com/
Raft in Java: https://github.com/allengeorge/libraft/
Twitter: https://twitter.com/allenageorge/
On Fri, Nov 4, 2016 at 12:35 PM, Jim King <Ji...@simplivity.com> wrote:
> The use of "type" as a field name seems suspect to me in a test thrift file for the reason you described.
>
> This is similar to the use of "what" that was in some example thrift files exception structures before we added what() to the C++ library, breaking those examples. Fortunately one can rename the field if using BinaryProtocol because it only cares about field ordinals, not names, and remain backwards compatible.
>
> Perhaps we need to update the list of reserved keywords and enforce them?
> Another possibility is to change the code generator to generate a field in a structure that has a different name like "__type", however would the definition of "getType()" then fail in your language?
>
> By the way, what is your target language? You didn't specify.
>
> - Jim
>
> -----Original Message-----
> From: Allen George [mailto:allen.george@gmail.com]
> Sent: Thursday, November 03, 2016 10:00 PM
> To: dev@thrift.apache.org
> Subject: What's the right approach to dealing with reserved words in thrift structs?
>
> The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword "type" in my target language. What's the right approach do dealing with reserved keywords during code generation? Obviously I'd prefer that "type" wasn't used at all, but...should I turn it into another name? Should I simply fail instead?
>
> Thanks,
> Allen
>
> Terminal Musings: http://www.allengeorge.com/ Raft in Java: https://github.com/allengeorge/libraft/
> Twitter: https://twitter.com/allenageorge/
> ---------------------------------------------------------------------------------------
> PRIVACY STATEMENT:
> This message is a PRIVATE communication. This message and all attachments are a private communication sent by SimpliVity and are considered to be confidential or protected by privilege. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or use of the information contained in or attached to this message is strictly prohibited. Please notify the sender of the delivery error by replying to this message, and then delete it from your system.
> ---------------------------------------------------------------------------------------
RE: What's the right approach to dealing with reserved words in
thrift structs?
Posted by Jim King <Ji...@simplivity.com>.
The use of "type" as a field name seems suspect to me in a test thrift file for the reason you described.
This is similar to the use of "what" that was in some example thrift files exception structures before we added what() to the C++ library, breaking those examples. Fortunately one can rename the field if using BinaryProtocol because it only cares about field ordinals, not names, and remain backwards compatible.
Perhaps we need to update the list of reserved keywords and enforce them?
Another possibility is to change the code generator to generate a field in a structure that has a different name like "__type", however would the definition of "getType()" then fail in your language?
By the way, what is your target language? You didn't specify.
- Jim
-----Original Message-----
From: Allen George [mailto:allen.george@gmail.com]
Sent: Thursday, November 03, 2016 10:00 PM
To: dev@thrift.apache.org
Subject: What's the right approach to dealing with reserved words in thrift structs?
The struct "Bonk" (in ThriftTest.thrift) uses a reserved keyword "type" in my target language. What's the right approach do dealing with reserved keywords during code generation? Obviously I'd prefer that "type" wasn't used at all, but...should I turn it into another name? Should I simply fail instead?
Thanks,
Allen
Terminal Musings: http://www.allengeorge.com/ Raft in Java: https://github.com/allengeorge/libraft/
Twitter: https://twitter.com/allenageorge/
---------------------------------------------------------------------------------------
PRIVACY STATEMENT:
This message is a PRIVATE communication. This message and all attachments are a private communication sent by SimpliVity and are considered to be confidential or protected by privilege. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or use of the information contained in or attached to this message is strictly prohibited. Please notify the sender of the delivery error by replying to this message, and then delete it from your system.
---------------------------------------------------------------------------------------