<complex> Implement p0415r1 #367

Neargye · 2019-12-10T13:39:39Z

Description

This is an in progress implementation of constexpr <complex>.
Will resolve issue #16, #190.

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

Identifiers in product code changes are properly _Ugly as per
https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
The STL builds successfully and all tests have passed (must be manually
verified by an STL maintainer before automated testing is enabled on GitHub,
leave this unchecked for initial submission).
These changes introduce no known ABI breaks (adding members, renaming
members, adding virtual functions, changing whether a type is an aggregate
or trivially copyable, etc.).
These changes were written from scratch using only this repository,
the C++ Working Draft (including any cited standards), other WG21 papers
(excluding reference implementations outside of proposed standard wording),
and LWG issues as reference material. If they were derived from a project
that's already listed in NOTICE.txt, that's fine, but please mention it.
If they were derived from any other project (including Boost and libc++,
which are not yet listed in NOTICE.txt), you must mention it here,
so we can determine whether the license is compatible and what else needs
to be done.

stl/inc/complex

stl/inc/yvals_core.h

StephanTLavavej

This is an excellent start, thank you! As we previously discussed, we know that _Div is incomplete. Additionally, I observe that P0415R1 specifies that the "additional overloads" of norm, conj, imag, and real need to be constexpr in C++20; that also appears to be unimplemented so far.

stl/inc/yvals_core.h

stl/inc/complex

Neargye · 2019-12-11T15:48:38Z

Maybe I should break it down into a few pr?
Or it's ok one big pr to update <complex>?

StephanTLavavej · 2019-12-12T00:48:31Z

Maybe I should break it down into a few pr?
Or it's ok one big pr to update <complex>?

It's a judgement call. We prefer feature PRs to be complete, i.e. not requiring additional PRs to complete the feature. (We make exceptions in rare cases, e.g. charconv, spaceship, and ranges.) We also prefer bugfixes to be complete, but incremental changes can be okay as long as each step is a strict improvement. Separating PRs is useful when changes are large and not intertwined (especially when one of the changes is mechanical or nearly so). It's usually okay to have a large change bundled with a few small improvements "while you're in the neighborhood", though. And sometimes it's just difficult to avoid bundling large, interconnected changes together.

One final reason to prefer splitting PRs when possible is that it makes the commit history more comprehensible, even if your reviewers have a high tolerance for larger PRs (as I do, having reviewed truly massive changes over the last decade).

In this case, I believe that fixing #325 isn't strongly related to constexprization (and WG21-P0415 doesn't change pow() as far as I see), so I'd prefer to see separate PRs. But if they're related for some reason, then a single PR is fine.

Neargye · 2019-12-12T08:07:10Z

@StephanTLavavej
Thanks for your explanation.

For fix #325 creat new pr #358

For this pr, everything is done, except operator/ because of block by #22.

StephanTLavavej · 2020-03-07T00:15:48Z

bit_cast is now available.

* operator+ * operator- * operator* * operator/ (partially) * norm * conj * real * imag * operator= * operator+= * operator-= * operator-= * operator*= * operator/= (partially)

Neargye · 2020-03-07T08:30:20Z

Rebase pr to master
constexpr operator/

cbezault · 2020-03-27T15:06:20Z

/azp run

azure-pipelines · 2020-03-27T15:06:28Z

Azure Pipelines successfully started running 1 pipeline(s).

stl/inc/complex

…xpr_complex

cbezault · 2020-03-31T18:50:52Z

I've enabled the libcxx tests that are supposed to check this feature.

cbezault · 2020-04-01T15:53:34Z

The last thing I might ask for is that you add a test to the std testsuite which tests the correctness of the constexpr isnan implementation. libcxx already has tests for simply checking that the functions are constexpr.

I'd probably defer to @StephanTLavavej as to whether or not that is necessary though.

is_constant_evaluated and bit_cast are now always available in C++20 mode. Additionally, we don't need `else` after `if ... return`, which avoids having non-braced control flow.

In C++20 mode, we can always use bit_cast. At runtime, it should be significantly more efficient than calling _Dtest etc. which is separately compiled and performs a full classification instead of testing for just NaN. Avoid unnecessary static_cast<double> - our long double is always 64-bit, and double is already double. Use auto after bit_cast to avoid repeating the type. Add const. For consistency, use named _Uint for 32-bit float too.

Copy bit_cast in <bit> to _Bit_cast in <xutility>, superseding _Bit_cast in <charconv>. Unlike C++20 bit_cast, _Bit_cast is always available. (<xutility> is a common ancestor of <complex> and <charconv>, and includes the necessary machinery for this definition.) I verified that MSVC, Clang, and IntelliSense all accept `__builtin_bit_cast` in 14/17 mode. For CUDA, I provide the memcpy fallback (which isn't constexpr, of course). Finally, _Isinf should use _Bit_cast; I'm marking that with a comment (and will file a GitHub issue in the future) instead of extending this PR indefinitely.

StephanTLavavej · 2020-04-25T02:43:12Z

I'm reviewing this now - I've pushed a merge with master, a feature-test macro test update, and bit_cast changes. Microsoft-internal tests are passing. I still need to actually review <complex> and write some basic testing. Should be ready for checkin next week. Thanks for your patience while we clear out the PR backlog! 😸

This extends the PR a bit more than strictly necessary, but I realized it was simpler than filing an issue. Also use `const auto` after static_cast<double>.

This eliminates the _GENERIC_MATHC0X and _GENERIC_MATHC1X macros in favor of directly stamping out the functions. (Over the years, we've moved away from the practice of using macros to stamp out highly repetitive code. The only remaining scenario where I believe that macros are valuable is when we need to stamp out something for every possible calling convention etc. as that's incredibly verbose and platform-dependent.) Additionally, this changes _Promote_to_float to _Upgrade_to_double. This is better-named in two different ways ("promote" is a Word Of Power that isn't exactly applicable here, and the replacement type is double). It's also less verbose and higher throughput (the former is obvious; the latter is because struct templates are more expensive for compilers to instantiate, as they have to do a lot of work when they generate a class definition). Avoiding macros allows us to perform a couple of micro-cleanups: we can directly return 0 (no need to static_cast to floating type) and we don't have to parenthesize `_Left * _Left` to make clang-format happy.

CaseyCarter

I'm happy with what I see here, but a little concerned about test coverage. The names of the libc++ tests suggest they only cover real and imag - should we have some basic smoke tests to ensure these functions are callable in a constant expression with at least one set of parameter values?

stl/inc/complex

StephanTLavavej · 2020-04-28T01:24:38Z

I am writing test coverage at this very moment.

StephanTLavavej · 2020-04-30T09:16:11Z

Thanks for implementing this feature - we're one step closer to C++20 completeness! 😸🎉

(Along this way, this uncovered a bug in the compiler's support for modules, which our FE dev Cameron DaCamara has already fixed!)

Neargye requested a review from a team as a code owner December 10, 2019 13:39

miscco reviewed Dec 10, 2019

View reviewed changes

stl/inc/complex Show resolved Hide resolved

miscco reviewed Dec 10, 2019

View reviewed changes

stl/inc/yvals_core.h Outdated Show resolved Hide resolved

Neargye force-pushed the constexpr_complex branch from 187c57a to 21ffa54 Compare December 10, 2019 15:54

StephanTLavavej requested changes Dec 11, 2019

View reviewed changes

stl/inc/yvals_core.h Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

Neargye force-pushed the constexpr_complex branch 2 times, most recently from 6537085 to 339c106 Compare December 11, 2019 10:44

Neargye requested a review from StephanTLavavej December 11, 2019 13:02

Neargye mentioned this pull request Dec 11, 2019

<complex>: pow(complex<float>, int) returns complex<float> instead of complex<double> #325

Closed

Neargye force-pushed the constexpr_complex branch from a0f8e9e to 339c106 Compare December 12, 2019 08:01

BillyONeal approved these changes Dec 13, 2019

View reviewed changes

StephanTLavavej added the cxx20 C++20 feature label Feb 5, 2020

SunnyWar mentioned this pull request Mar 3, 2020

<complex> simplify and cleanup #509

Closed

4 tasks

Neargye added 6 commits March 7, 2020 10:47

fix microsoft#190

a4c658c

Mark constexpr:

73433f7

* operator+ * operator- * operator* * operator/ (partially) * norm * conj * real * imag * operator= * operator+= * operator-= * operator-= * operator*= * operator/= (partially)

define __cpp_lib_constexpr_complex

eb17be0

operator= return complex&

a9f0640

marked _Ctraits functions constexpr that forward numeric_limits

a4cebd2

mark additional overloads norm, conj, imag and real constexpr

3472827

Neargye force-pushed the constexpr_complex branch from 339c106 to 27cbd9b Compare March 7, 2020 08:29

Neargye force-pushed the constexpr_complex branch 5 times, most recently from c600e01 to 3b94871 Compare March 7, 2020 09:26

cbezault suggested changes Mar 31, 2020

View reviewed changes

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Outdated Show resolved Hide resolved

stl/inc/complex Show resolved Hide resolved

cbezault added 2 commits March 31, 2020 11:49

Merge branch 'master' of https://github.com/Microsoft/STL into conste…

b5fc371

…xpr_complex

Remove skips for libcxx complex tests

af52e6c

small fix

84deed5

cbezault added 2 commits April 9, 2020 19:10

Merge branch 'master' into constexpr_complex

6372e4d

Fix Clang format issue

1d1e1ef

cbezault approved these changes Apr 10, 2020

View reviewed changes

StephanTLavavej added 5 commits April 24, 2020 16:21

Merge branch 'master' into constexpr_complex

2ca4ce4

Update VSO_0157762_feature_test_macros.

cb4ad70

Adjust conditional compilation.

d5b6b8f

is_constant_evaluated and bit_cast are now always available in C++20 mode. Additionally, we don't need `else` after `if ... return`, which avoids having non-braced control flow.

StephanTLavavej self-assigned this Apr 25, 2020

StephanTLavavej added 3 commits April 27, 2020 16:01

Use _Bit_cast in _Isinf.

6e0429e

This extends the PR a bit more than strictly necessary, but I realized it was simpler than filing an issue. Also use `const auto` after static_cast<double>.

Merge branch 'master' into constexpr_complex

9c53675

CaseyCarter approved these changes Apr 28, 2020

View reviewed changes

stl/inc/complex Show resolved Hide resolved

Add P0415R1_constexpr_complex testing.

fe89667

StephanTLavavej approved these changes Apr 28, 2020

View reviewed changes

cbezault approved these changes Apr 28, 2020

View reviewed changes

CaseyCarter approved these changes Apr 28, 2020

View reviewed changes

StephanTLavavej merged commit 2bc5f9b into microsoft:master Apr 30, 2020

Neargye deleted the constexpr_complex branch June 7, 2020 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

<complex> Implement p0415r1 #367

<complex> Implement p0415r1 #367

Neargye commented Dec 10, 2019 •

edited

Loading

StephanTLavavej left a comment

Neargye commented Dec 11, 2019 •

edited

Loading

StephanTLavavej commented Dec 12, 2019

Neargye commented Dec 12, 2019 •

edited

Loading

StephanTLavavej commented Mar 7, 2020

Neargye commented Mar 7, 2020

cbezault commented Mar 27, 2020

azure-pipelines bot commented Mar 27, 2020

cbezault commented Mar 31, 2020

cbezault commented Apr 1, 2020

StephanTLavavej commented Apr 25, 2020

CaseyCarter left a comment

StephanTLavavej commented Apr 28, 2020

StephanTLavavej commented Apr 30, 2020

<complex> Implement p0415r1 #367

<complex> Implement p0415r1 #367

Conversation

Neargye commented Dec 10, 2019 • edited Loading

Description

Checklist

StephanTLavavej left a comment

Choose a reason for hiding this comment

Neargye commented Dec 11, 2019 • edited Loading

StephanTLavavej commented Dec 12, 2019

Neargye commented Dec 12, 2019 • edited Loading

StephanTLavavej commented Mar 7, 2020

Neargye commented Mar 7, 2020

cbezault commented Mar 27, 2020

azure-pipelines bot commented Mar 27, 2020

cbezault commented Mar 31, 2020

cbezault commented Apr 1, 2020

StephanTLavavej commented Apr 25, 2020

CaseyCarter left a comment

Choose a reason for hiding this comment

StephanTLavavej commented Apr 28, 2020

StephanTLavavej commented Apr 30, 2020

Neargye commented Dec 10, 2019 •

edited

Loading

Neargye commented Dec 11, 2019 •

edited

Loading

Neargye commented Dec 12, 2019 •

edited

Loading