Skip to content

Extract bindings for LibC errno to src/lib_c/#15565

Merged
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:refactor/simplify-errno
Apr 17, 2025
Merged

Extract bindings for LibC errno to src/lib_c/#15565
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:refactor/simplify-errno

Conversation

@ysbaddaden
Copy link
Collaborator

I noticed multiple times that we define the LibC errno bindings right in src/errno.cr instead of doing in the per-target src/lib_c/*/c/errno.cr bindings.

I also harmonized the POSIX funs to be aliased as LibC.__errno_location so we avoid each target having its own name. I didn't harmonize as LibC.errno because errno is supposed to be the value, not a pointer to the value.

Harmonizes the POSIX definitions to use `LibC.__errno_location` so we
avoid each target having its own naming. Doesn't harmonize as
`LibC.errno` because it's supposed to be a value, not a pointer to the
errno value, and `__errno_location` makes the pointer explicit.
@straight-shoota
Copy link
Member

straight-shoota commented Mar 17, 2025

Moving the fun declarations to the libc bindings is a no-brainer. But I'm not sure if harmonizing the alias is a good idea. It seems nice at first, cleaning up the shared interface.

However, I believe there's great value in keeping the lib bindings as close to the original as possible. Yes, we can alias lib symbols to arbitrary names in the Crystal bindings. But that should be used with care, basically only when the original name doesn't work or doesn't make sense. Inventing a fake LibC.__errno_location symbol for the purpose of harmonizing the name across systems is quite a stretch.

Btw. Zig collects clowns on this platform difference: https://github.com/ziglang/zig/blob/2a4e06bcb30f71e83b14026bcbade6aac3aece84/lib/std/c.zig#L11201-L11209 🤡

@ysbaddaden
Copy link
Collaborator Author

I'd usually agree with you. I prefer to use the actual symbols when possible, but this particular case screams to just alias this damn symbol 🙀

I can iterate the symbol names until one is defined, twice (getter and setter) if you really insist (but again, that's really a job for the alias feature).

@straight-shoota
Copy link
Member

straight-shoota commented Mar 28, 2025

I'd probably prefer to just keep the current implementation of Errno.value and .value= with platform branches. There's nothing wrong about it. It doesn't need fixing.
Yes, it may be a bit more verbose than necessary. But it's not excessive.
Ultimately, I don't find either alternative is clearly better.

I'd just move the fun declarations to lib_c/*.

@straight-shoota straight-shoota changed the title Extract per target LibC errno definitions + harmonize differences Extract per target LibC errno definitions Mar 28, 2025
@straight-shoota straight-shoota added this to the 1.17.0 milestone Apr 17, 2025
@straight-shoota straight-shoota changed the title Extract per target LibC errno definitions Extract bindings for LibC errno to src/lib_c/ Apr 17, 2025
@straight-shoota straight-shoota merged commit 890cd13 into crystal-lang:master Apr 17, 2025
32 checks passed
@ysbaddaden ysbaddaden deleted the refactor/simplify-errno branch April 17, 2025 17:52
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.

2 participants