flatbuffers
flatbuffers copied to clipboard
rust: Introduce a trait for `get_fully_qualified_name`
This allows retrieving the name of a generic flatbuffer table type, which can be handy.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Hello, can you help explain the design choice as to why this is a trait instead of using the existing method? Do you plan on using Box<dyn flatbuffers::FullyQualifiedName>
? I can't see why that's useful
Also, the trait method should be implemented in terms of the existing method.
Hello, can you help explain the design choice as to why this is a trait instead of using the existing method? Do you plan on using
Box<dyn flatbuffers::FullyQualifiedName>
? I can't see why that's useful
I use it for things like this:
pub fn get_channel<T: FullyQualifiedName>(
&self,
name: &str,
) -> Result<&'event_loop Channel, ChannelLookupError> {
self.get_raw_channel(name, T::get_fully_qualified_name())
}
It's part of an IPC mechanism. It has a concept of "channels", which have a specific flatbuffer table type specified in a config file. Verifying the tables is redundant as long as both the sender and receiver are using the same type: the sender produces a valid table, and the receiver uses it. This API provides that safety by retrieving the name of the table generically, which needs a trait.
Also, the trait method should be implemented in terms of the existing method.
Done
Gotcha, so as it is right now, I'm not convinced this is useful enough to change the code generator for all our users. However, I see the value of a larger reflection stack that this could be a part of. WDYT?
Gotcha, so as it is right now, I'm not convinced this is useful enough to change the code generator for all our users. However, I see the value of a larger reflection stack that this could be a part of. WDYT?
Up to you, I don't mind carrying small patches like this. I have a C++-based project that I'm adding Rust wrappers to, and so far no need to do reflection in Rust because all of the infrastructure that does complicated things like that is C++. Being able to link the generic Rust code with templated C++ code via assertions on the type names provides a convenient escape hatch for things like that.
If it makes a difference, this only affects generated code with --gen-name-strings
enabled.
Another consideration: I do the same thing with C++ templates, which don't require any explicit unification of the concept across types. I think having a trait for Rust is necessary to have feature parity with the equivalent C++ APIs.
@CasperN @bsilver8192 Status on this one too?
I think we should think of a better design for reflection and perhaps generalize storage in the Table
struct itself.
I think we should think of a better design for reflection and perhaps generalize storage in the
Table
struct itself.
I'm not sure that applies here? I think a trait for this makes sense regardless of how the rest of reflection works, because it needs to be a separate trait that isn't used when the flatc option is turned off.
Following up on this again, where is this at?