Qualify C math functions

[Richard Thomson (LegalizeAdulthood)]

Use and std::isfinite/std::signbit for floating point checks in JSON and metrics code. Some C++ standard libraries do not expose these functions unqualified through the global namespace.

This fixes some build errors on BSD

[Copilot]

Pull request overview

This PR improves portability of floating-point checks by switching from C <math.h>/unqualified functions to C++ <cmath> with std::isfinite/std::signbit, aligning the JSON and metrics code with standard C++ usage and fixing build issues on some BSD toolchains.

Changes:

• Replaced <math.h> with <cmath> in metrics and JSON implementation code.
• Qualified floating-point checks with std::isfinite in metrics submission and JSON number construction.
• Updated JSON float parsing tests to use std::signbit and added <cmath> include.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/vcpkg/metrics.cpp	Uses <cmath> and std::isfinite for elapsed-time validation in metrics submission.
src/vcpkg/base/json.cpp	Uses <cmath> and std::isfinite to validate JSON number values at construction time.
src/vcpkg-test/json.cpp	Uses <cmath> and std::signbit to validate signed zero behavior in float parsing tests.

[Billy O'Neal (BillyONeal)]

This was previously litigated in #359

(See in particular the ultimate outcome #359 (comment) )

We need an understanding of why math.h is broken on these. We are intentionally trying to use the C / macro version.

[Richard Thomson (LegalizeAdulthood)]

I think you intended that link to take me to a specific comment, but it just takes me to the closed issue. The last comment on the issue says:

1. Instructions on how we can create such an environment ourselves so that we can test it,
2. A more complete understanding of the cause of the failure so that we may be able to create the conditions on a system on which we can test, or
3. Someone signing up who uses such a system who would sign up to at least let us know in a reasonable timeframe (~a week) in the event further changes are needed.

I don't know what to say about 2 or 3, but for 1 you can reproduce it on a build on BSD using a VM.

I created the action LegalizeAdulthood/vcpkg-github-cache to make caching with NuGet packages easy. While getting BSD support working, I ended up having to patch vcpkg for this scenario. You will find an example workflow of building under BSD in the ReadMe for that action.

Since vcpkg doesn't yet currently provide a vcpkg binary for BSD environments, the tool has to be bootstrapped by building from source. That's when you run into this problem. You're right to say that if you include <math.h> the non-namespace variants should be the only version available to your program and if you include <cmath> then the only version available should be the namespace qualified version. However, as we've seen many times with the way these headers are implemented the unavailable name often leaks through. In this case, it looks like the BSD environment got it even more wrong -- you included the C header, but it did some C++ compiler detection under the hood and didn't make the C form available. (This is my guess based on the symptoms.)

My action patches vcpkg-tool on BSD to get around this problem and that is the origin of this PR.

[Billy O'Neal (BillyONeal)]

In this case, it looks like the BSD environment got it even more wrong -- you included the C header, but it did some C++ compiler detection under the hood and didn't make the C form available. (This is my guess based on the symptoms.)

Emphasis mine: before "flipping the bit" on this I think we need a more concrete understanding of what's happening.

[Billy O'Neal (BillyONeal)]

(The reason I'm being such a stickler about this is that this fixes that broken BSD implementation and breaks the unknown set of happy customers who are currently working today. I don't think it's unreasonable to ask someone who is seriously trying to support one of these BSDs to describe concretely what happens on that platform by, for example, figuring out where the correct macros are supposed to come from on that platform or showing the specific condition their standard library headers are using to snip that off)

[Richard Thomson (LegalizeAdulthood)]

The reason I'm being such a stickler about this is [...]

Perfectly understandable. I agree that it's best to be conservative here and the proposed PR needs more validation beyond "it builds for me in a VM" before being accepted.

[Billy O'Neal (BillyONeal)]

Since the vcpkg team can't advance this right now I'm going to mark this as draft. Please feel free to un-draft if you find out how the broken BSD implementation is broken.

(My guess is that they're trying to use libstdc++, which mostly assumes glibc, with a different C runtime. They do an #include_next to some header which is expected to supply the C macros but they're missing from whatever header they targeted. I think the most likely fix is to find the header that is supposed to provide these and include it.)

[Richard Thomson (LegalizeAdulthood)]

Draft is fine and is realistically more accurate for where we are. I'll put Codex on the job and see what it can determine about the root cause of the failure.

[CQ_Undefine (cqundefine)]

Richard Thomson (@LegalizeAdulthood) I don't know if that's the exact issue you're experiencing, but when I was testing on BSD systems, by just using the builtin toolchain on FreeBSD worked just fine, but for NetBSD and DragonFly BSD (and possibly OpenBSD I don't remember exactly) the workaround was installing some more newer toolchain like GCC 14 from the official repositories. I remember debugging this exact issue and it boils down to most C++ standard libraries are gonna ship their own math.h header which takes precedence over the C math.h. The C++ version is meant to work correctly with cmath and C++. That C++ version of math.h is missing in default toolchain on both NetBSD and DragonFly BSD.

[Richard Thomson (LegalizeAdulthood)]

Thank you, that is good information.

[Billy O'Neal (BillyONeal)]

most C++ standard libraries are gonna ship their own math.h header which takes precedence over the C math.h

They generally need to #include_next or similar to get the C macros. (or otherwise be prepared to offer the C macros)