Add a C API #897
Add a C API #897
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| #include <stdbool.h> | ||
| #include <stdint.h> | ||
|
|
||
| #ifdef __has_include |
There was a problem hiding this comment.
this seems to be a C23 feature, so I think it must be changed or we require C23.
https://en.cppreference.com/w/c/preprocessor/include
There was a problem hiding this comment.
this seems to be a C23 feature, so I think it must be changed or we require C23.
Formally, you are correct, but both clang and gcc have supported __has_include for a really long time.
https://godbolt.org/z/895hcr7ff
The issue is that uchar.his C11 but Apple does not have the header. I don't know why.
|
@pauldreik Making as 'ready for review'. |
pauldreik
left a comment
There was a problem hiding this comment.
I tested the amalgamate script, it failed with:
FileNotFoundError: [Errno 2] No such file or directory: '/home/pauldreik/code/delaktig/simdutf/singleheader/simdutf_c.h'
|
@lemire I took the liberty to push some clang format changes and ci job, hope you don't mind. |
|
@pauldreik I don't mind if you push stuff, but the rvv CI changes seem to fail. |
Maybe I misunderstand you. Here is what I think. If I write a function definition in C++ and I want it to be callable from C, then I put it inside an |
|
@pauldreik I pushed two changes.
|
it was not my push that caused the failure, I pushed a fix for one of the three failing riscv jobs. I have now pushed fixes for the remaining two. |
It is the declaration that needs to be in extern C. But that is not what I am after here, it is that the wrapping of extern C around the include is at best redundant and at worst wrong (since it unconditionally puts everything, including not functions, from the header file in extern C) |
this is to prove that the amalgamation as described in the README works both for C++ and C.
This reverts commit 032c633.
|
@pauldreik I did not see that extern C at first, in the test file. Sorry. I just removed it. It did not make sense, I agree. |
|
no problem! are we good to merge then? |
|
Merged ! |
Short title (summary):
Add a C API
Description
This PR adds a C API to simdutf. Amazingly, I cannot find a corresponding open issue.
Note that simdutf itself depends on the C++ library.
Purpose: This should make it easy to call simdutf from Swift, Go and friends.
To keep it simpler, I am not adding the feature macros to it... I am concerned that it will add much complexity, although it can be done later.
See #893
cc @MahdiBM
Type of change