From 6da1cf79d90eb242e7da5318241d42279a3df3ba Mon Sep 17 00:00:00 2001 From: Max Burke Date: Sun, 19 Jan 2020 14:47:28 -0800 Subject: [PATCH] [rust] Add use declarations to Rust-generated bindings for imported FB definitions (#5645) * 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. * Add updated generated test files. * Fixing Rust test harness to handle new includes. Test binaries need to add references to generated code that's transitively included. This also has the knock-on in that this code (which is referenced by include directives directly in the flatbuffer schema files) also needs to be generated, hence the changes to generate_code.sh. * Test harnesses expect test data to be checked in. Put include_test2 files into the same directory as the include_test2 schema definition. Update all code generation scripts (forgot the batch file from last time). Path updates in Rust test. * Include updated generated code * Address comments raised in PR * Fix failing Rust tests. * Previous merge clobbered this branch change. * Add updated imports to benchmarks. * Clarifying comment per PR request * Update documentation comments per feedback * Remove non-Rust generated files for include tests, per feedback from @rw/@aardappel * Broken code generation batch file * Fix typo * Add TODO for tidying up use declaration traversal sometime in the future * Update test files. --- src/idl_gen_rust.cpp | 12 + tests/generate_code.bat | 2 + tests/generate_code.sh | 2 + tests/include_test/include_test1_generated.rs | 87 +++++++ .../sub/include_test2_generated.rs | 222 ++++++++++++++++++ tests/monster_test_generated.rs | 8 + .../namespace_test2_generated.rs | 3 + .../benches/flatbuffers_benchmarks.rs | 8 + tests/rust_usage_test/bin/alloc_check.rs | 8 + tests/rust_usage_test/bin/monster_example.rs | 9 + .../rust_usage_test/tests/integration_test.rs | 8 + 11 files changed, 369 insertions(+) create mode 100644 tests/include_test/include_test1_generated.rs create mode 100644 tests/include_test/sub/include_test2_generated.rs diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 6aaaddea9..9f317821d 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -1763,6 +1763,14 @@ class RustGenerator : public BaseGenerator { void GenNamespaceImports(const int white_spaces) { std::string indent = std::string(white_spaces, ' '); code_ += ""; + for (auto it = parser_.included_files_.begin(); + it != parser_.included_files_.end(); ++it) { + if (it->second.empty()) continue; + auto noext = flatbuffers::StripExtension(it->second); + auto basename = flatbuffers::StripPath(noext); + + code_ += indent + "use crate::" + basename + "_generated::*;"; + } code_ += indent + "use std::mem;"; code_ += indent + "use std::cmp::Ordering;"; code_ += ""; @@ -1847,3 +1855,7 @@ std::string RustMakeRule(const Parser &parser, const std::string &path, // 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. +// TODO(maxburke): We should see if there is a more flexible way of resolving +// module paths for use declarations. Right now if schemas refer to +// other flatbuffer files, the include paths in emitted Rust bindings +// are crate-relative which may undesirable. diff --git a/tests/generate_code.bat b/tests/generate_code.bat index 11846975e..7d5e28792 100644 --- a/tests/generate_code.bat +++ b/tests/generate_code.bat @@ -40,6 +40,8 @@ set TEST_NOINCL_FLAGS=%TEST_BASE_FLAGS% --no-includes --no-fb-import %TEST_NOINCL_FLAGS% %TEST_CPP_FLAGS% -o namespace_test namespace_test/namespace_test1.fbs namespace_test/namespace_test2.fbs || goto FAIL ..\%buildtype%\flatc.exe --cpp --java --csharp --js --ts --php %TEST_BASE_FLAGS% %TEST_CPP_FLAGS% -o union_vector ./union_vector/union_vector.fbs || goto FAIL +..\%buildtype%\flatc.exe --rust -I include_test -o include_test include_test/include_test1.fbs || goto FAIL +..\%buildtype%\flatc.exe --rust -I include_test -o include_test/sub include_test/sub/include_test2.fbs || goto FAIL ..\%buildtype%\flatc.exe -b --schema --bfbs-comments --bfbs-builtins -I include_test monster_test.fbs || goto FAIL ..\%buildtype%\flatc.exe -b --schema --bfbs-comments --bfbs-builtins -I include_test arrays_test.fbs || goto FAIL ..\%buildtype%\flatc.exe --jsonschema --schema -I include_test monster_test.fbs || goto FAIL diff --git a/tests/generate_code.sh b/tests/generate_code.sh index 57ed01456..a86ae2fed 100755 --- a/tests/generate_code.sh +++ b/tests/generate_code.sh @@ -37,6 +37,8 @@ $TEST_NOINCL_FLAGS $TEST_CPP_FLAGS -I include_test monster_test.fbs monsterdata_ $TEST_NOINCL_FLAGS $TEST_CPP_FLAGS -o namespace_test namespace_test/namespace_test1.fbs namespace_test/namespace_test2.fbs ../flatc --cpp --java --kotlin --csharp --js --ts --php $TEST_BASE_FLAGS $TEST_CPP_FLAGS -o union_vector ./union_vector/union_vector.fbs +../flatc --rust -I include_test -o include_test include_test/include_test1.fbs +../flatc --rust -I include_test -o include_test/sub include_test/sub/include_test2.fbs ../flatc -b --schema --bfbs-comments --bfbs-builtins -I include_test monster_test.fbs ../flatc -b --schema --bfbs-comments --bfbs-builtins -I include_test arrays_test.fbs ../flatc --jsonschema --schema -I include_test monster_test.fbs diff --git a/tests/include_test/include_test1_generated.rs b/tests/include_test/include_test1_generated.rs new file mode 100644 index 000000000..e6e7d4906 --- /dev/null +++ b/tests/include_test/include_test1_generated.rs @@ -0,0 +1,87 @@ +// automatically generated by the FlatBuffers compiler, do not modify + + + +use crate::include_test2_generated::*; +use std::mem; +use std::cmp::Ordering; + +extern crate flatbuffers; +use self::flatbuffers::EndianScalar; + +pub enum TableAOffset {} +#[derive(Copy, Clone, Debug, PartialEq)] + +pub struct TableA<'a> { + pub _tab: flatbuffers::Table<'a>, +} + +impl<'a> flatbuffers::Follow<'a> for TableA<'a> { + type Inner = TableA<'a>; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + Self { + _tab: flatbuffers::Table { buf: buf, loc: loc }, + } + } +} + +impl<'a> TableA<'a> { + #[inline] + pub fn init_from_table(table: flatbuffers::Table<'a>) -> Self { + TableA { + _tab: table, + } + } + #[allow(unused_mut)] + pub fn create<'bldr: 'args, 'args: 'mut_bldr, 'mut_bldr>( + _fbb: &'mut_bldr mut flatbuffers::FlatBufferBuilder<'bldr>, + args: &'args TableAArgs<'args>) -> flatbuffers::WIPOffset> { + let mut builder = TableABuilder::new(_fbb); + if let Some(x) = args.b { builder.add_b(x); } + builder.finish() + } + + pub const VT_B: flatbuffers::VOffsetT = 4; + + #[inline] + pub fn b(&self) -> Option> { + self._tab.get::>>(TableA::VT_B, None) + } +} + +pub struct TableAArgs<'a> { + pub b: Option>>, +} +impl<'a> Default for TableAArgs<'a> { + #[inline] + fn default() -> Self { + TableAArgs { + b: None, + } + } +} +pub struct TableABuilder<'a: 'b, 'b> { + fbb_: &'b mut flatbuffers::FlatBufferBuilder<'a>, + start_: flatbuffers::WIPOffset, +} +impl<'a: 'b, 'b> TableABuilder<'a, 'b> { + #[inline] + pub fn add_b(&mut self, b: flatbuffers::WIPOffset>) { + self.fbb_.push_slot_always::>(TableA::VT_B, b); + } + #[inline] + pub fn new(_fbb: &'b mut flatbuffers::FlatBufferBuilder<'a>) -> TableABuilder<'a, 'b> { + let start = _fbb.start_table(); + TableABuilder { + fbb_: _fbb, + start_: start, + } + } + #[inline] + pub fn finish(self) -> flatbuffers::WIPOffset> { + let o = self.fbb_.end_table(self.start_); + flatbuffers::WIPOffset::new(o.value()) + } +} + diff --git a/tests/include_test/sub/include_test2_generated.rs b/tests/include_test/sub/include_test2_generated.rs new file mode 100644 index 000000000..eee54a932 --- /dev/null +++ b/tests/include_test/sub/include_test2_generated.rs @@ -0,0 +1,222 @@ +// automatically generated by the FlatBuffers compiler, do not modify + + + +use crate::include_test1_generated::*; +use std::mem; +use std::cmp::Ordering; + +extern crate flatbuffers; +use self::flatbuffers::EndianScalar; + +#[allow(unused_imports, dead_code)] +pub mod my_game { + + use crate::include_test1_generated::*; + use std::mem; + use std::cmp::Ordering; + + extern crate flatbuffers; + use self::flatbuffers::EndianScalar; +#[allow(unused_imports, dead_code)] +pub mod other_name_space { + + use crate::include_test1_generated::*; + use std::mem; + use std::cmp::Ordering; + + extern crate flatbuffers; + use self::flatbuffers::EndianScalar; + +#[allow(non_camel_case_types)] +#[repr(i64)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub enum FromInclude { + IncludeVal = 0, + +} + +pub const ENUM_MIN_FROM_INCLUDE: i64 = 0; +pub const ENUM_MAX_FROM_INCLUDE: i64 = 0; + +impl<'a> flatbuffers::Follow<'a> for FromInclude { + type Inner = Self; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + flatbuffers::read_scalar_at::(buf, loc) + } +} + +impl flatbuffers::EndianScalar for FromInclude { + #[inline] + fn to_little_endian(self) -> Self { + let n = i64::to_le(self as i64); + let p = &n as *const i64 as *const FromInclude; + unsafe { *p } + } + #[inline] + fn from_little_endian(self) -> Self { + let n = i64::from_le(self as i64); + let p = &n as *const i64 as *const FromInclude; + unsafe { *p } + } +} + +impl flatbuffers::Push for FromInclude { + type Output = FromInclude; + #[inline] + fn push(&self, dst: &mut [u8], _rest: &[u8]) { + flatbuffers::emplace_scalar::(dst, *self); + } +} + +#[allow(non_camel_case_types)] +pub const ENUM_VALUES_FROM_INCLUDE:[FromInclude; 1] = [ + FromInclude::IncludeVal +]; + +#[allow(non_camel_case_types)] +pub const ENUM_NAMES_FROM_INCLUDE:[&'static str; 1] = [ + "IncludeVal" +]; + +pub fn enum_name_from_include(e: FromInclude) -> &'static str { + let index = e as i64; + ENUM_NAMES_FROM_INCLUDE[index as usize] +} + +// struct Unused, aligned to 4 +#[repr(C, align(4))] +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Unused { + a_: i32, +} // pub struct Unused +impl flatbuffers::SafeSliceAccess for Unused {} +impl<'a> flatbuffers::Follow<'a> for Unused { + type Inner = &'a Unused; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + <&'a Unused>::follow(buf, loc) + } +} +impl<'a> flatbuffers::Follow<'a> for &'a Unused { + type Inner = &'a Unused; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + flatbuffers::follow_cast_ref::(buf, loc) + } +} +impl<'b> flatbuffers::Push for Unused { + type Output = Unused; + #[inline] + fn push(&self, dst: &mut [u8], _rest: &[u8]) { + let src = unsafe { + ::std::slice::from_raw_parts(self as *const Unused as *const u8, Self::size()) + }; + dst.copy_from_slice(src); + } +} +impl<'b> flatbuffers::Push for &'b Unused { + type Output = Unused; + + #[inline] + fn push(&self, dst: &mut [u8], _rest: &[u8]) { + let src = unsafe { + ::std::slice::from_raw_parts(*self as *const Unused as *const u8, Self::size()) + }; + dst.copy_from_slice(src); + } +} + + +impl Unused { + pub fn new<'a>(_a: i32) -> Self { + Unused { + a_: _a.to_little_endian(), + + } + } + pub fn a<'a>(&'a self) -> i32 { + self.a_.from_little_endian() + } +} + +pub enum TableBOffset {} +#[derive(Copy, Clone, Debug, PartialEq)] + +pub struct TableB<'a> { + pub _tab: flatbuffers::Table<'a>, +} + +impl<'a> flatbuffers::Follow<'a> for TableB<'a> { + type Inner = TableB<'a>; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + Self { + _tab: flatbuffers::Table { buf: buf, loc: loc }, + } + } +} + +impl<'a> TableB<'a> { + #[inline] + pub fn init_from_table(table: flatbuffers::Table<'a>) -> Self { + TableB { + _tab: table, + } + } + #[allow(unused_mut)] + pub fn create<'bldr: 'args, 'args: 'mut_bldr, 'mut_bldr>( + _fbb: &'mut_bldr mut flatbuffers::FlatBufferBuilder<'bldr>, + args: &'args TableBArgs<'args>) -> flatbuffers::WIPOffset> { + let mut builder = TableBBuilder::new(_fbb); + if let Some(x) = args.a { builder.add_a(x); } + builder.finish() + } + + pub const VT_A: flatbuffers::VOffsetT = 4; + + #[inline] + pub fn a(&self) -> Option> { + self._tab.get::>>(TableB::VT_A, None) + } +} + +pub struct TableBArgs<'a> { + pub a: Option>>, +} +impl<'a> Default for TableBArgs<'a> { + #[inline] + fn default() -> Self { + TableBArgs { + a: None, + } + } +} +pub struct TableBBuilder<'a: 'b, 'b> { + fbb_: &'b mut flatbuffers::FlatBufferBuilder<'a>, + start_: flatbuffers::WIPOffset, +} +impl<'a: 'b, 'b> TableBBuilder<'a, 'b> { + #[inline] + pub fn add_a(&mut self, a: flatbuffers::WIPOffset>) { + self.fbb_.push_slot_always::>(TableB::VT_A, a); + } + #[inline] + pub fn new(_fbb: &'b mut flatbuffers::FlatBufferBuilder<'a>) -> TableBBuilder<'a, 'b> { + let start = _fbb.start_table(); + TableBBuilder { + fbb_: _fbb, + start_: start, + } + } + #[inline] + pub fn finish(self) -> flatbuffers::WIPOffset> { + let o = self.fbb_.end_table(self.start_); + flatbuffers::WIPOffset::new(o.value()) + } +} + +} // pub mod OtherNameSpace +} // pub mod MyGame + diff --git a/tests/monster_test_generated.rs b/tests/monster_test_generated.rs index a05073107..22240b960 100644 --- a/tests/monster_test_generated.rs +++ b/tests/monster_test_generated.rs @@ -2,6 +2,8 @@ +use crate::include_test1_generated::*; +use crate::include_test2_generated::*; use std::mem; use std::cmp::Ordering; @@ -11,6 +13,8 @@ use self::flatbuffers::EndianScalar; #[allow(unused_imports, dead_code)] pub mod my_game { + use crate::include_test1_generated::*; + use crate::include_test2_generated::*; use std::mem; use std::cmp::Ordering; @@ -83,6 +87,8 @@ impl<'a: 'b, 'b> InParentNamespaceBuilder<'a, 'b> { #[allow(unused_imports, dead_code)] pub mod example_2 { + use crate::include_test1_generated::*; + use crate::include_test2_generated::*; use std::mem; use std::cmp::Ordering; @@ -157,6 +163,8 @@ impl<'a: 'b, 'b> MonsterBuilder<'a, 'b> { #[allow(unused_imports, dead_code)] pub mod example { + use crate::include_test1_generated::*; + use crate::include_test2_generated::*; use std::mem; use std::cmp::Ordering; diff --git a/tests/namespace_test/namespace_test2_generated.rs b/tests/namespace_test/namespace_test2_generated.rs index 3c04c0f03..cd0a75ae9 100644 --- a/tests/namespace_test/namespace_test2_generated.rs +++ b/tests/namespace_test/namespace_test2_generated.rs @@ -2,6 +2,7 @@ +use crate::namespace_test1_generated::*; use std::mem; use std::cmp::Ordering; @@ -11,6 +12,7 @@ use self::flatbuffers::EndianScalar; #[allow(unused_imports, dead_code)] pub mod namespace_a { + use crate::namespace_test1_generated::*; use std::mem; use std::cmp::Ordering; @@ -198,6 +200,7 @@ impl<'a: 'b, 'b> SecondTableInABuilder<'a, 'b> { #[allow(unused_imports, dead_code)] pub mod namespace_c { + use crate::namespace_test1_generated::*; use std::mem; use std::cmp::Ordering; diff --git a/tests/rust_usage_test/benches/flatbuffers_benchmarks.rs b/tests/rust_usage_test/benches/flatbuffers_benchmarks.rs index 2c6be1f30..d38b8b4b8 100644 --- a/tests/rust_usage_test/benches/flatbuffers_benchmarks.rs +++ b/tests/rust_usage_test/benches/flatbuffers_benchmarks.rs @@ -20,6 +20,14 @@ use bencher::Bencher; extern crate flatbuffers; +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/include_test1_generated.rs"] +pub mod include_test1_generated; + +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/sub/include_test2_generated.rs"] +pub mod include_test2_generated; + #[allow(dead_code, unused_imports)] #[path = "../../monster_test_generated.rs"] mod monster_test_generated; diff --git a/tests/rust_usage_test/bin/alloc_check.rs b/tests/rust_usage_test/bin/alloc_check.rs index e7ec9e5bc..26f38e3aa 100644 --- a/tests/rust_usage_test/bin/alloc_check.rs +++ b/tests/rust_usage_test/bin/alloc_check.rs @@ -28,6 +28,14 @@ static A: TrackingAllocator = TrackingAllocator; // import the flatbuffers generated code: extern crate flatbuffers; +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/include_test1_generated.rs"] +pub mod include_test1_generated; + +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/sub/include_test2_generated.rs"] +pub mod include_test2_generated; + #[allow(dead_code, unused_imports)] #[path = "../../monster_test_generated.rs"] mod monster_test_generated; diff --git a/tests/rust_usage_test/bin/monster_example.rs b/tests/rust_usage_test/bin/monster_example.rs index 3c9a0a0e6..fe172db1f 100644 --- a/tests/rust_usage_test/bin/monster_example.rs +++ b/tests/rust_usage_test/bin/monster_example.rs @@ -1,10 +1,19 @@ extern crate flatbuffers; +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/include_test1_generated.rs"] +pub mod include_test1_generated; + +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/sub/include_test2_generated.rs"] +pub mod include_test2_generated; + #[allow(dead_code, unused_imports)] #[path = "../../monster_test_generated.rs"] mod monster_test_generated; pub use monster_test_generated::my_game; + use std::io::Read; fn main() { diff --git a/tests/rust_usage_test/tests/integration_test.rs b/tests/rust_usage_test/tests/integration_test.rs index d5b4f53ed..cc3a2ee08 100644 --- a/tests/rust_usage_test/tests/integration_test.rs +++ b/tests/rust_usage_test/tests/integration_test.rs @@ -19,6 +19,14 @@ extern crate quickcheck; extern crate flatbuffers; +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/include_test1_generated.rs"] +pub mod include_test1_generated; + +#[allow(dead_code, unused_imports)] +#[path = "../../include_test/sub/include_test2_generated.rs"] +pub mod include_test2_generated; + #[allow(dead_code, unused_imports)] #[path = "../../monster_test_generated.rs"] mod monster_test_generated;