Re: [PATCH] libstdc++/complex: Remove implicit type casts in complex

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]

On Mon, 27 Mar 2023 at 22:25, Weslley da Silva Pereira via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> Dear all,
>
> Here follows a patch that removes implicit type casts in std::complex.
>
> *Description:* The current implementation of `complex<_Tp>` assumes that
> `int, double, long double` are explicitly convertible to `_Tp`. Moreover,
> it also assumes that:
>
> 1. `int` is implicitly convertible to `_Tp`, e.g., when using
> `complex<_Tp>(1)`.
> 2. `long double` can be attributed to a `_Tp` variable, e.g., when using
> `const _Tp __pi_2 = 1.5707963267948966192313216916397514L`.
>
> This patch transforms the implicit casts (1) and (2) into explicit type
> casts. As a result, `std::complex` is now able to support more types. One
> example is the type `Eigen::Half` from
> https://eigen.tuxfamily.org/dox-devel/Half_8h_source.html which does not
> implement implicit type conversions.
>
> *ChangeLog:*
> libstdc++-v3/ChangeLog:
>
>         * include/std/complex:
>

Thank you for the patch. Now that we're in developement stage 1 for GCC 14,
it's time to consider it.

You're missing a proper changelog entry, I suggest:

       * include/std/complex (polar, __complex_sqrt)
       (__complex_pow_unsigned, pow, __complex_acos): Replace implicit
       conversions from int and long double to value_type.

You're also missing either a copyright assignment on file with the FSF
(unless you've completed that paperwork?), or a DCO sign-off. Please see
https://gcc.gnu.org/contribute.html#legal and https://gcc.gnu.org/dco.html
for more details.


>
> *Patch:* fix_complex.diff. (Also at
> https://github.com/gcc-mirror/gcc/pull/84)
>
> *OBS:* I didn't find a good reason for adding new tests or test results
> here since this is really a small upgrade (in my view) to std::complex.
>

I don't agree. The purpose of this is to support std::complex<Foo> for a
type Foo without implicit conversions (which isn't required by the standard
btw, only the floating-point types are required to work, but we can support
others as an extension). Without tests, we don't know if that goal has been
met, and we don't know if the goal continues to be met in future versions.
A test would ensure that we don't accidentally re-introduce code requiring
implicit conversions.

With a suitable test, I think this patch will be OK for GCC 14.

Thanks again for contributing.