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.

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

stage1, stage2: rename c_void to anyopaque #10316

Merged
merged 2 commits into from
Dec 19, 2021
Merged

stage1, stage2: rename c_void to anyopaque #10316

merged 2 commits into from
Dec 19, 2021

Conversation

ifreund
Copy link
Member

@ifreund ifreund commented Dec 12, 2021 •
edited
Loading

zig fmt now replaces c_void with anyopaque to make updating
code easy.

Closes #323

@ifreund
stage1, stage2: rename c_void to anyopaque
2fe209c 
zig fmt now replaces c_void with anyopaque to make updating
code easy.
@ifreund
Copy link
Member Author

ifreund commented Dec 12, 2021

The force push just fixed a typo in the commit message.

I had a full zig build test run pass for me locally so hopefully CI will be green when I wake up in the morning :)

@ifreund
Copy link
Member Author

ifreund commented Dec 12, 2021

Looks like the only CI failure was the windows build which timed out after 6 hours. Seems like it did make it to the last std test target though (std-native-ReleaseSmall-bare-single).

andrewrk
andrewrk requested changes Dec 13, 2021
View reviewed changes
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

Found a problem with the zig fmt auto fixer; other than that this looks ready to merge.

lib/std/zig/render.zig Outdated Show resolved Hide resolved
@ifreund
zig fmt: improve c_void -> anyopaque rewrite
8530f8b 
This now only rewrite identifiers, not e.g. field names or block labels,
which is more correct.
@ifreund
Copy link
Member Author

ifreund commented Dec 13, 2021 •
edited
Loading

Ironically, I think anytype would make a bit more sense for this primitive type than anyopaque does, if it wasn't already a keyword.

IMO we should replace the anytype keyword with ducktype (#5893 (comment)) and replace c_void with anytype instead of anyopaque.

Edit: and maybe consider waiting until after 0.9.0 so we don't rush this.

@ifreund ifreund marked this pull request as draft December 13, 2021 23:31
@andrewrk
Copy link
Member

I'm going to pull this in because it is an improvement over status quo in that it makes the type no longer imply that it has to do with C or void, and given that zig fmt auto-fixes it, it's pretty harmless as far as breaking changes go.

We can evaluate follow-up proposals separately.

@andrewrk andrewrk marked this pull request as ready for review December 19, 2021 05:24
@andrewrk andrewrk merged commit 9f9f215 into ziglang:master Dec 19, 2021
@joachimschmidt557 joachimschmidt557 mentioned this pull request Dec 19, 2021
Merged
@ifreund ifreund deleted the anyopaque branch December 19, 2021 15:52
@paulsnar paulsnar mentioned this pull request Dec 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewrk andrewrk Awaiting requested review from andrewrk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

proposal: rename c_void to anyopaque
2 participants
@ifreund @andrewrk