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

[Weslley da Silva Pereira <weslley.spereira@gmail.com>]

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

Hi Jonathan,

Is there a way I can see my patch merged (when it gets merged)?
Particularly, I want to have a link for the commit. I would like to add
this as "impact on third party software" for the software
https://github.com/tlapack/tlapack.

Thanks,
  Weslley

On Mon, Nov 6, 2023 at 3:44 AM Jonathan Wakely <jwakely@redhat.com> wrote:

> On Fri, 3 Nov 2023 at 17:47, Weslley da Silva Pereira
> <weslley.spereira@gmail.com> wrote:
> >
> > Hi Jonathan,
> >
> > I am sorry for the delay. The mailing lists libstdc++@gcc.gnu.org and
> gcc-patches@gcc.gnu.org have just too many emails, so your email got
> lost. I hope my changes still make sense to be included in GCC. Please,
> find my comments below.
>
> Hi,
>
> Thanks for the updated patch, test etc. Yes, I think this still makes
> sense and I'll take care of committing it.
>
>
>
> >
> > On Thu, May 11, 2023 at 3:57 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> >>
> >>
> >>
> >> 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.
> >
> >
> > I agree with your proposal for the changelog.
> >
> >>
> >> 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.
> >
> >
> > Here is my DCO sign-off:
> >
> > Copyright:
> > Signed-off-by: Weslley da Silva Pereira <weslley.spereira@gmail.com>
> >
> >>
> >>
> >>>
> >>>
> >>> *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.
> >>
> >>
> >
> > Tests:
> > See the attached file `test_complex_eigenhalf.cpp`
> >
> > Test results:
> > - When using x86-64 GCC (trunk), I obtained compilation errors as shown
> in the attached text file. Live example at:
> https://godbolt.org/z/oa9M34h8P.
> > - I observed no errors after applying the suggested patch on my machine.
> > - I tried it with the flag `-Wall`. No warnings were shown.
> > - My machine configuration and my GCC build information are displayed in
> the file `config.log` generated by the configuration step of GCC.
> >
> > Let me know if I need to do anything else.
> >
> > Thanks,
> >   Weslley
> >
> > --
> > Weslley S. Pereira
>
>

-- 
Weslley S. Pereira