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

Avoid using _FInf etc. in numeric_limits.h after MSVC's STL removes them #474

Merged
merged 1 commit into from
Jul 22, 2022
Merged

Avoid using _FInf etc. in numeric_limits.h after MSVC's STL removes them #474

merged 1 commit into from
Jul 22, 2022

Conversation

StephanTLavavej
Copy link
Contributor

I work on the Visual C++ team, where we regularly build many open-source projects, including EASTL, with development builds of the MSVC compiler and libraries, in order to prevent regressions that would affect projects. This also allows us to provide advance notice of breaking changes, such as this one:

We've merged microsoft/STL#2828, which is removing the declarations of the _FInf etc. variables from our headers. This change will ship in VS 2022 17.4 Preview 2, and it breaks EASTL because MSVC's STL still defines _CPPLIB_VER (identifying ourselves as Dinkumware-derived sources).

This PR uses our supported macro _MSVC_STL_UPDATE for identifying the updated versions of MSVC's STL, and instead uses the compiler's __builtin_huge_valf() etc. intrinsics, plus FLT_TRUE_MIN etc. from the UCRT.

These intrinsics and macros have been available for quite some time. Note that they will improve codegen, as they produce constants that are known to the compiler. In contrast, the old internal variables are separately compiled (and modifiable!), which inhibits optimizations like constant propagation.

@grojo-ea
Copy link
Contributor

Hi @StephanTLavavej, thank you very much for brining this to our attention and for the PR with the fix.

We'd like to take this PR but it looks like we don't have a Contributor License Agreement for you on file yet. Do you mind filling out this form so we can merge this and any future PRs that you author?

Thanks again.

@StephanTLavavej
Copy link
Contributor Author

@grojo-ea Done, thanks!

@grojo-ea grojo-ea merged commit ae8cc01 into electronicarts:master Jul 22, 2022
@StephanTLavavej StephanTLavavej deleted the fix-floating-limits branch July 22, 2022 23:51
blattersturm added a commit to citizenfx/fivem that referenced this pull request Sep 26, 2022
This change is mainly to pull in electronicarts/EASTL#474 for compat
with VS17.4p2.
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