Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Use C++11 std::isnan and std::isfinite, and don't include an unneeded header on linux. #294

Closed
wants to merge 2 commits into from

Conversation

leper
Copy link
Contributor

@leper leper commented Jan 16, 2019

This allows building against musl libc.

Not only is sysctl() not used on this platform, but musl libc does not have the header.
@castano
Copy link
Owner

castano commented Nov 24, 2019

I would prefer to not add dependencies on additional C++ headers. Is that necessary to handle any particular platform? Removing sysctl if not used seems like a good idea, though.

@leper
Copy link
Contributor Author

leper commented Nov 24, 2019

You already pull in <cmath> via src/nvtt/squish/maths.h in a few places, also <cmath> is in some ways just a C++/namespace std wrapper around C's <math.h>. And C99 and C++11 standardize isnan/isfinite, but requiring C99 in a C++ project is usually not something you want to do, especially on Windows where C is only getting C99 additions due to C++11.

The code would actually remove the other isnan and isfinite checks if C++11 could be used as a minimum required standard for building the library. But I do not know what platforms you actually care to support or what the minimum requirements for this library are.

This change is required to build on Linux with musl libc (e.g. Alpine Linux, Void Linux). The change itself has been tested by CI for a bundled copy patched with this on multiple platforms all using C++11 (or later) including Windows, Linux+glibc, Linux+musl, FreeBSD, macOS (which does need #270 to build).

@voroskoi voroskoi mentioned this pull request Feb 20, 2020
@castano castano closed this Aug 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants