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

Corrected build under msvc compiler #1833 #2121

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

kamil-goras-mobica
Copy link
Contributor

@kamil-goras-mobica kamil-goras-mobica commented Oct 18, 2024

Fixes #1833 according to task description.

@kamil-goras-mobica kamil-goras-mobica marked this pull request as ready for review October 21, 2024 06:52
@kamil-goras-mobica kamil-goras-mobica changed the title Corrected build under msvc compiler Corrected build under msvc compiler #1833 Oct 21, 2024
@kamil-goras-mobica
Copy link
Contributor Author

#1833

@bashbaug
Copy link
Contributor

This is an improvement, thanks, but I'm wondering if we might be able to do a bit better.

Could we switch entirely to the standard (C++11) functions for isfinite, isnan , and isinf?

Basically, can we change all of the calls to these functions to std::isfinite, std::isnan, and std::isinf?

@kamil-goras-mobica kamil-goras-mobica marked this pull request as draft October 31, 2024 10:35
@kamil-goras-mobica kamil-goras-mobica marked this pull request as ready for review October 31, 2024 11:10
@kamil-goras-mobica
Copy link
Contributor Author

kamil-goras-mobica commented Oct 31, 2024

@bashbaug I've added correction now with using directive all calls isnan should be using std::isnan, also is needed. Please take a look and decide is it what you wanted or not. Thanks

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me. It's definitely a step in the right direction.

Ultimately I'd like to eliminate these "using" lines and just call the std:: versions directly, but we can do that in a follow-on PR.

@bashbaug
Copy link
Contributor

Merging as discussed in the November 12th teleconference.

@bashbaug bashbaug merged commit 265cc18 into KhronosGroup:main Nov 12, 2024
7 checks passed
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.

isnan, isfinite, and isinf are implemented as macros for MSVC
2 participants