Skip to content

Update std to use pointer subtraction when appropriate #20712

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Update std to use pointer subtraction when appropriate #20712

wants to merge 10 commits into from

Conversation

SeanTheGleaming
Copy link
Contributor

In many placed in std, there are a few instances where we take the results of two @intFromPtr values and subtract them to perform pointer subtraction, which is now unnecessary (and suboptimal given @intFromPtr is not usable at comptime) with the addition of pointer subtraction as a language feature in #20089. This PR aims to replace instances of @intFromPtr subtraction with pointer subtraction.

@SeanTheGleaming
Copy link
Contributor Author

I also aim to generally reduce unnecessary @intFromPtr usage or to at least use alternative methods at comptime where appropriate

@SeanTheGleaming
Copy link
Contributor Author

Closing for now as I realize that I am not well versed enough in wasm to make the zig1 changes necessary for bootstrapping zig with pointer subtraction in the standard library

@mlugg
Copy link
Member

mlugg commented Jul 21, 2024 •
edited
Loading

zig1.wasm isn't manually written! It's a WASM binary which can be generated by running zig build update-zig1. Feel free to re-open this PR and I can commit a zig1 update for you (a Zig core team member has to be the one to commit those updates for security reasons).

@SeanTheGleaming
Copy link
Contributor Author

zig1.wasm isn't manually written! It's a WASM binary which can be generated by running zig build update-zig1. Feel free to re-open this PR and I can commit a zig1 update for you (a Zig core team member has to be the one to commit those updates for security reasons).

Thank you, that would be perfect. Sorry for the misunderstanding

@SeanTheGleaming @Vexu
Update hash_map.zig
51f3c09 
Updated `removeByPtr` to use pointer subtraction
@SeanTheGleaming @Vexu
Update hash_map.zig
56df6da 
Make pointer subtraction less ambiguous in `removeByPtr` by casting the left hand side to a multi item pointer
@SeanTheGleaming @Vexu
Update hash_map.zig
091ec1b 
Reverted previous commit which casted the left hand side of the pointer subtraction in `removeByPtr` to a multi item pointer
@SeanTheGleaming @Vexu
Update heap.zig
15c4516 
Use pointer subtraction, and provide a comptime implementation of `sliceContainsPtr` and `sliceContainsSlice`, making `FixedBufferAllocator` usable in comptime
@SeanTheGleaming @Vexu
Update net.zig
00ec679 
Use pointer subtraction instead of `@intFromPtr` value subtraction when appropriate
(Incidentally eliminates all usage of `@intFromPtr` in net.zig)
@SeanTheGleaming @Vexu
Update general_purpose_allocator.zig
2c611b5 
Rework some pointer arithmetic, make use of pointer subtraction
@SeanTheGleaming @Vexu
Update arena_allocator.zig
09546e7 
Rework some pointer arithmetic, reduce usage of `@intFromPtr`
@SeanTheGleaming @Vexu
Update sbrk_allocator.zig
f06925d 
Reworked pointer logic, reducing usage of `@intFromPtr`
@SeanTheGleaming @Vexu
Update dynamic_library.zig
c093ccd 
Reworked pointer arithmetic to reduce usage of `@intFromPtr`
@SeanTheGleaming
Update heap.zig
88747d1 
Altered todo comments in `sliceContainsPtr` and `sliceContainsSlice` to be more clear about the goal of a comptime available constant time implementation
@SeanTheGleaming
Copy link
Contributor Author

This has covered pretty much all of the obvious pointer subtraction cases I could find on a cursory look of the standard library. Given that this PR is just for the cases in the standard library and not the compiler, I will be submitting this for review.

The one bit of code I've added here that I'm iffy on, is in heap.zig, where I provide a comptime implementation of sliceContainsPtr and sliceContainsSlice, making FixedBufferAllocator comptime available. Due to a lack of pointer comparison, the comptime solution had to be implemented with a loop, giving it an O(n) time complexity.

@SeanTheGleaming SeanTheGleaming marked this pull request as ready for review July 30, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SeanTheGleaming @mlugg