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.

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

cl.exe and clang-cl have no ssize_t #326

Closed
makotokato opened this issue Apr 24, 2023 · 10 comments · Fixed by #327
Closed

cl.exe and clang-cl have no ssize_t #326

makotokato opened this issue Apr 24, 2023 · 10 comments · Fixed by #327

Comments

@makotokato
Copy link
Contributor

When integrating ICU4X with Firefox, Window build cannot compile icu_capi (try build log is https://treeherder.mozilla.org/logviewer?job_id=413380887&repo=try&lineNumber=19287)

[task 2023-04-22T08:23:15.509Z] 08:23:15    ERROR -  /builds/worker/checkouts/gecko/third_party/rust/icu_capi/c/include/diplomat_runtime.h(57,24): error: unknown type name 'ssize_t'
[task 2023-04-22T08:23:15.509Z] 08:23:15     INFO -  MAKE_SLICE_VIEW(Isize, ssize_t)

When looking diplomat code, runtime.h uses ssize_t. But cl.exe (and clang-cl.exe) doesn't have ssize_t. Could you use ifdef or something workaround?

If I am something wrong, let me know.

@Manishearth
Copy link
Contributor

Do we know what types we should use here?

@Manishearth
Copy link
Contributor

It also seems like it's standard and has existed for a while so it might just need some flags

@Manishearth
Copy link
Contributor

Manishearth commented Apr 24, 2023

Ah, it's POSIX standard, not C standard.

@Manishearth
Copy link
Contributor

Apparently it should be intptr_t or ptrdiff_t

@makotokato
Copy link
Contributor Author

I think that intptr_t is better, but both is OK.

@sffc
Copy link
Contributor

sffc commented Apr 26, 2023

I've always seen just size_t

@Manishearth
Copy link
Contributor

For isize? Seems like a potential hazard and a potential ABI mismatch

@sffc
Copy link
Contributor

sffc commented Apr 26, 2023

oh I didn't see this was for signed.

@Manishearth
Copy link
Contributor

#327

@makotokato do you need an icu_capi patch release with these changes?

@Manishearth
Copy link
Contributor

icu_capi patch in unicode-org/icu4x#3401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants