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

Don't inline recursive datatypes, defer a runtime call to the child type instead #2759

Closed
Tracked by #2646
teh-cmc opened this issue Jul 20, 2023 · 1 comment · Fixed by #2760
Closed
Tracked by #2646

Don't inline recursive datatypes, defer a runtime call to the child type instead #2759

teh-cmc opened this issue Jul 20, 2023 · 1 comment · Fixed by #2760
Assignees
Labels
🌊 C++ API C/C++ API specific codegen/idl 🧑‍💻 dev experience developer experience (excluding CI) 🦀 Rust API Rust logging API

Comments

@teh-cmc
Copy link
Member

teh-cmc commented Jul 20, 2023

The Rust backend currently inline child datatypes into their parent, leading to giant to_arrow_datatype() that both bloat the code and are unreadable.

Replace the inlining with a runtime call to the child's to_arrow_datatype() method instead, akin to what we do on the serialization/deserialization path.

@Wumpf
Copy link
Member

Wumpf commented Jul 20, 2023

Same on Cpp backend

@Wumpf Wumpf added the 🌊 C++ API C/C++ API specific label Jul 20, 2023
Wumpf pushed a commit that referenced this issue Jul 20, 2023
Fixes #2759 

### What

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2760) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2760)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Fdatatype_no_rec/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Fdatatype_no_rec/examples)
Wumpf added a commit that referenced this issue Jul 21, 2023
* Part of #2647
* Next step after #2756

### What

* Other half of #2759
* Equivalent to #2760

Disable whitespaces for reviewing!

It compiles (`cargo codegen && ./examples/cpp/minimal/build_and_run.sh`)

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2765) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2765)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fdatatype-no-rec-cpp/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fdatatype-no-rec-cpp/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 C++ API C/C++ API specific codegen/idl 🧑‍💻 dev experience developer experience (excluding CI) 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants