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

Add getSentinel helper to Pointer and Array builtin.Types #21993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xdBronch
Copy link
Contributor

No description provided.

@mlugg
Copy link
Member

mlugg commented Nov 15, 2024

I think it'd make more sense for the field to be sentinel_ptr and the method sentinel().

@xdBronch
Copy link
Contributor Author

the breakings will continue until morale improves 🙃

@xdBronch
Copy link
Contributor Author

xdBronch commented Nov 15, 2024

hmm im having second thoughts on this asserting that its not null. a decent number of uses of sentinel_ptr are first checking if its null, as does std.meta.sentinel (which i didnt notice existed before :p but i think itd still be favorable to have something builtin like this). thoughts? yeah nevermind i think this is vastly more useful if it returns null

@xdBronch
Copy link
Contributor Author

so if im not wrong this requires a zig1 update since it changes builtin, correct? assuming this change is deemed worth it, is the correct order of operations something like this?
commit 1. change the field in builtin.zig and sema + any uses of it
commit 2. update zig1.wasm, later to be replaced by a team member
commit 3. add the function and update any places where its useful

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