Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Use a custom optional instead of std::optional for non-scalar type #359

Open
SirusDoma opened this issue Dec 28, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@SirusDoma
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently bebop use concrete type when generating a field with non-scalar types. This make things simple since everything is value-based, but prevents user to make cyclic dependency which gracefully prevented in #111.

Describe the solution you'd like
I would like to suggest to revise and support cyclic dependency instead. Here what I had in mind:

  1. Define a custom std::optional implementation, but instead of use value type, use heap (e.g smart pointer). This is relatively simple to do.
  2. Determine definitions that are cyclic in parser, and then in the generator, forward declare them before generating the actual definitions.
  3. The custom optional will incur extra cost in runtime; typically it use 4-8 bytes (the std::optional use extra 1 byte only) and it loss cache locality. Thus, this custom optional should only be used only for non-scalar type where the size is trivial

Describe alternatives you've considered
The easiest way (and relatively safe) to do this is to expose smart pointer for non-scalar type. But this would be a breaking change to the user.

Additional context
N/A

@SirusDoma SirusDoma added the enhancement New feature or request label Dec 28, 2024
@SirusDoma
Copy link
Contributor Author

For reference, Protobuf use pointer in the backing field, capnproto use it too (although I'm not sure how exactly it is implemented in capnproto). Both seems to allow user to have cyclic dependency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant