Re: [wwwdocs] Document more C++2a support in libstdc++
On Wed, Jul 4, 2018 at 4:17 PM, Jonathan Wakely wrote: > + std::is_trivially_convertible, and s/trivially/nothrow/
Re: [PATCH 4/8] libstdc++: Fix error handling in std::print
[print.fun] requires a system_error, but I don't think
[ostream.formatted.print] does?
On Tue, Feb 27, 2024 at 5:47 AM Jonathan Wakely wrote:
> Tested x86_64-linux. Reviews invited.
>
> -- >8 --
>
> The standard requires an exception if std::print fails to write to a
> std::ostream.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ostream (vprint_nonunicode): Throw if stream state
> indicates writing failed.
> * testsuite/27_io/basic_ostream/print/1.cc: Check for exception.
> * testsuite/27_io/print/1.cc: Likewise.
> ---
> libstdc++-v3/include/std/ostream| 5 +
> .../testsuite/27_io/basic_ostream/print/1.cc| 17 +
> libstdc++-v3/testsuite/27_io/print/1.cc | 16
> 3 files changed, 38 insertions(+)
>
> diff --git a/libstdc++-v3/include/std/ostream
> b/libstdc++-v3/include/std/ostream
> index a136399ad0b..3740ad6edfa 100644
> --- a/libstdc++-v3/include/std/ostream
> +++ b/libstdc++-v3/include/std/ostream
> @@ -901,6 +901,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __catch(...)
> { __os._M_setstate(ios_base::badbit); }
>}
> +
> +if (!__os)
> + __throw_system_error(EIO);
>}
>
>inline void
> @@ -974,6 +977,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __catch(...)
> { __os._M_setstate(ios_base::badbit); }
>}
> +if (!__os)
> + __throw_system_error(EIO);
> #endif // _WIN32
>}
>
> diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> b/libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> index 71a4daa04c9..14bfb14d556 100644
> --- a/libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> +++ b/libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> @@ -103,6 +103,22 @@ test_locale()
>}
> }
>
> +void
> +test_errors()
> +{
> +#ifdef __cpp_exceptions
> + std::stringstream in(std::ios::in);
> + try
> + {
> +std::print(in, "{}", "nope");
> +VERIFY(false);
> + }
> + catch (const std::system_error&)
> + {
> + }
> +#endif
> +}
> +
> int main()
> {
>test_print_ostream();
> @@ -111,4 +127,5 @@ int main()
>test_print_no_padding();
>test_vprint_nonunicode();
>test_locale();
> + test_errors();
> }
> diff --git a/libstdc++-v3/testsuite/27_io/print/1.cc
> b/libstdc++-v3/testsuite/27_io/print/1.cc
> index 6a294e0454b..d570f7938be 100644
> --- a/libstdc++-v3/testsuite/27_io/print/1.cc
> +++ b/libstdc++-v3/testsuite/27_io/print/1.cc
> @@ -74,6 +74,21 @@ test_vprint_nonunicode()
>// { dg-output "garbage in . garbage out" }
> }
>
> +void
> +test_errors()
> +{
> +#ifdef __cpp_exceptions
> + try
> + {
> +std::print(stdin, "{}", "nope");
> +VERIFY(false);
> + }
> + catch (const std::system_error&)
> + {
> + }
> +#endif
> +}
> +
> int main()
> {
>test_print_default();
> @@ -82,4 +97,5 @@ int main()
>test_println_file();
>test_print_raw();
>test_vprint_nonunicode();
> + test_errors();
> }
> --
> 2.43.0
>
>
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely wrote:
> Tested x86_64-linux. Pushed to trunk.
>
> -- >8 --
>
> This adds the C++23 std::print functions, which use std::format to write
> to a FILE stream or std::ostream (defaulting to stdout).
>
> The new extern symbols are in the libstdc++exp.a archive, so we aren't
> committing to stable symbols in the DSO yet. There's a UTF-8 validating
> and transcoding function added by this change. That can certainly be
> optimized, but it's internal to libstdc++exp.a so can be tweaked later
> at leisure.
>
> Currently the external symbols work for all targets, but are only
> actually used for Windows, where it's necessary to transcode to UTF-16
> to write to the console. The standard seems to encourage us to also
> diagnose invalid UTF-8 for non-Windows targets when writing to a
> terminal (and only when writing to a terminal), but I'm reliably
> informed that that wasn't the intent of the wording. Checking for
> invalid UTF-8 sequences only needs to happen for Windows, which is good
> as checking for a terminal requires a call to isatty, and on Linux that
> uses an ioctl syscall, which would make std::print ten times slower!
>
> Testing the std::print behaviour is difficult if it depends on whether
> the output stream is connected to a Windows console or not, as we can't
> (as far as I know) do that non-interactively in DejaGNU. One of the new
> tests uses the internal __write_to_terminal function directly. That
> allows us to verify its UTF-8 error handling on POSIX targets, even
> though that's not actually used by std::print. For Windows, that
> __write_to_terminal function transcodes to UTF-16 but then uses
> WriteConsoleW which fails unless it really is writing to the console.
> That means the 27_io/print/2.cc test FAILs on Windows. The UTF-16
> transcoding has been manually tested using mingw-w64 and Wine, and
> appears to work.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/107760
> * include/Makefile.am: Add new header.
> * include/Makefile.in: Regenerate.
> * include/bits/version.def (__cpp_lib_print): Define.
> * include/bits/version.h: Regenerate.
> * include/std/format (__literal_encoding_is_utf8): New function.
> (_Seq_sink::view()): New member function.
> * include/std/ostream (vprintf_nonunicode, vprintf_unicode)
> (print, println): New functions.
> * include/std/print: New file.
> * src/c++23/Makefile.am: Add new source file.
> * src/c++23/Makefile.in: Regenerate.
> * src/c++23/print.cc: New file.
> * testsuite/27_io/basic_ostream/print/1.cc: New test.
> * testsuite/27_io/print/1.cc: New test.
> * testsuite/27_io/print/2.cc: New test.
> ---
> libstdc++-v3/include/Makefile.am | 1 +
> libstdc++-v3/include/Makefile.in | 1 +
> libstdc++-v3/include/bits/version.def | 9 +
> libstdc++-v3/include/bits/version.h | 29 +-
> libstdc++-v3/include/std/format | 53 +++
> libstdc++-v3/include/std/ostream | 152
> libstdc++-v3/include/std/print| 138 +++
> libstdc++-v3/src/c++23/Makefile.am| 8 +-
> libstdc++-v3/src/c++23/Makefile.in| 10 +-
> libstdc++-v3/src/c++23/print.cc | 348 ++
> .../testsuite/27_io/basic_ostream/print/1.cc | 112 ++
> libstdc++-v3/testsuite/27_io/print/1.cc | 85 +
> libstdc++-v3/testsuite/27_io/print/2.cc | 151
> 13 files changed, 1085 insertions(+), 12 deletions(-)
> create mode 100644 libstdc++-v3/include/std/print
> create mode 100644 libstdc++-v3/src/c++23/print.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/print/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/print/2.cc
>
> diff --git a/libstdc++-v3/include/Makefile.am
> b/libstdc++-v3/include/Makefile.am
> index 17d9d9cec31..368b92eafbc 100644
> --- a/libstdc++-v3/include/Makefile.am
> +++ b/libstdc++-v3/include/Makefile.am
> @@ -85,6 +85,7 @@ std_headers = \
> ${std_srcdir}/memory_resource \
> ${std_srcdir}/mutex \
> ${std_srcdir}/ostream \
> + ${std_srcdir}/print \
> ${std_srcdir}/queue \
> ${std_srcdir}/random \
> ${std_srcdir}/regex \
> diff --git a/libstdc++-v3/include/Makefile.in
> b/libstdc++-v3/include/Makefile.in
> index f038af709cc..a31588c0100 100644
> --- a/libstdc++-v3/include/Makefile.in
> +++ b/libstdc++-v3/include/Makefile.in
> @@ -441,6 +441,7 @@ std_freestanding = \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/memory_resource \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/mutex \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/ostream \
> +@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/print \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/queue \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/random \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcd
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Fri, Dec 15, 2023 at 4:43 AM Jonathan Wakely wrote:
> On Fri, 15 Dec 2023 at 01:17, Tim Song wrote:
> >
> > On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely
> wrote:
> >> + inline void
> >> + vprint_unicode(ostream& __os, string_view __fmt, format_args __args)
> >> + {
> >> +ostream::sentry __cerb(__os);
> >> +if (__cerb)
> >> + {
> >> +
> >> + const streamsize __w = __os.width();
> >> + const bool __left
> >> + = (__os.flags() & ios_base::adjustfield) == ios_base::left;
> >
> >
> > I'm pretty sure - when I wrote this wording anyway - that the intent was
> that it was just an unformatted write at the end. The wording in
> [ostream.formatted.print] doesn't use the "determines padding" words of
> power that would invoke [ostream.formatted.reqmts]/3.
>
> Ah, OK. I misunderstood "formatted output function" as implying
> padding, failing to notice that we need those words of power to be
> present. My thinking was that if the stream has padding set in its
> format flags, it could be surprising if they're ignored by a formatted
> output function. And padding in the format string applies to
> individual replacement fields, not the whole string, and it's hard to
> use the stream's fill character and alignment.
>
But we would get none of the Unicode-aware padding logic we
do in format, which puts it in a very weird place.
And for cases where Unicode is not a problem, it's easy to get padding
with just os << std::format(...);
> You can do this to use the ostream's width:
>
> std::print("{0:{1}}", std::format(...), os.width());
>
> But to reuse its fill char and adjustfield you need to do something
> awful like I did in the committed code:
>
> std::string_view align;
> if (os.flags() & ios::adjustfield) == ios::right)
> align = ">"
> auto fs = std::format("{{:{}{}{}}}", os.fill(), align, os.width());
> std::vprint_nonunicode(os, fs, std::make_args(std::format(...)));
> And now you have to hardcode a choice between vprint_unicode and
> vprint_nonunicode, instead of letting std::print decide it. Let's hope
> nobody ever needs to do any of that ;-)
>
At least the upcoming runtime_format alleviates that :)
>
> I'll remove the code for padding the padding, thanks for checking the
> patch.
>
>
Re: [committed] libstdc++: P1964R2 Wording for boolean-testable
On Mon, Feb 17, 2020 at 12:34 PM Jonathan Wakely wrote: > This removes the complicated std::boolean concept, as agreed in Prague. > > * include/bits/ranges_algo.h (__find_fn, __find_first_of_fn) > (__adjacent_find_fn): Cast result of predicate to bool. Um, why? These look fine without the cast. (The WP was doing == false, which is a different matter.) > * include/std/concepts (__boolean): Remove. > (__detail::__boolean_testable_impl, __detail::__boolean_testable): > Add > new helper concepts. > (__detail::__weakly_eq_cmp_with, totally_ordered, > totally_ordered_with) > (predicate): Use __boolean_testable instead of boolean. > * libsupc++/compare (__detail::__partially_ordered, _Synth3way): > Likewise. > > Tested powerpc64le-linux, committed to master. > >
Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers
On Fri, Jun 2, 2017 at 9:07 AM, Jonathan Wakely wrote: > we aren't qualifying calls to __ref_cast, Turns out this is fine. I somehow managed to completely misread the last sentence of [basic.lookup.argdep]/2. It's talking about the case where an argument to the call is a set of overloaded functions named by a template-id, not about the postfix-expression. At least qualifying those calls doesn't hurt either. Sorry :(
Re: std::advance istreambuf_iterator overload
On Mon, Nov 13, 2017 at 3:32 PM, Daniel Krügler wrote: > but as Jonathan already said, for std::istreambuf_iterator this can > never be true (because of the involved IO operations). > Except, of course, if you advance by zero. It's not a very useful case, for sure...
Re: [PATCH] PR libstdc++/83626 Don't throw for remove("") and remove_all("")
What if the file to be removed is externally removed between the symlink_status and the ::remove call? This is probably QoI because filesystem race, but it seems reasonable to double check errno if ::remove fails and not fail if the failure is due to the file not existing.
Re: [PATCH] P0935R0 Eradicating unnecessarily explicit default constructors
Since param_type's constructors are defined by reference to the constructors of the distribution, P0935's changes to the distribution's constructors apply to their param_type as well.
Re: [PATCH] PR preprocessor/84517 allow double-underscore macros after string literals
On Tue, Feb 27, 2018 at 9:41 AM, Jonathan Wakely wrote: > Since the fix for PR c++/80955 any suffix on a string literal that > begins with an underscore is assumed to be a user-defined literal > suffix, not a macro. This assumption is invalid for a suffix beginning > with two underscores, because such names are reserved and can't be used > for UDLs anyway. [lex.name]/3 reserves all identifiers containing double underscore, but the spaceless one-token form does not actually use such an identifier. See the (equally reserved) _Bq example in [over.literal]/8: double operator""_Bq(long double); // OK: does not use the reserved identifier _Bq ([lex.name]) double operator"" _Bq(long double); // uses the reserved identifier _Bq ([lex.name])
Re: [PATCH] LWG 3171: restore stream insertion for filesystem::directory_entry
On Tue, Dec 18, 2018 at 10:57 AM Jonathan Wakely wrote: > * include/bits/fs_dir.h (operator<<): Overload for directory_entry, > as per LWG 3171. > * testsuite/27_io/filesystem/directory_entry/lwg3171.cc: New test. > > Tested x86_64-linux, committed to trunk. > > > The test seems to be missing from the patch.
Re: C++ PATCH for c++/89024 - ICE with incomplete enum type
On Thu, Jan 24, 2019 at 4:14 PM Jason Merrill wrote: > > On 1/24/19 2:16 PM, Marek Polacek wrote: > > This test ICEs since r159006 which added > > > > type = ENUM_UNDERLYING_TYPE (type); > > > > to type_promotes_to. In this test ENUM_UNDERLYING_TYPE is null because we > > haven't yet parsed '}' of the enum and the underlying type isn't fixed, and > > so checking TYPE_UNSIGNED crashed. > > > > I've added some checks to the test to see if the types seem to be OK; > > clang++ > > agrees. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8/7? > > > > 2019-01-24 Marek Polacek > > > > PR c++/89024 - ICE with incomplete enum type. > > * cvt.c (type_promotes_to): Check if prom is non-null. > > 9.6/6: An enumeration whose underlying type is not fixed is an > incomplete type from its point of declaration to immediately after the > closing } of its enum-specifier, at which point it becomes a complete type. > > So the conversion is ill-formed. > > Jason But the conversion in the example (in decltype(__test_aux<_To1>(declval<_From1>( is in a SFINAE context, so shouldn't it gracefully fall back to the `(...)` overload?
Re: [PATCH] Simplify _Node_insert_return to avoid including
Um, why are those member get's there at all (and with an index mapping that doesn't agree with the member order)? [container.insert.return] says that "It has no base classes or members other than those specified."
Re: [PATCH] PR libstdc++/79162 ambiguity in string assignment due to string_view overload (LWG 2946)
On Fri, Jul 28, 2017 at 4:10 PM, Daniel Krügler
wrote:
> + // Performs an implicit conversion from _Tp to __sv_type.
> + template
> +static __sv_type _S_to_string_view(const _Tp& __svt)
> +{
> + return __svt;
> +}
I might have gone for
+static __sv_type _S_to_string_view(__sv_type __svt) noexcept
+{
+ return __svt;
+}
With that, we can also use noexcept(_S_to_string_view(__t)) to make up
for the absence of is_nothrow_convertible (basically the same thing I
did in LWG 2993's PR).
Tim
Re: [PATCH] libstdc++: Support std::is_aggregate on clang++ (was [cfe-dev] clang++: std::is_aggregate unusable with clang-5.0/libstdc++-7)
On Mon, Jul 31, 2017 at 10:53 AM, Jonathan Wakely wrote: > On 27/07/17 16:27 +0900, Katsuhiko Nishimra wrote: >> >> From 56c4a18d0d8c8ce7aa1239880138775e4db06645 Mon Sep 17 00:00:00 2001 >> From: Katsuhiko Nishimra >> Date: Thu, 27 Jul 2017 16:03:54 +0900 >> Subject: [PATCH] libstdc++: Support std::is_aggregate on clang++ >> >> Currently, libstdc++ tries to detect __is_aggregate built-in macro using >> __has_builtin, but this fails on clang++ because __has_builtin on >> clang++ detects only built-in functions, not built-in macros. This patch >> adds a test using __is_identifier. Tested on clang++ >> 5.0.0-svn308422-1~exp1 and g++ 7.1.0-10 from Debian unstable. >> --- >> libstdc++-v3/include/std/type_traits | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/libstdc++-v3/include/std/type_traits >> b/libstdc++-v3/include/std/type_traits >> index 390b6f40a..e7ec402fb 100644 >> --- a/libstdc++-v3/include/std/type_traits >> +++ b/libstdc++-v3/include/std/type_traits >> @@ -2894,6 +2894,11 @@ template >> >> #if __GNUC__ >= 7 >> # define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1 >> +#elif defined(__is_identifier) >> +// For clang >> +# if ! __is_identifier(__is_aggregate) >> +# define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1 >> +# endif >> #elif defined __has_builtin >> // For non-GNU compilers: >> # if __has_builtin(__is_aggregate) > > > This __has_bultin check only exists for Clang, so should be replaced > by the correct __is_identifier check, not left there in addition to > it. > > https://clang.llvm.org/docs/LanguageExtensions.html#checks-for-type-trait-primitives seems to suggest using __has_extension instead.
Re: [PATCH] libstdc++: Support std::is_aggregate on clang++ (was [cfe-dev] clang++: std::is_aggregate unusable with clang-5.0/libstdc++-7)
On Mon, Jul 31, 2017 at 11:13 AM, Tim Song wrote: > > https://clang.llvm.org/docs/LanguageExtensions.html#checks-for-type-trait-primitives > seems to suggest using __has_extension instead. Hmm, that doesn't work. Oh well.
Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")
On Fri, Aug 4, 2017 at 1:32 PM, Paolo Carlini wrote:
> About the exact wording, I'm a little puzzled, because, besides the
> "implicit" nit, the "copy-inizialization" already occurs in two other places
> in that function, in the preceding:
>
> if (elided && !cands)
> {
> error ("cannot deduce template arguments for copy-initialization"
> " of %qT, as it has no non-explicit deduction guides or "
> "user-declared constructors", type);
> return error_mark_node;
> }
>
> and in the following:
>
> if (elided)
> inform (input_location, "explicit deduction guides not considered "
> "for copy-initialization");
>
> and now I'm wondering which, if any, should be also removed?!? Both? Can you
> help me about that?
>
> Thanks,
> Paolo.
These messages are only emitted when elided == true, which, if I'm
reading the code correctly, is only the case when you are in
copy-initialization context (and have skipped at least one explicit
guide).
Re: [PATCH] istream_iterator: unexpected read in ctor
On Thu, Aug 24, 2017 at 4:55 AM, Petr Ovtchenkov wrote: > istream_iterator do unexpected read from stream > when initialized by istream&. > This is pretty much required by the specification. See the discussion in http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0738r0.html And the "fix" is also wrong because it makes operator* not thread-safe, which it is required to be (as a const member function).
Re: [wwwdocs] Update reference to MMIX
On Sat, Sep 9, 2017 at 2:34 AM, Gerald Pfeifer wrote: > + href="http://www-cs-faculty.stanford.edu/~knuth/mmix.htmll";>MMIX This one has one too many l's.
Re: [C++ PATCH] OVERLOAD iterators #1
On Tue, May 16, 2017 at 2:50 PM, Martin Sebor wrote: > I'm not sure I understand why the ovl_iterator assignment needs > to be provided but if it does, not also defining one on the derived > class will call the base and return a reference to the base, making > the result no longer suitable where the derived is needed. This > is true for any other base members that return [a reference to] > the base type. > Huh? The implicitly declared copy assignment operator in a derived class will always hide the assignment operators from the base class. Also, operator bool() seems suspect. Consider the safe bool idiom? And is it intended that tree implicitly converts to both iterators, or should those constructors be explicit?
Re: [C++ PATCH] OVERLOAD iterators #1
On Tue, May 16, 2017 at 3:24 PM, Nathan Sidwell wrote: > On 05/16/2017 03:20 PM, Tim Song wrote: > >> Also, operator bool() seems suspect. Consider the safe bool idiom? > > ? https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool I suspect that we don't want adding two ovl_iterators together to compile.
Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor
On Mon, May 22, 2017 at 9:21 AM, Jonathan Wakely wrote: > On 19/05/17 22:40 -0700, Tim Shen via libstdc++ wrote: >> >> diff --git a/libstdc++-v3/include/std/variant >> b/libstdc++-v3/include/std/variant >> index 0e04a820d69..b9824a5182c 100644 >> --- a/libstdc++-v3/include/std/variant >> +++ b/libstdc++-v3/include/std/variant >> @@ -936,9 +936,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = >> default; >> >> template> + typename = enable_if_t, variant>>, >>typename = >> enable_if_t<__exactly_once<__accepted_type<_Tp&&>> >> - && is_constructible_v<__accepted_type<_Tp&&>, >> _Tp&&> >> - && !is_same_v, variant>>> >> + && is_constructible_v<__accepted_type<_Tp&&>, >> _Tp&&>>> > > > Does this definitely short-circuit? I seem to recall a similar case > where either Clang or GCC (I think it was Clang) was evaluating the > second default template argument even though the first had produce a > substition failure. > > If we need to guarantee it short-circuits then we'd want: > > templatetypename = enable_if_t<__and_< > __not_, variant>>, > __bool_constant< >__exactly_once<__accepted_type<_Tp&&>> >&& > is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>>> > > i.e. use __and_< is-this-type, everything-else> where > "everything-else" still uses && to avoid making the instantiations too > deep. > > Also, this is another place where we could use an __is_samey > trait that does is_same, U>. > The thing that needs to be short-circuited out is the evaluation of __accepted_type<_Tp&&>, which starts the tower of turtles. The original patch does that (assuming core issue 1227's resolution), but the __and_ version doesn't; __and_ only short circuits the immediate parameter, not things used in forming it.
Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor
On Mon, May 22, 2017 at 4:14 PM, Tim Song wrote: > assuming core issue 1227's resolution Actually, 1227 doesn't touch default template arguments :( OTOH, the paragraph dealing with default template arguments seems to be full of issues - it says "invalid type" rather than "invalid type or expression", and "above" when the description is actually "below". Anyway, that should be easily fixable by moving the SFINAE into the type of a non-type parameter, I think.
Re: [PATCH] Add header implementation of std::to_string for integers (PR libstdc++/71108)
libstdc++ has a to_chars implementation already. Assuming that the locale issue isn't a problem, can that be reused?
Re: Optimisation of std::binary_search of the header
This is not a patch. Nor is it an implementation of std::binary_search, which is a function template that accepts arbitrary forward iterators and optionally arbitrary comparator objects, rather than just pointers. If you want to find someone to review your code, consider something like https://codereview.stackexchange.com/. While I'm not a maintainer, I strongly suspect that for something like this to have a chance of getting into libstdc++, it needs to, at the very minimum, implement the standard signature and be accompanied by sufficient performance benchmarks showing that it is better performing than the existing implementation.
Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers
On Fri, Jun 2, 2017 at 3:19 PM, Tim Shen wrote: > On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote: >> As the PR points out, we aren't qualifying calls to __ref_cast, and >> have 'constexpr' on function templates that can never be usable in >> constant expressions. > > Apology for the constexpr trolling, but that was not intentional. :) > > I'm curious why no tests break. Is it because constexpr in a template > function is a no-op instead of a hard error, when the function > definition is not constexpr? > > The patch looks good. > A non-template, non-default constexpr function that can never be used in a constant expression is ill-formed NDR. ([dcl.constexpr]/5) A constexpr function template for which no specialization that satisfies the requirements for a constexpr function (when considered as a non-template) can be generated is ill-formed NDR. ([dcl.constexpr]/6) It's not really clear to me whether the second rule incorporates the first or if it's just talking about the requirements in [dcl.constexpr]/3, but regardless it's not required to break.
Re: [PATCH] PR77528 add default constructors for container adaptors
On Tue, Jan 10, 2017 at 12:33 PM, Jonathan Wakely wrote:
> The standard says that the container adaptors have a constructor with
> a default argument, which serves as a default constructor. That
> involves default-constructing the underlying sequence as the default
> argument and then move-constructing the member variable from that
> argument. Because std::deque allocates memory in its move constructor
> this means the default constructor of an adaptor using std::deque
> will allocate twice, which is wasteful and expensive.
>
> This change adds a separate default constructor, defined as defaulted
> (and adding default member-initializers to ensure the member variables
> get value-initialized). This avoids the move-construction, so we only
> allocate once when using std::deque.
>
The new default member initializers use {}, and it's not too hard to find
test cases where {} and value-initialization do different things, including
cases where {} doesn't compile but () does. (So much for "uniform"
initialization.)
> Because the default constructor is defined as defaulted it will be
> deleted when the underlying sequence isn't default constructible,
That's not correct. There's no implicit deletion due to the presence of DMIs.
The reason the explicit instantiation works is that constructors explicitly
defaulted at their first declaration are not implicitly defined until ODR-used.
So, unlike the SFINAE-based approach outlined in the bugzilla issue, this
patch causes is_default_constructible>
to trigger a hard error (though the standard's version isn't SFINAE-friendly
either).
Also, the change to .../priority_queue/requirements/explicit_instantiation/1.cc
adds a Cmp class but doesn't actually use it in the explicit instantiation.
Re: [PATCH] PR77528 add default constructors for container adaptors
On Wed, Jan 11, 2017 at 7:21 AM, Jonathan Wakely wrote: > This patch uses the _Enable_default_constructor mixin to properly > delete the default constructors. It's a bit cumbersome, because we > have to add an initializer for the base class to every > ctor-initializer-list, but I think I prefer this to making the default > constructor a constrained template. > I'm happy with either approach - my primary concern is making sure that is_constructible and friends work and don't lie, in a world where increasing numbers of library components depend on it. Though I'm a bit curious as to why you found this approach more preferable. Re the new DMI, my brain compiler says that _Sequence c = _Sequence(); breaks anything with an explicit copy/move constructor pre-C++17, but I also don't think we care about those, right?
Re: [PATCH] PR77528 add default constructors for container adaptors
On Wed, Jan 11, 2017 at 8:30 AM, Jonathan Wakely wrote: >>> Re the new DMI, my brain compiler says that _Sequence c = _Sequence(); >>> breaks anything with an explicit copy/move constructor pre-C++17, but >>> I also don't think we care about those, right? >> >> >> I dislike them, > > > I meant to add "but we try to support them where plausible". > > If the standard requires CopyConstructible then we don't need to care > about explicit copy constructors. But if it only requires > is_copy_constructible then that does work with explicit copy ctors. > And if it says one thing, but means the other, then we just have to > guess what's intended! :-) > > Clause 23 is ... not a model of clarity. It depends on how strongly you read the "any sequence container" phrasing, I suppose. Table 83 requires X u = a; to work for containers, but it also requires a == b to work. There's also the problem of Compare (which I don't see any requirement w/r/t CopyConstructible and like on). It does say things like "initializes comp with x", but doesn't say what kind of initialization...
Re: [v3 PATCH] PR libstdc++/78389
On Thu, Jan 12, 2017 at 8:11 PM, Ville Voutilainen wrote: > This patch doesn't try to fix the reported sort() issue, because > a) it would require undoing a throwing move operation, which > is impossible. > b) in order to avoid the throwing move, we would need to add a > level of indirection and the scratch space for the indirect data > would need to be allocated. Wait, what throwing move? list::sort should be all splicing and no moving, unless I missed something.
Re: [v3 PATCH] PR libstdc++/78389
On Fri, Jan 13, 2017 at 1:39 AM, Ville Voutilainen wrote: > On 13 January 2017 at 08:01, Tim Song wrote: >> On Thu, Jan 12, 2017 at 8:11 PM, Ville Voutilainen >> wrote: >>> This patch doesn't try to fix the reported sort() issue, because >>> a) it would require undoing a throwing move operation, which >>> is impossible. >>> b) in order to avoid the throwing move, we would need to add a >>> level of indirection and the scratch space for the indirect data >>> would need to be allocated. >> >> Wait, what throwing move? list::sort should be all splicing and no >> moving, unless I missed something. > > It operates based on merge, which moves elements from one list to > another using a throwing > comparator. Undoing that operation is fairly tricky, because I don't > know where the merged > items landed. Splice is another move operation, but in case of splice, > I would know where > the items land, and it also doesn't throw, but merge does. But it must be in either the source or the destination, so any missing elements have to be in one of the 65 temporary lists. Is there a problem with just going through all of them and splicing the contents (if any) back to *this?
Re: [v3 PATCH] PR libstdc++/78389
On Fri, Jan 13, 2017 at 3:00 AM, Ville Voutilainen wrote: > On 13 January 2017 at 09:56, Ville Voutilainen > wrote: >>> problem with just going through all of them and splicing the contents >>> (if any) back to *this? >> >> Well, in addition to the computational complexity of it, not knowing >> which elements should be spliced >> back where. If a comparator given to sort() throws, trying to "unsort" >> with the same comparator >> can also throw, so I don't know how to reverse the operations done by >> that point. > > Ah, I think I see what you're saying. Just splice them back in any > order. Ok, I'll give that a spin. Right, list::sort doesn't promise strong exception safety, so "unsorting" is not needed. Also, shouldn't merge() rethrow the caught exception rather than swallow it?
Re: [v3 PATCH] PR libstdc++/78389
On Fri, Jan 13, 2017 at 3:27 AM, Ville Voutilainen wrote: > On 13 January 2017 at 10:09, Ville Voutilainen > wrote: Ah, I think I see what you're saying. Just splice them back in any order. Ok, I'll give that a spin. >>> >>> Right, list::sort doesn't promise strong exception safety, so >>> "unsorting" is not needed. >>> >>> Also, shouldn't merge() rethrow the caught exception rather than swallow it? >> >> Ha, yes, well spotted. I'll cook up an improved patch. > > Thus: > > 2017-01-13 Ville Voutilainen > > PR libstdc++/78389 > * include/bits/list.tcc (merge(list&&)): > Adjust list sizes if the comparator throws. > (merge(list&&, _StrictWeakOrdering)): Likewise. > (sort()): Splice elements back from the scratch buffers > if the comparator throws. > (sort(_StrictWeakOrdering)): Likewise. > * testsuite/23_containers/list/operations/78389.cc: New. Shouldn't __carry be spliced back as well?
Re: [v3 PATCH] PR libstdc++/78389
On Fri, Jan 13, 2017 at 9:26 AM, Ville Voutilainen
wrote:
> Update patch with splices for __carry added. Hopefully this resolves
> the remaining concerns that we had.
On rereading the patch today, the size calculation for merge() appears
to be backwards. [__first2, __last2) consists of the nodes not
transferred into *this, so the new size of __x should be __dist while
this->size() should be incremented by (__orig_size - __dist).
While the original test case in the patch passes (because __dist
happens to be 4 and __orig_size happens to be 8), I see the following
minimally modified test case failing on Wandbox:
std::list a{1, 2, 3, 4};
std::list b{5, 6, 7, 8, 9, 10, 11, 12};
try {
a.merge(b, ThrowingComparator{4});
} catch (...) {
}
assert(a.size() == std::distance(a.begin(), a.end()) && b.size() ==
std::distance(b.begin(), b.end()));
Sorry for not spotting this earlier.
Re: [PATCH] PR64903 fix number of predicate tests in std::is_partitioned
On Thu, Jan 19, 2017 at 6:33 PM, Jonathan Wakely wrote: > std::advance(__first, 1); Why not just ++__first;?
Re: [v3 PATCH] PR libstdc++/78420
On Sun, Jan 22, 2017 at 10:42 AM, Ville Voutilainen
wrote:
> + _GLIBCXX14_CONSTEXPR
> + bool
> + operator()(_Tp* __x, _Tp* __y) const
> + { return uintptr_t(__x) > uintptr_t(__y); }
These reinterpret_casts can't possibly be constexpr.
Simple test case:
#include
constexpr bool g() {
int a[10] = {};
return std::greater{}(a, a+5);
}
static_assert(!g(), "");
Re: [Patch] Inline Variables for the Standard Library (p0607r0)
On Thu, Mar 23, 2017 at 1:45 PM, Jonathan Wakely wrote: > + // NOTE: This makes use of the fact that we know how moveable > + // is implemented on pair, and also vector. If the implementation > + // changes this test may begin to fail. Maybe drop this comment?
Re: [PATCH] PR libstdc++/81468 constrain std::chrono::time_point constructor
On Wed, Sep 13, 2017 at 10:55 AM, Jonathan Wakely wrote:
>
> +// DR 1177
> +static_assert(is_constructible, duration>{},
> +"can convert duration with one floating point rep to another");
> +static_assert(is_constructible, duration>{},
> +"can convert duration with integral rep to one with floating point rep");
> +static_assert(!is_constructible, duration>{},
> +"cannot convert duration with floating point rep to one with integral
> rep");
> +static_assert(is_constructible, duration>{},
> +"can convert duration with one integral rep to another");
> +
> +static_assert(!is_constructible, duration>>{},
> +"cannot convert duration to one with different period");
> +static_assert(is_constructible, duration>>{},
> +"unless it has a floating-point representation");
"it" is a little ambiguous here unless you read the next message's
mention of "the original"...
> +static_assert(is_constructible, duration>>{},
> +"or a period that is an integral multiple of the original");
This is backwards: duration is convertible to duration iff P1 is an integral multiple of P2, i.e., if the original's
period is an integral multiple of "its" period.
The static assert only passed because duration was used as the
destination type (presumably because of a copy/paste error).
Tim
Re: [C++ PATCH] C++2A P0386R1 - default member initializers for bit-fields
On Tue, Sep 19, 2017 at 8:54 AM, Jakub Jelinek wrote: > Hi! > > The following patch implements P0386R1 - NSDMIs for bit-fields. It's P0683R1.
Re: [v3 PATCH] Implement LWG 2825, LWG 2756 breaks class template argument deduction for optional.
On Mon, Jan 30, 2017 at 9:36 PM Jonathan Wakely wrote: > > On 30/01/17 13:28 +, Jonathan Wakely wrote: > >On 30/01/17 13:47 +0200, Ville Voutilainen wrote: > >>Tested on Linux-x64. > > > >OK, thanks. > > To be clear: this isn't approved by LWG yet, but I think we can be a > bit adventurous with deduction guides and add them for experimental > C++17 features. Getting more usage experience before we standardise > these things will be good, and deduction guides are very new and > untried. If we find problems we can remove them again, and will have > invaluable feedback for the standards committee. > My brain compiler says that this may cause problems with std::optional o1; std::optional o2 = o1; // wanted optional, deduced optional> Trunk GCC deduces optional, but I don't think it implements P0512R0 yet, which prefers explicit guides to implicit ones before considering partial ordering. This example is very similar to the example in https://timsong-cpp.github.io/cppwp/over.match.best#1.6. Tim
Re: [v3 PATCH] Implement LWG 2825, LWG 2756 breaks class template argument deduction for optional.
On Tue, Jan 31, 2017 at 8:48 AM Ville Voutilainen wrote: > > On 31 January 2017 at 00:41, Ville Voutilainen > wrote: > > I don't actually need to constrain it, I could just add a guide like > > template optional(optional<_Tp>) -> optional<_Tp>; > > However, I'm not convinced I need to. The preference to an explicit > guide is, at least based > on that paper, a tie-breaker rule. If the copy/move constructors are > better matches than the guide, > those should be picked over a guide. Jason? Yes, but they are not "better matches". They are equally good matches after deduction and substitution. The mechanism that selects template void f(const optional&) over template void f(T) given an optional argument is partial ordering, and that's the last tiebreaker in the list, *after* the implicit/explicit guide tiebreaker.
Re: [C++PATCH] c++/79393 virtual base of abstract class
On Wed, Feb 8, 2017 at 10:08 PM, Nathan Sidwell wrote: > > 'potentially constructed subobject' appears to be a term without definition. [special]/5: For a class, its non-static data members, its non-virtual direct base classes, and, if the class is not abstract, its virtual base classes are called its potentially constructed subobjects.
Re: [wwwdocs] Added /gcc-7/porting_to.html
On Tue, Jan 31, 2017 at 1:54 AM, Jonathan Wakely wrote: > after including unrelated headers such as , , , and > or ?
Re: [Patch] Inline Variables for the Standard Library (p0607r0)
On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler wrote: > This patch applies inline to all namespace scope const variables > according to options A and B2 voted in during the Kona meeting, see: > > http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html > > During that work it has been found that std::ignore was not declared > constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773), > which was fixed as well. Just adding constexpr to std::ignore does ~nothing. The assignment operator needs to be made constexpr too; there is really no other reason to use std::ignore.
Re: [Patch] Inline Variables for the Standard Library (p0607r0)
On Sat, Mar 11, 2017 at 3:37 PM, Daniel Krügler wrote: > 2017-03-11 21:23 GMT+01:00 Tim Song : >> On Sat, Mar 11, 2017 at 1:32 PM, Daniel Krügler >> wrote: >>> This patch applies inline to all namespace scope const variables >>> according to options A and B2 voted in during the Kona meeting, see: >>> >>> http://wiki.edg.com/pub/Wg21kona2017/StrawPolls/p0607r0.html >>> >>> During that work it has been found that std::ignore was not declared >>> constexpr (http://cplusplus.github.io/LWG/lwg-defects.html#2773), >>> which was fixed as well. >> >> Just adding constexpr to std::ignore does ~nothing. The assignment >> operator needs to be made constexpr too; there is really no other >> reason to use std::ignore. > > There is nothing in the resolution of the issue (Nor in the discussion > that argues for the change) that says so. Yes, I considered to make > the assignment function template constexpr, but decided against it, > because the wording of the issue doesn't have this requirement). I'm > also aware of a test-case which would show the difference, but again: > This was not what the issue said. In fact I'm in the process to open a > new library issue that should impose that additional requirement. > > - Daniel Well, technically speaking, the issue resolution doesn't even guarantee that the use case mentioned in the issue would compile since the standard says nothing about whether the copy constructor of decltype(std::ignore) is constexpr, or even whether it can be copied at all. Yes, std::ignore is underspecified, but I'm not sure I see the point in waiting; it's a completely conforming change and IMHO the issue's intent is clearly to make std::ignore fully usable in constexpr functions. Also, the only specification we have for std::ignore's semantics is "when an argument in t is ignore, assigning any value to the corresponding tuple element has no effect". I think one can argue that "causes an expression to be non-constant" is an "effect". Re "new library issue", we already have issue 2933. Tim
Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors
Trying again...with a few edits.
> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont
> wrote:
>
> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> struct _Rb_tree_impl : public _Node_allocator
> {
>_Key_compare _M_key_compare;
> - _Rb_tree_node_base _M_header;
> + _Rb_header_node _M_header;
> +#if __cplusplus < 201103L
>size_type _M_node_count; // Keeps track of size of tree.
> +#else
> + size_type _M_node_count = 0; // Keeps track of size of tree.
> +#endif
>
> +#if __cplusplus < 201103L
>_Rb_tree_impl()
> - : _Node_allocator(), _M_key_compare(), _M_header(),
> -_M_node_count(0)
> - { _M_initialize(); }
> + : _M_node_count(0)
> + { }
> +#else
> + _Rb_tree_impl() = default;
> +#endif
The default constructor of the associative containers is required to
value-initialize the comparator (see their synopses in
[map/set/multimap/multiset.overview]).
_Rb_tree_impl() = default; doesn't do that; it default-initializes the
comparator instead.
Tim
Re: [PATCH] 77864 Fix noexcept conditions for map/set default constructors
On Wed, Oct 12, 2016 at 4:36 PM, François Dumont wrote:
> On 10/10/2016 23:01, Tim Song wrote:
>>
>> Trying again...with a few edits.
>>
>>> On Mon, Oct 10, 2016 at 3:24 PM, François Dumont
>>> wrote:
>>>
>>> @@ -602,24 +612,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>> struct _Rb_tree_impl : public _Node_allocator
>>> {
>>> _Key_compare _M_key_compare;
>>> - _Rb_tree_node_base _M_header;
>>> + _Rb_header_node _M_header;
>>> +#if __cplusplus < 201103L
>>> size_type _M_node_count; // Keeps track of size of tree.
>>> +#else
>>> + size_type _M_node_count = 0; // Keeps track of size of tree.
>>> +#endif
>>>
>>> +#if __cplusplus < 201103L
>>> _Rb_tree_impl()
>>> - : _Node_allocator(), _M_key_compare(), _M_header(),
>>> -_M_node_count(0)
>>> - { _M_initialize(); }
>>> + : _M_node_count(0)
>>> + { }
>>> +#else
>>> + _Rb_tree_impl() = default;
>>> +#endif
>>
>>
>> The default constructor of the associative containers is required to
>> value-initialize the comparator (see their synopses in
>> [map/set/multimap/multiset.overview]).
>
> I don't have latest Standard version so can't see the exact word but I find
> quite annoying that the Standard doesn't allow this simple implementation.
The "exact word" is:
map() : map(Compare()) { }
which mandates the comparator be copied from a value-initialized Compare.
The use of () means value-initialization.
>
> I don't know if unodered containers have same kind of requirements for equal
> or hash functors but if so current implementation doesn't do this value
> initialization.
Yes, unordered_meows are required to do that as well. See, e.g.,
https://timsong-cpp.github.io/cppwp/unord.map.cnstr:
unordered_map() : unordered_map(size_type(see below)) { }
explicit unordered_map(size_type n,
const hasher& hf = hasher(),
const key_equal& eql = key_equal(),
const allocator_type& a =
allocator_type());
The default constructor is specified to call the second constructor,
which copies the hasher etc. from a value-initialized object.
Tim
>
> So here is another attempt. This time it simply allows to have noexcept
> condition in one place and closer to where operations are being invoked.
>
> Ok to commit after tests ?
>
> François
>
>>
>> _Rb_tree_impl() = default; doesn't do that; it default-initializes the
>> comparator instead.
>>
>> Tim
>>
>
Re: [PATCH] Fix filesystem::path for iterators with const value_type
On Fri, Oct 28, 2016 at 1:47 PM, Jonathan Wakely wrote: > For some reason the Filesystem library says that you can construct > paths from iterators with value_type that is a possibly const encoded > character type. I don't know why we support const value_type in this > place, when normally that is bogus (even const_iterators have a > non-const value_type, and various algorithms won't compile with const > value_type). It doesn't say that. [path.req]/1 says that "the value type shall be an encoded character type". [path.req]/2 says that the relevant overloads need not be SFINAE'd away (or equivalent) if the value_type is a const encoded character type, but doesn't actually say that they are required to work.
Re: [PATCH] Add noexcept to various basic_string string operations
On Tue, Dec 6, 2016 at 5:43 AM, Jonathan Wakely wrote: > None of these functions can throw, so we should mark them as such. > I've reported this as a defect against C++17, because they should have > been made noexcept after the application of https://wg21.link/p0254r2 Surely the const _CharT* variants are narrow contract? Or was there a policy change here?
Re: [PATCH] Add noexcept to various basic_string string operations
On Tue, Dec 6, 2016 at 6:00 AM, Jonathan Wakely wrote: > Oh, but if you mean what the standard should say, my issue submission > proposed "Throws: Nothing." for the ones with a narrow contract. It > should be in the issue list soon, but I only sent it yesterday. > Yes, I was talking about the standard. Maybe it would have been clearer if I just quoted the second sentence. Of course there's no problem with you strengthening noexcept in your implementation :)
Re: [PATCH] Add std::future_error constructor from future_errc
On Fri, Nov 11, 2016 at 10:39 PM, Jonathan Wakely wrote:
> making the existing not-required-by-the-standard constructor private.
> + public:
> +explicit
> +future_error(error_code __ec)
> +: logic_error("std::future_error: " + __ec.message()), _M_code(__ec)+
> { }
That doesn't look private to me...
Tim
Re: [committed] libstdc++: Add deduction guide for std::ranges::join_view [LWG 3474]
I thought LWG approved the other option in the PR (changing views::join to not use CTAD)? On Mon, Aug 24, 2020 at 10:22 AM Jonathan Wakely via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > This implements the proposed resolution for LWG 3474. > > libstdc++-v3/ChangeLog: > > * include/std/ranges (join_view): Add deduction guide (LWG 3474). > * testsuite/std/ranges/adaptors/join_lwg3474.cc: New test. > > Tested powerpc64le-linux. Committed to trunk. > >
Re: [committed] libstdc++: std::make_signed_t should be ill-formed
On Mon, Oct 10, 2022 at 8:09 AM Patrick Palka via Libstdc++
wrote:
>
> On Mon, 10 Oct 2022, Jonathan Wakely via Libstdc++ wrote:
>
> > Tested powerpc64le-linux. Pushed to trunk.
> >
> > -- >8 --
> >
> > Currently we only reject std::make_signed_t but not cv bool.
> > Similarly for std::make_unsigned_t.
> >
> > As well as making those ill-formed, this adds a requires-clause to the
> > make_signed and make_unsigned primary templates. This makes
> > non-integral, non-enum cases fail immediately with a clear error, rather
> > than giving an error about __make_signed_selector being
> > incomplete.
>
> IIUC the requires-clause turns what was once a hard error into a SFINAE
> error, so e.g. for
>
> template typename make_signed::type f(int);
> template void f(...);
> int main() { f(0); }
>
> the call to f would previously be rejected due to an error outside the
> immediate context about incomplete __make_signed_selector, and now with
> the requires-clause resolves to the second overload. I wonder if this
> new behavior is conforming -- the examples in [structure.specifications]
> of how to implement 'Mandates' suggest that a failed 'Mandates' should
> yield a hard error?
I'm also concerned about the inability to name make_signed in a
context that doesn't require its instantiation (e.g.,
conditional_t, make_signed,
type_identity>::type). That seems a plausible use case, and
breaking it doesn't seem great to me (conformance aside).
Re: [PATCH] libstdc++: Avoid hard error in ranges::unique_copy [PR100770]
I noticed that output_iterator_wrapper still has a (non-void)
value_type. Perhaps we can get better coverage if it doesn't have one?
The existing tests should have caught this case with that change, at least.
On Wed, May 26, 2021 at 12:00 PM Patrick Palka via Libstdc++
wrote:
>
> - else if constexpr (input_iterator<_Out>
> - && same_as,
> iter_value_t<_Out>>)
> + else if constexpr (requires { requires (input_iterator<_Out>
> + &&
> same_as,
> +
> iter_value_t<_Out>>); })
It's arguably cleaner to extract this into a concept which can then
also be used in the constraint.
Re: [PATCH] libstdc++: Avoid hard error in ranges::unique_copy [PR100770]
On Wed, May 26, 2021 at 1:43 PM Patrick Palka wrote:
>
> On Wed, 26 May 2021, Tim Song wrote:
> >
> > On Wed, May 26, 2021 at 12:00 PM Patrick Palka via Libstdc++
> > wrote:
> > >
> > > - else if constexpr (input_iterator<_Out>
> > > - && same_as,
> > > iter_value_t<_Out>>)
> > > + else if constexpr (requires { requires (input_iterator<_Out>
> > > + &&
> > > same_as,
> > > +
> > > iter_value_t<_Out>>); })
> >
> > It's arguably cleaner to extract this into a concept which can then
> > also be used in the constraint.
>
> Sounds good, though I'm not sure what name to give to this relatively
> ad-hoc set of requirements. Any suggestions? :)
>
Something along the lines of "__can_reread_output", perhaps?
Re: [PATCH] libstdc++: Avoid hard error in ranges::unique_copy [PR100770]
On Wed, May 26, 2021 at 2:55 PM Jonathan Wakely wrote: > > On Wed, 26 May 2021 at 20:11, Patrick Palka via Libstdc++ > wrote: > > > > On Wed, 26 May 2021, Tim Song wrote: > > > > > I noticed that output_iterator_wrapper still has a (non-void) > > > value_type. Perhaps we can get better coverage if it doesn't have one? > > > The existing tests should have caught this case with that change, at > > > least. > > > > Good point, and I guess it should be fine to make its pointer and > > reference void as well. I'm testing: > > Defining difference_type as void is also OK. C++20 requires (new-style) output iterators to have a valid difference_type too (that requirement comes from weakly_incrementable).
Re: [committed] libstdc++: Implement monadic operations for std::optional (P0798R8)
On Tue, Oct 19, 2021 at 9:05 AM Jonathan Wakely via Gcc-patches
wrote:
>
> +constexpr bool
> +test_copy_elision()
> +{
> + return true;
> +}
> +
> +static_assert( test_copy_elision() );
> +
This isn't much of a test :)
Re: [PATCH] libstdc++: Implement P2328 changes to join_view
On Fri, Apr 30, 2021 at 12:11 PM Patrick Palka via Libstdc++
wrote:
>
> + template
> + _Tp&
> + _M_emplace_deref(const _Iter& __i)
> + {
> + this->reset();
> + return this->emplace(*__i);
> + }
This incurs a move, avoiding of which is the sole reason for
emplace-deref's existence.
> + };
> + }
> +
Re: [PATCH] libstdc++: Fix iterator caching inside range adaptors [PR100479]
On Mon, May 17, 2021 at 11:46 AM Patrick Palka via Libstdc++
wrote:
> constexpr void
> _M_set(const _Range&, const iterator_t<_Range>& __it)
> {
> __glibcxx_assert(!_M_has_value());
> - _M_iter = __it;
> + this->_M_payload._M_payload._M_value = __it;
> + this->_M_payload._M_engaged = true;
> }
>};
This part looks questionable - if we don't have a value then we can't
assign to a nonexistent object.
Also, I believe the offset partial specialization of _CachedPosition
needs a change to invalidate the source on move.
Re: [PATCH] libstdc++: Fix access issues in elements_view::_Sentinel [PR100631]
On Mon, May 17, 2021 at 12:21 PM Patrick Palka via Gcc-patches
wrote:
>
> using _Base = elements_view::_Base<_Const>;
> sentinel_t<_Base> _M_end = sentinel_t<_Base>();
> @@ -3800,7 +3807,7 @@ namespace views::__adaptor
> requires sized_sentinel_for, iterator_t<_Base2>>
> friend constexpr range_difference_t<_Base>
Preexisting, but this one should be _Base2 - we always want to get the
difference type from the iterator being used.
> operator-(const _Sentinel& __x, const _Iterator<_Const2>& __y)
> - { return __x._M_end - __y._M_current; }
> + { return __x._M_distance_from(__y); }
>
Re: [PATCH] libstdc++: Fix iterator caching inside range adaptors [PR100479]
On Mon, May 17, 2021 at 2:59 PM Patrick Palka wrote:
>
> + constexpr _CachedPosition&
> + operator=(_CachedPosition&& __other) noexcept
> + {
> + if (std::__addressof(__other) != this)
I don't think we need this check - self-move-assigning the underlying
view isn't required to be a no-op, so we should still invalidate.
> + {
> + // Propagate the cached offset, but invalidate the source.
> + this->_M_offset = __other._M_offset;
> + __other._M_offset = -1;
> + }
> + return *this;
> + }
Re: [committed] libstdc++: Add std::is_scoped_enum for C++23
On Fri, Mar 19, 2021 at 3:13 PM Jonathan Wakely via Libstdc++
wrote:
>
> Implement this C++23 feature, as proposed by P1048R1.
>
> This implementation assumes that a C++23 compiler supports concepts
> already. I don't see any point in using preprocessor hacks to detect
> compilers which define __cplusplus to a post-C++20 value but don't
> support concepts yet.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/type_traits (is_scoped_enum): Define.
> * include/std/version (__cpp_lib_is_scoped_enum): Define.
> * testsuite/20_util/is_scoped_enum/value.cc: New test.
> * testsuite/20_util/is_scoped_enum/version.cc: New test.
>
> Tested powerpc64le-linux. Committed to trunk.
>
Using __underlying_type breaks for incomplete enumeration types. GCC
doesn't have incomplete scoped enums due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89025 but unscoped ones
exist:
enum E {
x = std::is_scoped_enum_v
};
Re: [committed] libstdc++: Add std::is_scoped_enum for C++23
On Sat, Mar 20, 2021 at 3:58 AM Jonathan Wakely wrote:
>
>
>
> On Sat, 20 Mar 2021, 01:13 Tim Song via Libstdc++,
> wrote:
>>
>> On Fri, Mar 19, 2021 at 3:13 PM Jonathan Wakely via Libstdc++
>> wrote:
>> >
>> > Implement this C++23 feature, as proposed by P1048R1.
>> >
>> > This implementation assumes that a C++23 compiler supports concepts
>> > already. I don't see any point in using preprocessor hacks to detect
>> > compilers which define __cplusplus to a post-C++20 value but don't
>> > support concepts yet.
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> > * include/std/type_traits (is_scoped_enum): Define.
>> > * include/std/version (__cpp_lib_is_scoped_enum): Define.
>> > * testsuite/20_util/is_scoped_enum/value.cc: New test.
>> > * testsuite/20_util/is_scoped_enum/version.cc: New test.
>> >
>> > Tested powerpc64le-linux. Committed to trunk.
>> >
>>
>> Using __underlying_type breaks for incomplete enumeration types. GCC
>> doesn't have incomplete scoped enums due to
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89025 but unscoped ones
>> exist:
>>
>> enum E {
>> x = std::is_scoped_enum_v
>> };
>
>
> Thanks, I'll just use int then. Maybe not until Monday though.
>
>
Using int avoids the hard error, but it appears to give the wrong
answer (presumably because the is_convertible check fails due to E
being incomplete). This may need to be handled explicitly?
Re: [committed] libstdc++: Make ranges CPOs final and not addressable
CPOs are specified as actual semiregular function objects that can be copied and constructed freely, so it seems a bit hostile to make them final/non-addressable? (It's debatable whether the type of a CPO is a type "specified in the C++ standard library" for which [derivation]/4 would apply.) On Tue, Jun 15, 2021 at 1:34 PM Jonathan Wakely via Gcc-patches wrote: > > This restricts the API of the CPOs and other function objects so they > cannot be misused by deriving from them or taking their addresses. > > Signed-off-by: Jonathan Wakely > > libstdc++-v3/ChangeLog: > > * include/bits/ranges_base.h (ranges::begin, ranges::end) > (ranges::cbegin, ranges::cend, ranges::rbeing, ranges::rend) > (ranges::crbegin, ranges::crend, ranges::size, ranges::ssize) > (ranges::empty, ranges::data, ranges::cdata): Make types final. > Add deleted operator& overloads. > (ranges::advance, ranges::distance, ranges::next, ranges::prev): > Likewise. > * testsuite/std/ranges/headers/ranges/synopsis.cc: Replace > ill-formed & expressions with using-declarations. Add checks for > other function objects. > > Tested powerpc64le-linux. Committed to trunk. >
Re: [committed] libstdc++: Implement LWG 2762 for std::unique_ptr::operator*
That example violates http://eel.is/c++draft/unique.ptr.runtime.general#3
On Thu, Jun 24, 2021 at 1:55 PM Patrick Palka via Gcc-patches
wrote:
>
> On Thu, 24 Jun 2021, Jonathan Wakely via Libstdc++ wrote:
>
> > The LWG issue proposes to add a conditional noexcept-specifier to
> > std::unique_ptr's dereference operator. The issue is currently in
> > Tentatively Ready status, but even if it isn't voted into the draft, we
> > can do it as a conforming extensions. This commit also adds a similar
> > noexcept-specifier to operator[] for the unique_ptr partial
> > specialization.
>
> The conditional noexcept added to unique_ptr::operator[] seems to break
> the case where T is complete only after the fact:
>
> struct T;
> extern std::unique_ptr p;
> auto& t = p[1];
> struct T { };
>
> /include/c++/12.0.0/bits/unique_ptr.h: In instantiation of ‘typename
> std::add_lvalue_reference<_Tp>::type std::unique_ptr<_Tp [],
> _Dp>::operator[](std::size_t) co
> nst [with _Tp = A; _Dp = std::default_delete; typename
> std::add_lvalue_reference<_Tp>::type = A&; std::size_t = long unsigned int]’:
> testcase.cc:5:14: required from here
> /include/c++/12.0.0/bits/unique_ptr.h:658:48: error: invalid use of
> incomplete type ‘struct A’
> 658 |
> noexcept(noexcept(std::declval()[std::declval()]))
> | ~~~^
> testcase.cc:3:8: note: forward declaration of ‘struct A’
> 3 | struct A;
> |^
>
> >
> > Also ensure that all dereference operators for shared_ptr are noexcept,
> > and adds tests for the std::optional accessors modified by the issue,
> > which were already noexcept in our implementation.
> >
> > Signed-off-by: Jonathan Wakely
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/shared_ptr_base.h (__shared_ptr_access::operator[]):
> > Add noexcept.
> > * include/bits/unique_ptr.h (unique_ptr::operator*): Add
> > conditional noexcept as per LWG 2762.
> > * testsuite/20_util/shared_ptr/observers/array.cc: Check that
> > dereferencing cannot throw.
> > * testsuite/20_util/shared_ptr/observers/get.cc: Likewise.
> > * testsuite/20_util/optional/observers/lwg2762.cc: New test.
> > * testsuite/20_util/unique_ptr/lwg2762.cc: New test.
> >
> > Tested powerpc64le-linux. Committed to trunk.
> >
> >
