[Rust] Missing 'use' statements when generating from .fbs that has imports

[andygrove]

Flatbuffers v1.11.0
Ubuntu 19.04

When generating code from multiple schema files, the necessary Rust use statements are not generated, therefore the generated code does not compile.

For example, when generating code for Apache Arrow Tensor.fbs, which includes Schema.fbs, I get the following compilation error:

error[E0412]: cannot find type `Type` in this scope
   --> arrow/src/ipc/gen/Tensor_generated.rs:443:27
    |
443 |     self.fbb_.push_slot::<Type>(Tensor::VT_TYPE_TYPE, type_type, Type::NONE);
    |                           ^^^^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
    |
38  |   use crate::ipc::gen::Schema_generated::org::apache::arrow::flatbuf::Type;

The workaround is for me to manually add use crate::ipc::gen::Schema_generated::org::apache::arrow::flatbuf::Type to the generated code.

[andygrove]

@rw FYI this is one of the issues I ran into

[jcrevier]

I ran into this as well. I was able to work around it using the --gen-all option, and properly specifying the import path. This puts things into a single generated file, which was acceptable for my case. Strangely, it doesn't work if you specify both --gen-onefile and --gen-all. Some better documentation on how imports are supposed to work with the rust generator would be nice.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

[plwalsh]

Any plans of addressing this one?

[CasperN]

I don't think anyone is currently working on it.

Also, this could be a duplicate of #6063

[plwalsh]

My workaround was to manually combine all my schemas into one giant file. Not at all ideal, and a maintenance nightmare, but it at least kept me moving forward.

[CasperN]

There's some incompatibility between how C++ and Rust namespaces work that were probably not addressed in the code generator. Specifically, in C++, every file can unilaterally decide to contribute to the any namespace; while in Rust, each file is in its own namespace, named after the filename.

I have to think a bit how to best structure this on the flatc side, but --gen-all seems like a good idea. @plwalsh Did that not work for your use case?

[CasperN]

@dvtomas was affected by #6063, which I'm considering a duplicate of this problem.

[dvtomas]

--gen-all is not an option for me. I need multiple root_types (one for my Request table one for my Response table), thus multiple generated files. However, those have to share common functionality (other .fbs/*generated.rs). In the end I've had to remove all namespaces altogether, which is kind of a bummer, but I don't see any other option.

[CasperN]

I need multiple root_types (one for my Request table one for my Response table

If you're referring to the get_root_as_request function, you can use flatbuffers::get_root::<Request> too.

[jcrevier]

For what its worth, as far as I can tell --gen-all stopped working as a work around for this in v1.12 unfortunately. So at the moment, there's not really a good way to solve this with the latest flatbuffers release. I'd love to see this issue get some attention as the issues around namespaces have been somewhat of a pain point for using flatbuffers in Rust.

[CasperN]

So I think one way to emulate C style namespaces is to have a special new file, say generated.rs, who's job is to import from all other generated_files in a way that correctly models the C++ style namespaces. We can also update the generated code to use fully qualified types relative to that file.

#[allow(unused_imports)]
mod generated {
    // Construct Flatbuffers namespace.
    pub mod ns1 {
        pub use crate::generated::file1_generated::ns1::*;
        pub use crate::generated::file2_generated::ns1::*;
        pub use crate::generated::file3_generated::ns1::*;

        pub mod ns2 {
            pub use crate::generated::file1_generated::ns1::ns2::*;
            // file2_generated omitted because it doesn't have ns2.
            pub use crate::generated::file3_generated::ns1::ns2::*;
        }
    }
    // In practice the generated files won't be inlined, these would be separate files.
    mod file1_generated {
        pub mod ns1 {
            pub mod ns2 {
                pub struct Foo;
            }
        }
    }
    mod file2_generated {
        pub mod ns1 {
            pub struct Bar(crate::generated::ns1::ns2::Foo);
        }
    }
    mod file3_generated {
        pub mod ns1 {
            pub mod ns2 {
                pub struct Baz(
                    crate::generated::ns1::ns2::Foo,
                    crate::generated::ns1::Bar);
            }
        }
    }
}

Problems / Questions:

• Obviously generate.rs is arbitrary, we can pass it in different names via flag.
• We need to know where generate.rs lives for file1_generated.rs to import properly. I think at the crate root is an okay assumption.
• We need to know where all the generated files live to import from them in generate.rs.
  • Should we generate the mod file1_generated; declaration in generate.rs with a #[path=] specifier? This would assume people don' move their generated files...
  • Where do people like to keep their schemas and generated files? If we can't assume/enforce a pattern;
  • Can we assume the schema-writers to declare mod file1_generated; at the crate root, or some other known location?
• I think it'll be pretty hard to modify the parser to do this... @aardappel thoughts?
• Is there an easier way?

[CasperN]

@aardappel can you LGTM the high level changes to flatc.cpp idea before I actually go do it?

1. I'll add two flags, '--combined-imports' and '--generated-files-unmoved':

    struct CombinedImports {
        // if true, flatc.cpp will build the `namespaces` map call `generate_combined_imports`
        // on the code generators after all files are processed
        bool combined_imports;
   
        // If true, assume schema-writers won't move the generated files.
        // For rust this enables generating module declarations in the combined
        // imports file. Otherwise, schema writers must declare modules in the crate root.
        bool generated_files_unmoved;
   
        // Map from schemafile to namespaces therein
        std::map<std::string, std::vector<Namespace>> namespaces;
    };
    class CodeGenerators {
      public:
        // Implementors can output a single file that imports from all the generated files. 
        virtual bool generate_combined_imports(CombinedImports& combined_imports) {
           return true;
        }
    };

2. flatc.cpp will construct CombinedImports while files are being processed and invoke generate_combined_imports afterwards.

3. Then I'll fix this issue's problem in Rust.

[aardappel]

Hmm, having this generated.rs seems less than ideal, because it makes things less modular, both in terms of generated files and how they are generated.

I'd be tempted to go for a simpler solution, that if a schema is being parsed while --rust is on and --gen-all is NOT on, and it "reopens" a namespace, that it gives an error to explain the situation. The user can then choose to write Rust conformant namespaces (if they have control over this) or use --gen-all.

[CasperN]

Hmm, having this generated.rs seems less than ideal, because it makes things less modular, both in terms of generated files and how they are generated.

Its definitely a little complex, but I think its the best way to model C++ style namespaces in Rust. It also kind of matches the structure of many rust crates where the root file of the crate, lib.rs imports symbols from some internal module structure into the root module to present a flat public API surface; except here we're not presenting a flat API surface but a different tree structure based on flatbuffers namespaces.

I'd be tempted to go for a simpler solution, that if a schema is being parsed while --rust is on and --gen-all is NOT on, and it "reopens" a namespace, that it gives an error to explain the situation.

I'd rather not do that because it'd make it harder for a C++ based project to start using Rust, lol.

[aardappel]

It also kind of matches the structure of many rust crates where the root file of the crate, lib.rs imports symbols

Maybe it should be actually called lib.rs then?

I'd rather not do that because it'd make it harder for a C++ based project to start using Rust

It wouldn't be, it just makes it obvious that in these case you need to use --gen-all.

You could even invert it, make --gen-all the default for Rust, and have a flag to undo it if a project is written with Rust in mind. But I kinda like having the --gen-all to remind people what is going on.

I am fine with any of these solutions in the end.

[aardappel]

@rw @krojew may have opinions?

[krojew]

I actually ran into multiple problems with how generated rust needs to be imported to run. In one project I actually have the same code imported twice with types doubled, because it wouldn't work otherwise. Therefore I'm all for a file grouping everything in one place and acting as an entrypoint. I'm even for generating a whole lib crate, if this would make things easier.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

[rw]

Keep this open

[CasperN]

Ok, I finally made time to try a solution. My current plan is to go the way of Go and Csharp and generate types in their own files, which is simpler than what I suggested in #5589 (comment) since it doesn't include the .fbs filename.

[CasperN]

I have something of a solution at #6731, which unfortunately will be a bit of a breaking change (see the PR for details). Please take a look and comment!

[CasperN]

Ok, I'm closing this now that #6731 is merged. It will be a while before we next release things but if one of you uses flatbuffers at HEAD, do give it a go!