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

Refactor String header layout reflection #13335

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Apr 17, 2023

This patch removes the assumption of Strings memory layout in stdlib when allocating an instance by hand.
The exact memory layout is governed by the compiler (or rather the LLVM backend takes care of the specifics). The assumed layout is probably correct most of the time, but it's not guaranteed.
And it's easy to implement in a way that does not assume anything about the memory layout except that the char array is the last field.
This action decouples the stdlib implementation from compiler internals.

It also paves a way for potential future changes to Strings layout, for example as suggested in #12020.

@straight-shoota straight-shoota added this to the 1.9.0 milestone Apr 17, 2023
@straight-shoota
Copy link
Member Author

I added a commit that re-uses Object.set_crystal_type_id instead of a custom implementation.
The bug fixed in #13338 does not have any effect (same as in WeakRef.allocate) because the first four bytes are overridden by the bytesize afterwards.

@straight-shoota straight-shoota removed this from the 1.9.0 milestone Apr 18, 2023
@straight-shoota straight-shoota modified the milestones: 1.8.1, 1.9.0 Apr 18, 2023
@straight-shoota straight-shoota merged commit 061dc10 into crystal-lang:master Apr 20, 2023
@straight-shoota straight-shoota deleted the refactor/string-header-layout branch April 20, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants