Skip to content

Conversation

@fawdlstty
Copy link
Contributor

@fawdlstty fawdlstty commented Nov 25, 2025

enumeration types generated by flatc. This enables users to freely use the objects generated by flatc, and after being serialized into various formats, they can also be deserialized back to their original state. A more intuitive example is that the user may want to store the objects generated by flatc as a JSON string (for storage purposes), and then restore them when needed.

@github-actions github-actions bot added c++ codegen Involving generating code from schema rust labels Nov 25, 2025
@fawdlstty
Copy link
Contributor Author

@jtdavis777 Hello, I'm sorry to disturb you. Regarding this modification, I tried running generate_code.py, and found that this script modified a large number of files. However, I don't think those are related to my modifications. Do you have any suggestions on this?

@jtdavis777
Copy link
Collaborator

hey! I just pulled master and ran the code gen script for the root directory, and had no changes. are you rebased on latest master? I would recommend going through the same steps and reporting back -- check out master and run the script, see what happens, then go to your commit and try again. let me know!

@fawdlstty
Copy link
Contributor Author

I switched to the master branch and used the flatc program that I had compiled. I successfully obtained the modified content (7 files, all related to my current modifications). I'm going to try updating now.

@fawdlstty
Copy link
Contributor Author

@jtdavis777 Hello, could you spare some time to review this PR? Thank you very much!

@jtdavis777
Copy link
Collaborator

Hello! I know effectively nothing about rust :( and do not feel confident in merging this code without having review from someone with a bit more experience and context in the flatbuffers rust implementation. I can, however, kick off the pipeline and make sure all is well there -- I do not see any tests provided which cover this new functionality, would that be something you could work on?

@fawdlstty
Copy link
Contributor Author

@jtdavis777 Thank you for your reply. I have already conducted the relevant tests.
@aardappel Hello, could you spare some time to review this PR? Thank you very much!

@jtdavis777
Copy link
Collaborator

Could you also please fill in your PR description?

@fawdlstty
Copy link
Contributor Author

Could you also please fill in your PR description?

done.

@aardappel
Copy link
Collaborator

Don't know Rust either, but this seems mostly fine.. surprised we have serde serialize by string, but that is apparently a problem of the existing code already.

If anything, generating error strings deep down in low level code like this seems bad design to me, it should return something from which higher level / user code can decide to report an error, and how. In this case, an unknown variant isn't even an error, it is an expected thing part of forwards/backwards compatibility so generating errors is even more inappropriate. This should probably return an Option type instead?

@fawdlstty
Copy link
Contributor Author

fawdlstty commented Nov 27, 2025

This should probably return an Option type instead?

The serialization and deserialization of the type T generated by flatc is the core of its functionality and necessary. However, implementing deserialization for Option<T> is a topic worth exploring, as it involves changing the implementation of deserialization, especially for binary types that may result in unexpected outcomes. I hope to use new issue to involve people familiar with flatbuffers and rust in the discussion

@fawdlstty
Copy link
Contributor Author

@aardappel Hello, I would like to ask if the differences in ideas regarding implementing deserialization features for Option<T> will affect the merging progress of this PR? So far, I need to use the T type that implements deserialization features, and I don't need the Option<T> type that implements deserialization features

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ codegen Involving generating code from schema rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants