lib/c: implement various string functions #25238
Conversation
There was a problem hiding this comment.
Thanks for working on this - it looks like a good start.
Moving forward, I think this subproject would benefit with a short design document (in the form of a .md file for now) with the following things outlined:
- ABI compatibility strategy (including the
c_charsituation which several people have gone back and forth on). Clarify the decisions made by this implementation. - Clarify conditional compilation stance. Which functions should be exported, and which functions are conditionally excluded, and the reasoning behind it.
- Testing strategy. There are already existing libc test suites that can be used to test for conformance. We should get that going already right now, and see where we're at, and then use it to help evaluate contributions faster.
That could be a nice task for @alexrp or a particularly ambitious contributor.
| // Functions specific to musl and wasi-libc. | ||
| @export(&strcmp, .{ .name = "strcmp", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strncmp, .{ .name = "strncmp", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strcasecmp, .{ .name = "strcasecmp", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strncasecmp, .{ .name = "strncasecmp", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&__strcasecmp_l, .{ .name = "__strcasecmp_l", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&__strncasecmp_l, .{ .name = "__strncasecmp_l", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&__strcasecmp_l, .{ .name = "strcasecmp_l", .linkage = .weak, .visibility = common.visibility }); | ||
| @export(&__strncasecmp_l, .{ .name = "strncasecmp_l", .linkage = .weak, .visibility = common.visibility }); | ||
| @export(&strspn, .{ .name = "strspn", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strcspn, .{ .name = "strcspn", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strpbrk, .{ .name = "strpbrk", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strstr, .{ .name = "strstr", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strtok, .{ .name = "strtok", .linkage = common.linkage, .visibility = common.visibility }); | ||
| @export(&strtok_r, .{ .name = "strtok_r", .linkage = common.linkage, .visibility = common.visibility }); |
There was a problem hiding this comment.
I don't think all these functions are specific to musl and wasi-libc.
I'm not against special casing those ABIs sometimes but I'm pretty sure most functions should more simply be directly exposed independently of target.
Is this conditional compilation solving a problem?
| if (builtin.target.isMuslLibC() or builtin.target.isWasiLibC()) { | ||
| // Functions specific to musl and wasi-libc. | ||
| @export(&bzero, .{ .name = "bzero", .linkage = common.linkage, .visibility = common.visibility }); | ||
| } |
There was a problem hiding this comment.
Again, I don't see why this should be a conditional export.
| } | ||
|
|
||
| fn strcmp(s1: [*:0]const c_char, s2: [*:0]const c_char) callconv(.c) c_int { | ||
| fn strcmp(s1: [*:0]const u8, s2: [*:0]const u8) callconv(.c) c_int { |
There was a problem hiding this comment.
The previous version uses c_char quite intentionally, since it has ABI implications. While pointer types have the same ABI independently of element type, this could have an effect on generated header files for example. Such a change should not be unadorned with an explanation.
Added
str*functions not included in #23847:strspn,strcspn,strpbrk,strstr,strtok,strtok_rRelated: #2879