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

P1467R9 Extended Floating-Point Types #3583

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

StephanTLavavej
Copy link
Member

Fixes #2956.

  • <stdfloat> contains a couple of comments explaining why it's almost, but not completely, empty.
    • All of the push-pop machinery is extremely intentional - it is important that every header contains this machinery, so that we won't introduce errors during later maintenance or copy-pasting.
  • I'm adding this to the Core section of <__msvc_all_public_headers.hpp>.
  • <yvals_core.h> contains a clarifying comment, so that readers will understand what we're claiming to support.
  • I don't think we need to add a dedicated test for this. Technically, the include-each-header-alone, header units, and modules tests aren't verifying that <stdfloat> itself provides namespace std, but the comment in the header ensures that we won't remove this during maintenance. It doesn't seem to be worth spending further test throughput to verify one of the world's emptiest headers.

@StephanTLavavej StephanTLavavej added the cxx23 C++23 feature label Mar 20, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 20, 2023 22:25
@github-actions github-actions bot added this to Initial Review in Code Reviews Mar 20, 2023
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Mar 20, 2023
@strega-nil-ms strega-nil-ms moved this from Final Review to Ready To Merge in Code Reviews Mar 20, 2023
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

In WG21-P1467R9, 9.3.6 adds a requirement that specializations of std::complex are trivially-copyable; should we have test coverage?

stl/inc/yvals_core.h Show resolved Hide resolved
@StephanTLavavej
Copy link
Member Author

@strega-nil-ms @CaseyCarter I've pushed test coverage as requested, after finding the best place to add it.

Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Approved harder!

@StephanTLavavej StephanTLavavej self-assigned this Mar 23, 2023
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit c353b68 into microsoft:main Mar 24, 2023
Code Reviews automation moved this from Ready To Merge to Done Mar 24, 2023
@StephanTLavavej StephanTLavavej deleted the stdfloat branch March 24, 2023 01:42
@TautologicalContradiction

I think at least std::float32_t and std::float64_t should be implemented atm, both could be aliases to float and double respectively, since these two types based on the MSVC's implementation, accomplish the requirements of std::float32_t and std::float64_t

@frederick-vs-ja
Copy link
Contributor

I think at least std::float32_t and std::float64_t should be implemented atm, both could be aliases to float and double respectively, since these two types based on the MSVC's implementation, accomplish the requirements of std::float32_t and std::float64_t

This is not allowed by the standard. float and double are standard floating-point types, while std::float32_t and std::float64_t are extended floating-point types, so they must be distinct.

I want to add std::float16_t (i.e. _Float16) support for Clang (perhaps together with EDG), but maintainers seemly don't like doing so until MSVC supports std::float16_t.

@2bitsin
Copy link

2bitsin commented Aug 20, 2023

What is the actual point of adding this header if it does not contain std::float32_t or std::float64_t atleast...

@CaseyCarter
Copy link
Member

What is the actual point of adding this header if it does not contain std::float32_t or std::float64_t atleast...

We don't design the Standard Library, we just implement it. The working draft for the C++23 Standard says this header exists - it's not optional - so here it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

P1467R9 Extended Floating-Point Types
6 participants