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

Storing Layout for Reflect types #17498

Open
makspll opened this issue Jan 22, 2025 · 1 comment
Open

Storing Layout for Reflect types #17498

makspll opened this issue Jan 22, 2025 · 1 comment
Labels
A-Reflection Runtime information about types C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@makspll
Copy link
Contributor

makspll commented Jan 22, 2025

What problem does this solve or what need does it fill?

It allows for lower level manipulation, working with custom allocators in the context of dynamic types etc.

What solution would you like?

Something along the lines of:

Reflect::layout(dyn_type) -> Layout

Or:

let layout: Layout = dyn_type.get_represented_type_info().unwrap().layout()

Maybe Optional<Layout> instead if this is too restrictive

What alternative(s) have you considered?

  • Storing a custom TypeData but it requires registering it for all foreign types as well, not really tractable
  • Adding a Sized bound to Reflect but not sure if that's possible

Additional context

N/A

@makspll makspll added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 22, 2025
@BenjaminBrienen BenjaminBrienen added A-Reflection Runtime information about types S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed S-Needs-Triage This issue needs to be labelled labels Jan 23, 2025
@MrGVSV
Copy link
Member

MrGVSV commented Jan 23, 2025

Requiring Sized isn't ideal (not that we reflect any unsized types currently). So we'd probably want to use something like Layout::for_value, though I’m not sure if that's different from Layout::new.

And yeah Option<Layout> may be best for now. I don't know how well it will play with runtime-defined types once we add better support for those, so it's probably safer to make it optional for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

3 participants