This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH, libstdc++] Implement P0415 More constexpr for std::complex.
On Mon, 26 Nov 2018 at 12:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 26/11/18 09:30 +0100, Christophe Lyon wrote:
> >On Thu, 22 Nov 2018 at 10:20, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> On 20/11/18 17:58 -0500, Ed Smith-Rowland wrote:
> >> >On 11/19/18 6:13 AM, Jonathan Wakely wrote:
> >> >>On 16/11/18 19:39 -0500, Ed Smith-Rowland wrote:
> >> >>>@@ -322,67 +323,43 @@
> >> >>> //@{
> >> >>> /// Return new complex value @a x plus @a y.
> >> >>> template<typename _Tp>
> >> >>>- inline complex<_Tp>
> >> >>>+ inline _GLIBCXX20_CONSTEXPR complex<_Tp>
> >> >>> operator+(const complex<_Tp>& __x, const complex<_Tp>& __y)
> >> >>>- {
> >> >>>- complex<_Tp> __r = __x;
> >> >>>- __r += __y;
> >> >>>- return __r;
> >> >>>- }
> >> >>>+ { return complex<_Tp>(__x.real() + __y.real(), __x.imag() +
> >> >>>__y.imag()); }
> >> >>
> >> >>Is this change (and all the similar ones) really needed?
> >> >>
> >> >>Doesn't the fact that all the constructors and member operators of
> >> >>std::complex mean that the original definition is also valid in a
> >> >>constexpr function?
> >> >These changes are rolled back. Sorry.
> >> >>>@@ -1163,50 +1143,43 @@
> >> >>>#endif
> >> >>>
> >> >>> template<typename _Tp>
> >> >>>- complex&
> >> >>>+ _GLIBCXX20_CONSTEXPR complex&
> >> >>> operator=(const complex<_Tp>& __z)
> >> >>> {
> >> >>>- __real__ _M_value = __z.real();
> >> >>>- __imag__ _M_value = __z.imag();
> >> >>>+ _M_value = __z.__rep();
> >> >>
> >> >>These changes look OK, but I wonder if we shouldn't ask the compiler
> >> >>to make it possible to use __real__ and __imag__ in constexpr
> >> >>functions instead.
> >> >>
> >> >>I assume it doesn't, and that's why you made this change. But if it
> >> >>Just Worked, and the other changes I commented on above are also
> >> >>unnecessary, then this patch would *mostly* just be adding
> >> >>_GLIBCXX20_CONSTEXPR which is OK for stage 3 (as it doesn't affect any
> >> >>dialects except C++2a).
> >> >
> >> >Yes, this is the issue. I agree that constexpr _real__, __imag__would
> >> >be better.
> >> >
> >> >Do you have any idea where this change would be? I grepped around a
> >> >little and couldn't figure it out. if you don't I'll look more.
> >>
> >> No idea, sorry.
> >>
> >> >Actually, looking at constexpr.c it looks like the old way ought to work...
> >> >
> >> >OK, plain assignment works but not the others. Interesting.
> >> >
> >> >>
> >> >>>@@ -1872,7 +1831,7 @@
> >> >>> { return _Tp(); }
> >> >>>
> >> >>> template<typename _Tp>
> >> >>>- inline typename __gnu_cxx::__promote<_Tp>::__type
> >> >>>+ _GLIBCXX_CONSTEXPR inline typename
> >> >>>__gnu_cxx::__promote<_Tp>::__type
> >> >>
> >> >>This should be _GLIBCXX20_CONSTEXPR.
> >> >Done.
> >> >>>Index:
> >> >>>testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
> >> >>>===================================================================
> >> >>>---
> >> >>>testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
> >> >>>(nonexistent)
> >> >>>+++
> >> >>>testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
> >> >>>(working copy)
> >> >>>@@ -0,0 +1,51 @@
> >> >>>+// { dg-do compile { target c++2a } }
> >> >>
> >> >>All the tests with { target c++2a} should also have:
> >> >>
> >> >>// { dg-options "-std=gnu++2a" }
> >> >>
> >> >>Because otherwise they are skipped by default, and only get run when
> >> >>RUNTESTFLAGS explicitly includes something like
> >> >>--target_board=unix/-std=gnu++2a
> >> >>
> >> >>The dg-options needs to come first, or it doesn't apply before the
> >> >>check for { target c++2a }.
> >> >>
> >> >Thank you, done.
> >>
> >> OK for trunk, thanks.
> >>
> >> >Updated patch attached. I'd like to understand why
> >> >
> >> > __real__ _M_value += __z.real();
> >> >
> >> >doesn't work though.
> >>
> >> Yes, I agree it should. If you don't figure it out please file a bug
> >> requesting that it works, so somebody else might look into it.
> >>
> >
> >Hi,
> >I have noticed that
> >FAIL: 26_numerics/complex/requirements/more_constexpr.cc
> >on arm and aarch64
> >The error messages:
> >Excess errors:
> >/libstdc++-v3/testsuite/26_numerics/complex/requirements/more_constexpr.cc:168:
> >error: '__float128' was not declared in this scope
> >/libstdc++-v3/testsuite/26_numerics/complex/requirements/more_constexpr.cc:168:
> >error: no matching function for call to
> >'test_operator_members<<expression error>, __float128>()'
> >/libstdc++-v3/testsuite/26_numerics/complex/requirements/more_constexpr.cc:168:
> >error: template argument 1 is invalid
>
> Should be fixed by this patch, committed to trunk.
>
>
Indeed, thanks.