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

P0631R8 <numbers> Math Constants #29

Closed
StephanTLavavej opened this issue Sep 6, 2019 · 4 comments · Fixed by #261
Closed

P0631R8 <numbers> Math Constants #29

StephanTLavavej opened this issue Sep 6, 2019 · 4 comments · Fixed by #261
Labels
cxx20 C++20 feature fixed Something works now, yay!
Milestone

Comments

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Sep 6, 2019

P0631R8 <numbers> Math Constants

The IDE's list of extensionless headers has been updated for VS 2019 16.5.

@StephanTLavavej StephanTLavavej added the cxx20 C++20 feature label Sep 6, 2019
@CaseyCarter
Copy link
Member

Per the author of P0631, we should submit an update to the documentation at https://docs.microsoft.com/en-us/cpp/c-runtime-library/math-constants?view=vs-2019 after implementing this feature.

@SuperWig
Copy link
Contributor

SuperWig commented Nov 4, 2019

This should be pretty straight forward, right?
Should it just support concepts or not?

@StephanTLavavej
Copy link
Member Author

  • Requiring concepts support seems reasonable.
  • License discipline is important. I don't think we should refer to any of the References in WG21-P0631. For example, the MIT license is different from our Apache License v2.0 with LLVM Exception.
  • It's fine to obtain pure mathematical constants from sources like Wolfram Alpha.
  • In general, it is incorrect to round an infinitely-precise real number to 64-bit double and then round it again to 32-bit float; see LWG-2403 for an explicit example. (Note: compilers themselves mishandle this; at least two front-ends parse 3.14f as double then round to float. Clang handles it correctly.) This is very rare, and reportedly, none of the real numbers in <numbers> trigger this twice-rounding issue (for IEEE 64/32). However, it still makes me nervous to see decimal constants in source code (how do I know if they've been rounded too early?) and also nervous to see double rounded to float. I think what I'd like to see is a program that takes the pure mathematical constants and uses charconv or strtod/strtof to round them to 64-bit/32-bit, then print them out as hexadecimal floating-point. Then the header can have double and float hexfloat constants, which will be nice and fast to parse, and alleviate my concerns. (I observe that Wolfram Alpha will give me 3,648 significant digits for sqrt(3); 768 significant digits is sufficient for double and more than enough for float.) Alternatively, the program could double-check that for these values, rounding double to float is binary-identical to directly converting decimal to float; I would then accept static_cast<_Floating>(0x1.abcd1234p0) in the header, since that would be more convenient than having specializations for float.

@SuperWig
Copy link
Contributor

SuperWig commented Nov 5, 2019

Thanks for the direction!

One question, if it's dependant on concepts, what do I do re: <yvals_core>. There doesn't seem to be a "conditionally implemented" section.

Or could a non-concepts version also been provided via a type alias to side-step that?

template <class _Ty>
using _Floating = enable_if_t<is_floating_point_v<_Ty>, _Ty>;

Edit: I ask because I already did that.
I currently have it implemented using decimal and with the same amount of sig figs as M_PI et al.
The test program shows both what I have currently and from_chars result in the same hexfloat (unless I've cocked it up, which is likely). Would you still prefer then as hexadecimal?

StephanTLavavej pushed a commit that referenced this issue Nov 12, 2019
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Nov 12, 2019
@cbezault cbezault added this to the Conformance milestone May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx20 C++20 feature fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants