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

chore: use RawPtr::Env to signal raw pointers for environments #1203

Closed
wants to merge 1 commit into from

Conversation

arthurpaulino
Copy link
Contributor

It's safer to use a new variant RawPtr::Env to signal raw pointers for environments than relying solely on tag checks. This would probably have avoided the bug introduced in #1183, whose fix was implemented in #1200

@arthurpaulino arthurpaulino requested review from a team as code owners March 7, 2024 15:58
@arthurpaulino arthurpaulino force-pushed the ap/raw-ptr-env branch 2 times, most recently from f1be69f to 94c18a8 Compare March 7, 2024 17:57
It's safer to use a new variant `RawPtr::Env` to signal raw pointers
for environments than relying solely on tag checks. This would
probably have avoided the bug introduced in #1183, whose fix was
implemented in #1200
Copy link
Contributor

@gabriel-barrett gabriel-barrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the concern, but this is mixing the concepts of the raw pointer and the pointer. The raw pointer is supposed to only give information on the number of children it has. The pointer, then, gives the layout, with the tag. I think that the correct approach is actually restructuring the zstore, since we do not assume all children are always pointers anymore. This is why I'm blocking this PR for now

@arthurpaulino
Copy link
Contributor Author

Closing in favor of #1221

@arthurpaulino arthurpaulino deleted the ap/raw-ptr-env branch March 18, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants