-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
std.c: more fixes #22654
base: master
Are you sure you want to change the base?
std.c: more fixes #22654
Conversation
@@ -4817,7 +4817,7 @@ pub const AccessError = error{ | |||
/// | |||
/// On Windows, `mode` is ignored. This is a POSIX API that is only partially supported by | |||
/// Windows. See `fs` for the cross-platform file system API. | |||
pub fn access(path: []const u8, mode: u32) AccessError!void { | |||
pub fn access(path: []const u8, mode: c_int) AccessError!void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Andrew prefers to keep c_int
(& co) usage limited to std.c
. It's unclear to me where that leaves std.posix
, considering POSIX is an extension of C. cc @andrewrk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try to think of "std.posix" as a concept that is what it needs to be rather than being defined by some third party group. Everyone gets hung up on this, so I think it will have to be renamed yet again.
POSIX is irrelevant. C is irrelevant. This is an abstraction layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to be completely clear: no c_int
, yeah?
regressions found during oven-sh/bun#16862