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: [email protected] 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: [email protected] 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
