From eddebec1b6136a3248c99aff1bcb4c3012a766be Mon Sep 17 00:00:00 2001 From: Max Burke Date: Sun, 15 Dec 2019 15:18:22 -0800 Subject: [PATCH] Bugfix for Rust generation of union fields named with language keywords (#5592) * Bugfix for Rust generation of union fields named with language keywords Looking at ParseField, it appears that in the case of unions, an extra field with a `UnionTypeFieldSuffix` is added to the type definition, however, if the name of this field is a keyword in the target language, it isn't escaped. For example, if generating code for rust for a union field named `type`, flatc will generate a (non-keyword escaped) field named `type_type` for this hidden union field, and one (keyword escaped) called `type_` for the actual union contents. When the union accessors are generated, they refer to this `type_type` field, but they will escape it mistakenly, generating code like this: ``` #[inline] #[allow(non_snake_case)] pub fn type__as_int(&self) -> Option> { if self.type__type() == Type::Int { self.type_().map(|u| Int::init_from_table(u)) } else { None } } ``` Which will fail to build because the field is called `self.type_type()`, not `self.type__type()`. * [Rust] Add crate-relative use statements for FBS includes. At present if a flatbuffer description includes a reference to a type in another file, the generated Rust code needs to be hand-modified to add the appropriate `use` statements. This assumes that the dependencies are built into the same crate, which I think is a reasonable assumption? * Revert "[Rust] Add crate-relative use statements for FBS includes." This reverts commit d554d79fecf5afd6da6fb993b30b4cd523a5889a. * Address comments raised in PR * Update documentation comments per feedback * Fix typo --- src/idl_gen_rust.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index fdf58e345..74aecbdf5 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -1305,6 +1305,7 @@ class RustGenerator : public BaseGenerator { auto u = field.value.type.enum_def; code_.SetValue("FIELD_NAME", Name(field)); + code_.SetValue("FIELD_TYPE_FIELD_NAME", field.name); for (auto u_it = u->Vals().begin(); u_it != u->Vals().end(); ++u_it) { auto &ev = **u_it; @@ -1325,8 +1326,19 @@ class RustGenerator : public BaseGenerator { code_ += " pub fn {{FIELD_NAME}}_as_{{U_ELEMENT_NAME}}(&self) -> " "Option<{{U_ELEMENT_TABLE_TYPE}}<'a>> {"; + // If the user defined schemas name a field that clashes with a + // language reserved word, flatc will try to escape the field name by + // appending an underscore. This works well for most cases, except + // one. When generating union accessors (and referring to them + // internally within the code generated here), an extra underscore + // will be appended to the name, causing build failures. + // + // This only happens when unions have members that overlap with + // language reserved words. + // + // To avoid this problem the type field name is used unescaped here: code_ += - " if self.{{FIELD_NAME}}_type() == {{U_ELEMENT_ENUM_TYPE}} {"; + " if self.{{FIELD_TYPE_FIELD_NAME}}_type() == {{U_ELEMENT_ENUM_TYPE}} {"; code_ += " self.{{FIELD_NAME}}().map(|u| " "{{U_ELEMENT_TABLE_TYPE}}::init_from_table(u))"; @@ -1832,3 +1844,6 @@ std::string RustMakeRule(const Parser &parser, const std::string &path, // TODO(rw): Generated code should generate endian-safe Debug impls. // TODO(rw): Generated code could use a Rust-only enum type to access unions, // instead of making the user use _type() to manually switch. +// TODO(maxburke): There should be test schemas added that use language +// keywords as fields of structs, tables, unions, enums, to make sure +// that internal code generated references escaped names correctly.