<complex> fix pow overload

[Neargye]

Description

Will resolve issue #325.

Checklist

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

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.

[SuperWig]

Shouldn't type be _Type?

[Neargye]

Same as

STL/stl/inc/complex

Line 1822 in 70e49a0

using type = complex<_Ty1>;

and

STL/stl/inc/complex

Line 1803 in 70e49a0

using type = complex<_Common_float_type_t<_Ty1, _Ty2>>;

[StephanTLavavej]

@SuperWig

Shouldn't type be _Type?

This is a special exception to the general rule of "all internal identifiers must be _Ugly". When internal structs/classes implement helper type traits, we use type and value for them without uglification. This is because we don't have to worry about macros (those identifiers can't be macroized by users, or it would break the Standard names), and this avoids confusing catastrophes. (e.g. If some code provides _Type and other code expects type, SFINAE might happen, leading to really confusing diagnostics - this has happened in practice.) Note that this convention of using type and value isn't possible when otherwise-internal structs/classes make their contents visible to users (e.g. by serving as base classes of Standard classes).

Edit: Actually looked at the context. This is slightly different than the type trait rule, as it's a local alias, but it is permissible for the same reason. (Often we would use an _Ugly identifier here, but @Neargye found existing practice in this header.)

[Neargye]

https://github.com/microsoft/STL/search?p=1&q=%22using+type%22&unscoped_q=%22using+type%22
https://github.com/microsoft/STL/search?q=_Type&unscoped_q=_Type

I noticed that the code doesn't use "_Type" as typedef, only "type".

[Neargye]

Also on /W4 get many warnings conversion from 'double' to '_Ty', possible loss of data

    static _Ty pow(_Ty _Left, _Ty _Right) {
        return _CSTD pow(static_cast<double>(_Left), static_cast<double>(_Right));
    }

Maybe change like that?

    static _Ty pow(_Ty _Left, _Ty _Right) {
        return static_cast<_Ty>(_CSTD pow(static_cast<double>(_Left), static_cast<double>(_Right)));
    }

[Neargye]

Why clang-format wanna '&&is_integral_v<_Ty2>' 🤔

[AdamBucior]

Why clang-format wanna '&&is_integral_v<_Ty2>' 🤔

That is probably a bug. You can put // clang-format off/on around this line.

[BillyONeal]

Why clang-format wanna '&&is_integral_v<_Ty2>' 🤔

Clang-format doesn't understand how to format variable templates in clang 9. We presently value the ability to mechanically clang-format over it being as pretty as possible. AFAIK @StephanTLavavej filed a bug against clang-format which they have slated to fix in Clang 10.

[StephanTLavavej]

This is LLVM-43055 "clang-format misformats && expressions with variable templates in noexcept(expr)" but it is not yet fixed.

We generally disable clang-format only when it behaves egregiously, in a way that can't be worked around with tricks like // empty comments. Its poor formatting of variable templates generally doesn't rise to that level.

[Neargye]

🤔 work around: !((is_floating_point_v<_Ty1> || is_integral_v<_Ty1>) && is_integral_v<_Ty2>) -> !conjunction_v<is_integral<_Ty2>, disjunction<is_floating_point<_Ty1>, is_integral<_Ty1>>

[Neargye]

Change log:

• fix warning "C4244 'return': conversion from 'double' to '_Ty', possible loss of data" <complex> fix pow overload #383 (comment)
• fix pow overload
  • Optimized overload for two integral parameters.
    • _Right& -> const _Right

  template <class _Ty1, class _Ty2, enable_if_t<is_integral_v<_Ty1> && is_integral_v<_Ty2>, int> = 0>
  _NODISCARD complex<_Ty1> pow(const complex<_Ty1>& _Left, const _Ty2 _Right);
  

  • Optimized overload for floating and integral parameters.
    • Fix return type to complex<double> <complex>: pow(complex<float>, int) returns complex<float> instead of complex<double> #325
    • Accept any integer type as the _Right
    • Add SFINAE.

  template <class _Ty1, class _Ty2, enable_if_t<!is_integral_v<_Ty1> && is_integral_v<_Ty2>, int> = 0>
  _NODISCARD complex<_Common_float_type_t<_Ty1, _Ty2>> pow(const complex<_Ty1>& _Left, const _Ty2 _Right);
  

  • Default pow for other cases.
    • Add SFINAE.

  template <class _Ty1, class _Ty2, enable_if_t<!is_integral_v<_Ty2>, int> = 0>
  _NODISCARD complex<_Common_float_type_t<_Ty1, _Ty2>> pow(const complex<_Ty1>& _Left, const _Ty2& _Right);
  

[BillyONeal]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[StephanTLavavej]

Looks good, just three minor issues and one significant issue regarding signed vs. unsigned. Thanks!

[Neargye]

@StephanTLavavej

• Rebase to master.
• Fix make_unsigned_t & minor issues

[StephanTLavavej]

Looks good! I think I can write a small test for this and prepare a Microsoft-internal PR.

[StephanTLavavej]

Thanks for the fix, we really appreciate it!