From: Jakub Jelinek <jakub@redhat.com>
To: Jonathan Wakely <jwakely@redhat.com>
Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org
Subject: Re: [PATCH] libstdc++-v3: <complex> support for extended floating point types
Date: Mon, 31 Oct 2022 17:57:33 +0100 [thread overview]
Message-ID: <Y1/+faSCyvTztVBH@tucnak> (raw)
In-Reply-To: <CACb0b4k+tOXw=-M_WQ95e6miEFbT6ZSS5G96ffXvR0PmM4VMYQ@mail.gmail.com>
On Mon, Oct 31, 2022 at 10:26:11AM +0000, Jonathan Wakely wrote:
> > --- libstdc++-v3/include/std/complex.jj 2022-10-21 08:55:43.037675332 +0200
> > +++ libstdc++-v3/include/std/complex 2022-10-21 17:05:36.802243229 +0200
> > @@ -142,8 +142,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >
> > /// Converting constructor.
> > template<typename _Up>
> > +#if __cplusplus > 202002L
> > + explicit(!requires(_Up __u) { _Tp{__u}; })
> > + constexpr complex(const complex<_Up>& __z)
> > + : _M_real(_Tp(__z.real())), _M_imag(_Tp(__z.imag())) { }
>
> Do these casts to _Tp do anything? The _M_real and _M_imag members are
> already of type _Tp and we're using () initialization not {} so
> there's no narrowing concern. _M_real(__z.real()) is already an
> explicit conversion from decltype(__z.real()) to decltype(_M_real) so
> the extra _Tp is redundant.
If I use just
: _M_real(__z.real()), _M_imag(__z.imag()) { }
then without -Wsystem-headers there are no regressions, but compiling
g++.dg/cpp23/ext-floating12.C with additional -Wsystem-headers
-pedantic-errors results in lots of extra errors on that line:
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float32’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float32’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float32’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float32’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float32’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float32’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float32’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float32’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float64’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float64’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float64’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float64’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘float’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘float’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘_Float32’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘_Float32’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘_Float16’ from ‘__bf16’ with unordered conversion ranks
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘_Float16’ from ‘__bf16’ with unordered conversion ranks
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘float’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘float’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘long double’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘_Float32’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘_Float32’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘_Float64’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘_Float128’ with greater conversion rank
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:27: error: converting to ‘__bf16’ from ‘_Float16’ with unordered conversion ranks
.../x86_64-pc-linux-gnu/libstdc++-v3/include/complex:149:48: error: converting to ‘__bf16’ from ‘_Float16’ with unordered conversion ranks
__z doesn't have _Tp type, but _Up, which is some other floating
point type. For the conversion constructors where we have
explicit(false) all is fine, we've tested in the requires that
_Up is implicitly convertable to _Tp.
But if explicit(true), it is fine without the explicit casts only
for long double -> float, long double -> double and double -> float
casts which are ok for implicit casts but not in narrowing conversions
with non-constants.
Otherwise it results in the above diagnostics.
> I think the diff between C++23 and older standards would be smaller
> done like this:
>
> /// Converting constructor.
> template<typename _Up>
> #if __cplusplus > 202002L
> explicit(!requires(_Up __u) { _Tp{__u}; })
> #endif
> _GLIBCXX_CONSTEXPR complex(const complex<_Up>& __z)
> : _M_real(__z.real()), _M_imag(__z.imag()) { }
I could go for this with the _Tp() casts everywhere, or
make also that line (perhaps except for { }) also conditional on
#if __cplusplus > 202002L.
> This also means the constructor body is always defined on the same
> line, which avoids warnings from ld.gold's -Wodr-violations which IIRC
> is based on simple heuristics like the line where the function is
> defined.
>
> Otherwise this looks great. If the above alternative works, please use
> that, but OK for trunk either way (once all the other patches it
> depends on are in).
All the builtins patches are in since today, and the
"Small extended float support tweaks" patch is in too (it had a small
testsuite conflict due to the
https://gcc.gnu.org/pipermail/libstdc++/2022-October/054849.html
patch still pending, but I've just resolved that conflict).
I think this patch doesn't depend on anything anymore.
OT, I also get on the same testcase with -Wsystem-headers -pedantic-errors
.../x86_64-pc-linux-gnu/libstdc++-v3/include/cmath:47:2: error: #include_next is a GCC extension
.../x86_64-pc-linux-gnu/libstdc++-v3/include/bits/std_abs.h:38:2: error: #include_next is a GCC extension
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/numbers:223:1: error: non-standard suffix on floating constant [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/cstdlib:79:2: error: #include_next is a GCC extension
.../libstdc++-v3/libsupc++/compare:844:45: error: ISO C++ does not support ‘__int128’ for ‘type name’ [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/cstddef:91:36: error: ISO C++ does not support ‘__int128’ for ‘type name’ [-Wpedantic]
.../x86_64-pc-linux-gnu/libstdc++-v3/include/cstddef:93:45: error: ISO C++ does not support ‘__int128’ for ‘type name’ [-Wpedantic]
For the numbers case (__float128 in there), wonder if we can wrap it in __extension__ or
use __extension__ __float128 as the type (apparently e.g. type_traits uses
__extension__ before the template keyword of the specializations), for the __int128 cases perhaps
too, no idea about #include_next though.
Jakub
next prev parent reply other threads:[~2022-10-31 16:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-21 15:58 Jakub Jelinek
2022-10-31 10:26 ` Jonathan Wakely
2022-10-31 16:57 ` Jakub Jelinek [this message]
2022-10-31 17:04 ` Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y1/+faSCyvTztVBH@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jwakely@redhat.com \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).