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++][Flight] Add a base CRTP class template to serializable types #43258

Closed
felipecrv opened this issue Jul 15, 2024 · 1 comment
Closed

[C++][Flight] Add a base CRTP class template to serializable types #43258

felipecrv opened this issue Jul 15, 2024 · 1 comment

Comments

@felipecrv
Copy link
Contributor

Describe the enhancement requested

This will ensure the class interfaces on the types are consistent and layers that build on top of these can explore this regularity (like FlightSQL).

Component(s)

C++, FlightRPC

@felipecrv felipecrv self-assigned this Jul 15, 2024
felipecrv added a commit that referenced this issue Jul 19, 2024
…C calls (#43255)

### Rationale for this change

Flight should eventually be buildable without a specific protobuf version. As such, all the protobuf types are wrapped in hand-written classes. Uniformity of the interface is not enforced even though it's desirable. Extending the interface requires adding functions to every struct. A common base class can mitigates risks and reduce the amount of hand-written code.

### What changes are included in this PR?

 - Definition of a BaseType<> template (a CRTP)
 - Add constructors that aren't implicitly defined anymore
 - Having a default value for some fields that would be undefined values otherwise
 - Leverage this structure to add `SerializeToBuffer` to all the types
 - ~30KiB in binary size reduction 
 
### Are these changes tested?

By existing tests.
* GitHub Issue: #43258

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Felipe Oliveira Carvalho <[email protected]>
@felipecrv felipecrv added this to the 18.0.0 milestone Jul 19, 2024
@felipecrv
Copy link
Contributor Author

Issue resolved by pull request 43255
#43255

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

No branches or pull requests

1 participant