Skip to content

🐮 Disable implicit function declaration error on clang-16 and up #43

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jasper-Bekkers
Copy link

Starting with clang-16 GKlib fails to compile with an implicit function declaration error. Just disable the error for it as it's most friendly to library users.

@MarijnS95
Copy link

This happens specifically when (cross?-)compiling to x86_64-pc-windows-msvc:

> cargo b --target x86_64-pc-windows-msvc
warning: [email protected]: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(63,18): error: call to undeclared function 'read'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
warning: [email protected]:    63 |     if ((rsize = read(fd, buf, tsize)) == -1)
warning: [email protected]:       |                  ^
...
warning: [email protected]: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(84,17): error: call to undeclared function 'write'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
warning: [email protected]:    84 |     if ((size = write(fd, buf, tsize)) == -1)
warning: [email protected]:       |                 ^

These functions are pretty much POSIX which Windows isn't expected to provide, but they do have it:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-read
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read

They don't specify where read() comes from, but _read() comes from <io.h>. Adding this manually fixes the error, and I think upstream metis should do just that. They could've defined these manually or imported <process.h> for getpid() too when doing KarypisLab/GKlib@a8e7e25.

MarijnS95 added a commit to MarijnS95/GKlib that referenced this pull request May 22, 2025
…h Clang

When (cross?) compiling this crate to Windows (in a Rust project) using
Clang 16 or newer, we're seeing these declaration errors:

    > cargo b --target x86_64-pc-windows-msvc
    warning: [email protected]: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(63,18): error: call to undeclared function 'read'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    warning: [email protected]:    63 |     if ((rsize = read(fd, buf, tsize)) == -1)
    warning: [email protected]:       |                  ^
    ...
    warning: [email protected]: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(84,17): error: call to undeclared function 'write'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    warning: [email protected]:    84 |     if ((size = write(fd, buf, tsize)) == -1)
    warning: [email protected]:       |                 ^

(LIHPC-Computational-Geometry/metis-rs#43 (comment))

Now it's yet unknown (we haven't checked) if older Clang had these
declarations in a header, or didn't treat this warning as error yet,
but the functions are POSIX which Windows isn't required to implement.
Fortunately they provide it but with a deprecation warning and a
recommended replacement of `_read()` (and `_write()`), but for this
`<io.h>` needs to be included:

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-read
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read

The same is true for `getpid()` that commit a8e7e25 ("port to vs2017")
used to provide a `win32/adapt.c/h` workaround for, but this can instead
be pulled from `<process.h>` (equally with a deprecation warning, but
we should probably address these all in a consistent manner instead
of defining a workaround).  Let's take this as a starting point and go
from there.
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 this pull request may close these issues.

2 participants