Re: Adding noexcept-specification on tuple constructors (LWG 2899)
noted, thanks :) Best, Nina On Sun, 28 Apr 2019 at 22:46, Jonathan Wakely wrote: > > On 28/04/19 22:44 +0100, Jonathan Wakely wrote: > >On 29/04/19 00:18 +0300, Ville Voutilainen wrote: > >>On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: > >>> > >>>On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: > New diff attached. > >>> > >>>Thanks, this looks great. I think we can apply this as soon as stage 1 > >>>begins (which should be Real Soon Now). > >> > >>Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch > >>in, let there be many more. :) > > > >Thanks, Nina, and Ville for taking care of the commit. > > > >Apologies for not noticing it earlier, but the first line of the > >ChangeLog entry was not formatted correctly. There should be two > >spaces either side of the author's name: > > > >2019-04-28 John Doe > > > >not: > > > >2019-04-28 John Doe > > > >I've committed r270633 to fix it. > > > >Thanks again. > > Oh, and the files in the ChangeLog entry should use paths relative to > the ChangeLog file, so no libstdc++-v3/ prefixes. I missed that in the > review too. > >
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 29/04/19 00:18 +0300, Ville Voutilainen wrote: On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: >New diff attached. Thanks, this looks great. I think we can apply this as soon as stage 1 begins (which should be Real Soon Now). Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch in, let there be many more. :) Thanks, Nina, and Ville for taking care of the commit. Apologies for not noticing it earlier, but the first line of the ChangeLog entry was not formatted correctly. There should be two spaces either side of the author's name: 2019-04-28 John Doe not: 2019-04-28 John Doe I've committed r270633 to fix it. Thanks again.
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 28/04/19 22:44 +0100, Jonathan Wakely wrote: On 29/04/19 00:18 +0300, Ville Voutilainen wrote: On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: New diff attached. Thanks, this looks great. I think we can apply this as soon as stage 1 begins (which should be Real Soon Now). Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch in, let there be many more. :) Thanks, Nina, and Ville for taking care of the commit. Apologies for not noticing it earlier, but the first line of the ChangeLog entry was not formatted correctly. There should be two spaces either side of the author's name: 2019-04-28 John Doe not: 2019-04-28 John Doe I've committed r270633 to fix it. Thanks again. Oh, and the files in the ChangeLog entry should use paths relative to the ChangeLog file, so no libstdc++-v3/ prefixes. I missed that in the review too.
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: > > On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: > >New diff attached. > > Thanks, this looks great. I think we can apply this as soon as stage 1 > begins (which should be Real Soon Now). Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch in, let there be many more. :)
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Mon, 29 Apr 2019 at 00:18, Ville Voutilainen wrote: > > On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: > > > > On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: > > >New diff attached. > > > > Thanks, this looks great. I think we can apply this as soon as stage 1 > > begins (which should be Real Soon Now). > > Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch > in, let there be many more. :) And as I was corrected, this was the second patch, but that doesn't change the suggestion that there should be many more. :P
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: New diff attached. Thanks, this looks great. I think we can apply this as soon as stage 1 begins (which should be Real Soon Now).
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Tue, 23 Apr 2019 at 21:28, Jonathan Wakely wrote: > > On 23/04/19 18:43 +0100, Nina Dinka Ranns wrote: > >On Thu, 18 Apr 2019 at 21:35, Jonathan Wakely wrote: > >> > >> On 16/04/19 17:59 +0100, Nina Dinka Ranns wrote: > >> >On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely wrote: > >> >> > >> >> On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: > >> >> >Tested on Linux-PPC64 > >> >> >Adding noexcept-specification on tuple constructors (LWG 2899) > >> >> > >> >> Thanks, Nina! > >> >> > >> >> This looks great, although as I think Ville has explained we won't > >> >> commit it until the next stage 1, after the GCC 9 release. > >> >ack > >> > > >> >> > >> >> The changes look good, I just have some mostly-stylistic comments, > >> >> which are inline below ... > >> >> > >> >> > >> >> >2019-04-13 Nina Dinka Ranns > >> >> > > >> >> >Adding noexcept-specification on tuple constructors (LWG 2899) > >> >> >* libstdc++-v3/include/std/tuple: > >> >> >(tuple()): Add noexcept-specification. > >> >> >(tuple(const _Elements&...)): Likewise > >> >> >(tuple(_UElements&&...)): Likewise > >> >> >(tuple(const tuple<_UElements...>&)): Likewise > >> >> >(tuple(tuple<_UElements...>&&)): Likewise > >> >> >(tuple(const _T1&, const _T2&)): Likewise > >> >> >(tuple(_U1&&, _U2&&)): Likewise > >> >> >(tuple(const tuple<_U1, _U2>&): Likewise > >> >> >(tuple(tuple<_U1, _U2>&&): Likewise > >> >> >(tuple(const pair<_U1, _U2>&): Likewise > >> >> >(tuple(pair<_U1, _U2>&&): Likewise > >> >> > > >> >> > > >> >> > >> >> There should be no blank lines in the changelog entry here. A single > >> >> change should be recorded as a single block in the changelog, with no > >> >> blank lines within it. > >> >ack. Do you need me to do anything about this or is it for future > >> >reference only ? > >> > >> For future reference. Whoever commits the patch can correct the > >> changelog. > >> > >> >> > >> >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: > >> >> > New > >> >> >* > >> >> > libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: New > >> >> >* > >> >> > libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: New > >> >> >* > >> >> > libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: New > >> >> >* > >> >> > libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: New > >> >> >* > >> >> > libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: New > >> >> > >> >> This is a lot of new test files for a small-ish QoI feature. Could > >> >> they be combined into one file? Generally we do want one test file > >> >> per feature, but I think all of these are arguably testing one feature > >> >> (just on different constructors). The downside of lots of smaller > >> >> files is that we have to compile+assemble+link+run each one, which > >> >> adds several fork()s to launch a new process for each step. On some > >> >> platforms that can be quite slow. > >> >I can do that, but there may be an issue. See below. > >> > > >> >> > >> >> > >> >> >@@ -624,6 +634,7 @@ > >> >> > && (sizeof...(_Elements) >= 1), > >> >> > bool>::type=true> > >> >> > constexpr tuple(_UElements&&... __elements) > >> >> >+ > >> >> >noexcept(__and_...>::value) > >> >> > >> >> Can this be __nothrow_constructible<_UElements>() ? > >> >It should have been that in the first place. Apologies. Fixed. > >> > > >> > > >> >> > >> >> > : _Inherited(std::forward<_UElements>(__elements)...) { } > >> >> > > >> >> > template >> >> >@@ -635,6 +646,7 @@ > >> >> > && (sizeof...(_Elements) >= 1), > >> >> > bool>::type=false> > >> >> > explicit constexpr tuple(_UElements&&... __elements) > >> >> >+noexcept(__nothrow_constructible<_UElements&&...>()) > >> >> > >> >> The && here is redundant, though harmless. > >> >> > >> >> is_constructible is exactly equivalent to is_constructible > >> >> because U means construction from an rvalue of type U and so does U&&. > >> >> > >> >> It's fine to leave the && there though. > >> >I'm happy to go either way. The only reason I used && form is because > >> >it mimics the wording in the LWG resolution. > >> > >> I suspect if STL had reviewed the wording in the resolution he'd have > >> asked for the && to be removed :-) > >:) ack. Removed. > > > > > >> > >> > >> >> >@@ -966,6 +995,7 @@ > >> >> > && !is_same<__remove_cvref_t<_U1>, > >> >> > allocator_arg_t>::value, > >> >> > bool>::type = true> > >> >> > constexpr tuple(_U1&& __a1, _U2&& __a2) > >> >> >+noexcept(__nothrow_constructible<_U1&&,_U2&&>()) > >> >> > >> >> There should be a space after the comma here, and all the later > >> >> additions in the file. > >> >ack. Fixed > >> > > >> >> > >> >> > >> >> >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc >
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 23/04/19 18:43 +0100, Nina Dinka Ranns wrote: On Thu, 18 Apr 2019 at 21:35, Jonathan Wakely wrote: On 16/04/19 17:59 +0100, Nina Dinka Ranns wrote: >On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely wrote: >> >> On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: >> >Tested on Linux-PPC64 >> >Adding noexcept-specification on tuple constructors (LWG 2899) >> >> Thanks, Nina! >> >> This looks great, although as I think Ville has explained we won't >> commit it until the next stage 1, after the GCC 9 release. >ack > >> >> The changes look good, I just have some mostly-stylistic comments, >> which are inline below ... >> >> >> >2019-04-13 Nina Dinka Ranns >> > >> >Adding noexcept-specification on tuple constructors (LWG 2899) >> >* libstdc++-v3/include/std/tuple: >> >(tuple()): Add noexcept-specification. >> >(tuple(const _Elements&...)): Likewise >> >(tuple(_UElements&&...)): Likewise >> >(tuple(const tuple<_UElements...>&)): Likewise >> >(tuple(tuple<_UElements...>&&)): Likewise >> >(tuple(const _T1&, const _T2&)): Likewise >> >(tuple(_U1&&, _U2&&)): Likewise >> >(tuple(const tuple<_U1, _U2>&): Likewise >> >(tuple(tuple<_U1, _U2>&&): Likewise >> >(tuple(const pair<_U1, _U2>&): Likewise >> >(tuple(pair<_U1, _U2>&&): Likewise >> > >> > >> >> There should be no blank lines in the changelog entry here. A single >> change should be recorded as a single block in the changelog, with no >> blank lines within it. >ack. Do you need me to do anything about this or is it for future >reference only ? For future reference. Whoever commits the patch can correct the changelog. >> >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: New >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: New >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: New >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: New >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: New >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: New >> >> This is a lot of new test files for a small-ish QoI feature. Could >> they be combined into one file? Generally we do want one test file >> per feature, but I think all of these are arguably testing one feature >> (just on different constructors). The downside of lots of smaller >> files is that we have to compile+assemble+link+run each one, which >> adds several fork()s to launch a new process for each step. On some >> platforms that can be quite slow. >I can do that, but there may be an issue. See below. > >> >> >> >@@ -624,6 +634,7 @@ >> > && (sizeof...(_Elements) >= 1), >> > bool>::type=true> >> > constexpr tuple(_UElements&&... __elements) >> >+ noexcept(__and_...>::value) >> >> Can this be __nothrow_constructible<_UElements>() ? >It should have been that in the first place. Apologies. Fixed. > > >> >> > : _Inherited(std::forward<_UElements>(__elements)...) { } >> > >> > template> >@@ -635,6 +646,7 @@ >> > && (sizeof...(_Elements) >= 1), >> > bool>::type=false> >> > explicit constexpr tuple(_UElements&&... __elements) >> >+noexcept(__nothrow_constructible<_UElements&&...>()) >> >> The && here is redundant, though harmless. >> >> is_constructible is exactly equivalent to is_constructible >> because U means construction from an rvalue of type U and so does U&&. >> >> It's fine to leave the && there though. >I'm happy to go either way. The only reason I used && form is because >it mimics the wording in the LWG resolution. I suspect if STL had reviewed the wording in the resolution he'd have asked for the && to be removed :-) :) ack. Removed. >> >@@ -966,6 +995,7 @@ >> > && !is_same<__remove_cvref_t<_U1>, allocator_arg_t>::value, >> > bool>::type = true> >> > constexpr tuple(_U1&& __a1, _U2&& __a2) >> >+noexcept(__nothrow_constructible<_U1&&,_U2&&>()) >> >> There should be a space after the comma here, and all the later >> additions in the file. >ack. Fixed > >> >> >> >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc >> >=== >> >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (nonexistent) >> >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (working copy) >> >@@ -0,0 +1,191 @@ >> >+// { dg-options { -std=gnu++2a } } >> >+// { dg-do run { target c++2a } } >> >> This new file doesn't use std::is_nothrow_convertible so could just >> use: { dg-do run { target c++11 } } and no dg-options. >> >> For the other new tests that do use is_nothrow_convertible, I'm >> already planning to add std::is_nothrow_convertible for our internal >> use in C++11, so they could use that. >> >> Alternatively, the test fil
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Thu, 18 Apr 2019 at 21:35, Jonathan Wakely wrote: > > On 16/04/19 17:59 +0100, Nina Dinka Ranns wrote: > >On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely wrote: > >> > >> On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: > >> >Tested on Linux-PPC64 > >> >Adding noexcept-specification on tuple constructors (LWG 2899) > >> > >> Thanks, Nina! > >> > >> This looks great, although as I think Ville has explained we won't > >> commit it until the next stage 1, after the GCC 9 release. > >ack > > > >> > >> The changes look good, I just have some mostly-stylistic comments, > >> which are inline below ... > >> > >> > >> >2019-04-13 Nina Dinka Ranns > >> > > >> >Adding noexcept-specification on tuple constructors (LWG 2899) > >> >* libstdc++-v3/include/std/tuple: > >> >(tuple()): Add noexcept-specification. > >> >(tuple(const _Elements&...)): Likewise > >> >(tuple(_UElements&&...)): Likewise > >> >(tuple(const tuple<_UElements...>&)): Likewise > >> >(tuple(tuple<_UElements...>&&)): Likewise > >> >(tuple(const _T1&, const _T2&)): Likewise > >> >(tuple(_U1&&, _U2&&)): Likewise > >> >(tuple(const tuple<_U1, _U2>&): Likewise > >> >(tuple(tuple<_U1, _U2>&&): Likewise > >> >(tuple(const pair<_U1, _U2>&): Likewise > >> >(tuple(pair<_U1, _U2>&&): Likewise > >> > > >> > > >> > >> There should be no blank lines in the changelog entry here. A single > >> change should be recorded as a single block in the changelog, with no > >> blank lines within it. > >ack. Do you need me to do anything about this or is it for future > >reference only ? > > For future reference. Whoever commits the patch can correct the > changelog. > > >> > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: New > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: > >> > New > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: > >> > New > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: > >> > New > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: > >> > New > >> >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: > >> > New > >> > >> This is a lot of new test files for a small-ish QoI feature. Could > >> they be combined into one file? Generally we do want one test file > >> per feature, but I think all of these are arguably testing one feature > >> (just on different constructors). The downside of lots of smaller > >> files is that we have to compile+assemble+link+run each one, which > >> adds several fork()s to launch a new process for each step. On some > >> platforms that can be quite slow. > >I can do that, but there may be an issue. See below. > > > >> > >> > >> >@@ -624,6 +634,7 @@ > >> > && (sizeof...(_Elements) >= 1), > >> > bool>::type=true> > >> > constexpr tuple(_UElements&&... __elements) > >> >+ > >> >noexcept(__and_...>::value) > >> > >> Can this be __nothrow_constructible<_UElements>() ? > >It should have been that in the first place. Apologies. Fixed. > > > > > >> > >> > : _Inherited(std::forward<_UElements>(__elements)...) { } > >> > > >> > template >> >@@ -635,6 +646,7 @@ > >> > && (sizeof...(_Elements) >= 1), > >> > bool>::type=false> > >> > explicit constexpr tuple(_UElements&&... __elements) > >> >+noexcept(__nothrow_constructible<_UElements&&...>()) > >> > >> The && here is redundant, though harmless. > >> > >> is_constructible is exactly equivalent to is_constructible > >> because U means construction from an rvalue of type U and so does U&&. > >> > >> It's fine to leave the && there though. > >I'm happy to go either way. The only reason I used && form is because > >it mimics the wording in the LWG resolution. > > I suspect if STL had reviewed the wording in the resolution he'd have > asked for the && to be removed :-) :) ack. Removed. > > > >> >@@ -966,6 +995,7 @@ > >> > && !is_same<__remove_cvref_t<_U1>, > >> > allocator_arg_t>::value, > >> > bool>::type = true> > >> > constexpr tuple(_U1&& __a1, _U2&& __a2) > >> >+noexcept(__nothrow_constructible<_U1&&,_U2&&>()) > >> > >> There should be a space after the comma here, and all the later > >> additions in the file. > >ack. Fixed > > > >> > >> > >> >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >> >=== > >> >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >> >(nonexistent) > >> >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >> >(working copy) > >> >@@ -0,0 +1,191 @@ > >> >+// { dg-options { -std=gnu++2a } } > >> >+// { dg-do run { target c++2a } } > >> > >> This new file doesn't use std::is_nothrow_convertible so could just > >> use: { dg-do run { target
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 16/04/19 17:59 +0100, Nina Dinka Ranns wrote: On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely wrote: On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: >Tested on Linux-PPC64 >Adding noexcept-specification on tuple constructors (LWG 2899) Thanks, Nina! This looks great, although as I think Ville has explained we won't commit it until the next stage 1, after the GCC 9 release. ack The changes look good, I just have some mostly-stylistic comments, which are inline below ... >2019-04-13 Nina Dinka Ranns > >Adding noexcept-specification on tuple constructors (LWG 2899) >* libstdc++-v3/include/std/tuple: >(tuple()): Add noexcept-specification. >(tuple(const _Elements&...)): Likewise >(tuple(_UElements&&...)): Likewise >(tuple(const tuple<_UElements...>&)): Likewise >(tuple(tuple<_UElements...>&&)): Likewise >(tuple(const _T1&, const _T2&)): Likewise >(tuple(_U1&&, _U2&&)): Likewise >(tuple(const tuple<_U1, _U2>&): Likewise >(tuple(tuple<_U1, _U2>&&): Likewise >(tuple(const pair<_U1, _U2>&): Likewise >(tuple(pair<_U1, _U2>&&): Likewise > > There should be no blank lines in the changelog entry here. A single change should be recorded as a single block in the changelog, with no blank lines within it. ack. Do you need me to do anything about this or is it for future reference only ? For future reference. Whoever commits the patch can correct the changelog. >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: New >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: New >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: New >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: New >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: New >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: New This is a lot of new test files for a small-ish QoI feature. Could they be combined into one file? Generally we do want one test file per feature, but I think all of these are arguably testing one feature (just on different constructors). The downside of lots of smaller files is that we have to compile+assemble+link+run each one, which adds several fork()s to launch a new process for each step. On some platforms that can be quite slow. I can do that, but there may be an issue. See below. >@@ -624,6 +634,7 @@ > && (sizeof...(_Elements) >= 1), > bool>::type=true> > constexpr tuple(_UElements&&... __elements) >+ noexcept(__and_...>::value) Can this be __nothrow_constructible<_UElements>() ? It should have been that in the first place. Apologies. Fixed. > : _Inherited(std::forward<_UElements>(__elements)...) { } > > template@@ -635,6 +646,7 @@ > && (sizeof...(_Elements) >= 1), > bool>::type=false> > explicit constexpr tuple(_UElements&&... __elements) >+noexcept(__nothrow_constructible<_UElements&&...>()) The && here is redundant, though harmless. is_constructible is exactly equivalent to is_constructible because U means construction from an rvalue of type U and so does U&&. It's fine to leave the && there though. I'm happy to go either way. The only reason I used && form is because it mimics the wording in the LWG resolution. I suspect if STL had reviewed the wording in the resolution he'd have asked for the && to be removed :-) >@@ -966,6 +995,7 @@ > && !is_same<__remove_cvref_t<_U1>, allocator_arg_t>::value, > bool>::type = true> > constexpr tuple(_U1&& __a1, _U2&& __a2) >+noexcept(__nothrow_constructible<_U1&&,_U2&&>()) There should be a space after the comma here, and all the later additions in the file. ack. Fixed >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc >=== >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (nonexistent) >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (working copy) >@@ -0,0 +1,191 @@ >+// { dg-options { -std=gnu++2a } } >+// { dg-do run { target c++2a } } This new file doesn't use std::is_nothrow_convertible so could just use: { dg-do run { target c++11 } } and no dg-options. For the other new tests that do use is_nothrow_convertible, I'm already planning to add std::is_nothrow_convertible for our internal use in C++11, so they could use that. Alternatively, the test files themselves could define: template struct is_nothrow_convertible : std::integral_constant && is_nothrow_constructible> { }; and then use that. That way we can test the exception specs are correct in C++11 mode, the default C++14 mode, and C++17 mode. Otherwise we're adding code that affects all those modes but only testing it works correctly for the experimental C++2a mode. There is a reason
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely wrote: > > On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: > >Tested on Linux-PPC64 > >Adding noexcept-specification on tuple constructors (LWG 2899) > > Thanks, Nina! > > This looks great, although as I think Ville has explained we won't > commit it until the next stage 1, after the GCC 9 release. ack > > The changes look good, I just have some mostly-stylistic comments, > which are inline below ... > > > >2019-04-13 Nina Dinka Ranns > > > >Adding noexcept-specification on tuple constructors (LWG 2899) > >* libstdc++-v3/include/std/tuple: > >(tuple()): Add noexcept-specification. > >(tuple(const _Elements&...)): Likewise > >(tuple(_UElements&&...)): Likewise > >(tuple(const tuple<_UElements...>&)): Likewise > >(tuple(tuple<_UElements...>&&)): Likewise > >(tuple(const _T1&, const _T2&)): Likewise > >(tuple(_U1&&, _U2&&)): Likewise > >(tuple(const tuple<_U1, _U2>&): Likewise > >(tuple(tuple<_U1, _U2>&&): Likewise > >(tuple(const pair<_U1, _U2>&): Likewise > >(tuple(pair<_U1, _U2>&&): Likewise > > > > > > There should be no blank lines in the changelog entry here. A single > change should be recorded as a single block in the changelog, with no > blank lines within it. ack. Do you need me to do anything about this or is it for future reference only ? > > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: New > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: New > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: New > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: New > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: New > >* libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: New > > This is a lot of new test files for a small-ish QoI feature. Could > they be combined into one file? Generally we do want one test file > per feature, but I think all of these are arguably testing one feature > (just on different constructors). The downside of lots of smaller > files is that we have to compile+assemble+link+run each one, which > adds several fork()s to launch a new process for each step. On some > platforms that can be quite slow. I can do that, but there may be an issue. See below. > > > >@@ -624,6 +634,7 @@ > > && (sizeof...(_Elements) >= 1), > > bool>::type=true> > > constexpr tuple(_UElements&&... __elements) > >+ > >noexcept(__and_...>::value) > > Can this be __nothrow_constructible<_UElements>() ? It should have been that in the first place. Apologies. Fixed. > > > : _Inherited(std::forward<_UElements>(__elements)...) { } > > > > template >@@ -635,6 +646,7 @@ > > && (sizeof...(_Elements) >= 1), > > bool>::type=false> > > explicit constexpr tuple(_UElements&&... __elements) > >+noexcept(__nothrow_constructible<_UElements&&...>()) > > The && here is redundant, though harmless. > > is_constructible is exactly equivalent to is_constructible > because U means construction from an rvalue of type U and so does U&&. > > It's fine to leave the && there though. I'm happy to go either way. The only reason I used && form is because it mimics the wording in the LWG resolution. > > > >@@ -966,6 +995,7 @@ > > && !is_same<__remove_cvref_t<_U1>, allocator_arg_t>::value, > > bool>::type = true> > > constexpr tuple(_U1&& __a1, _U2&& __a2) > >+noexcept(__nothrow_constructible<_U1&&,_U2&&>()) > > There should be a space after the comma here, and all the later > additions in the file. ack. Fixed > > > >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >=== > >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >(nonexistent) > >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >(working copy) > >@@ -0,0 +1,191 @@ > >+// { dg-options { -std=gnu++2a } } > >+// { dg-do run { target c++2a } } > > This new file doesn't use std::is_nothrow_convertible so could just > use: { dg-do run { target c++11 } } and no dg-options. > > For the other new tests that do use is_nothrow_convertible, I'm > already planning to add std::is_nothrow_convertible for our internal > use in C++11, so they could use that. > > Alternatively, the test files themselves could define: > > template > struct is_nothrow_convertible > : std::integral_constant is_convertible && is_nothrow_constructible> > { }; > > and then use that. That way we can test the exception specs are > correct in C++11 mode, the default C++14 mode, and C++17 mode. > Otherwise we're adding code that affects all those modes but only > testing it works correctly for the experimental C++2a mode. There is a reason why the tests are
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Tue, 16 Apr 2019 at 17:18, Jonathan Wakely wrote: > >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >(nonexistent) > >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc > >(working copy) > >@@ -0,0 +1,191 @@ > >+// { dg-options { -std=gnu++2a } } > >+// { dg-do run { target c++2a } } > > This new file doesn't use std::is_nothrow_convertible so could just > use: { dg-do run { target c++11 } } and no dg-options. > > For the other new tests that do use is_nothrow_convertible, I'm > already planning to add std::is_nothrow_convertible for our internal > use in C++11, so they could use that. > > Alternatively, the test files themselves could define: > > template > struct is_nothrow_convertible > : std::integral_constant is_convertible && is_nothrow_constructible> > { }; > > and then use that. That way we can test the exception specs are > correct in C++11 mode, the default C++14 mode, and C++17 mode. > Otherwise we're adding code that affects all those modes but only > testing it works correctly for the experimental C++2a mode. That doesn't look right to me. If we go down the path of defining the trait in the tests, let's please copy the implementation of std::is_nothrow_convertible, which properly uses a noexcept-check of a helper function call (which makes the conversion be copy-initialization, since it's on a function parameter). Otherwise, https://wandbox.org/permlink/ALXkVVANdD4Z2AZm
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On 16/04/19 14:08 +0100, Nina Dinka Ranns wrote: Tested on Linux-PPC64 Adding noexcept-specification on tuple constructors (LWG 2899) Thanks, Nina! This looks great, although as I think Ville has explained we won't commit it until the next stage 1, after the GCC 9 release. The changes look good, I just have some mostly-stylistic comments, which are inline below ... 2019-04-13 Nina Dinka Ranns Adding noexcept-specification on tuple constructors (LWG 2899) * libstdc++-v3/include/std/tuple: (tuple()): Add noexcept-specification. (tuple(const _Elements&...)): Likewise (tuple(_UElements&&...)): Likewise (tuple(const tuple<_UElements...>&)): Likewise (tuple(tuple<_UElements...>&&)): Likewise (tuple(const _T1&, const _T2&)): Likewise (tuple(_U1&&, _U2&&)): Likewise (tuple(const tuple<_U1, _U2>&): Likewise (tuple(tuple<_U1, _U2>&&): Likewise (tuple(const pair<_U1, _U2>&): Likewise (tuple(pair<_U1, _U2>&&): Likewise There should be no blank lines in the changelog entry here. A single change should be recorded as a single block in the changelog, with no blank lines within it. * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc: New * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc: New * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs3.cc: New * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs4.cc: New * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs5.cc: New * libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs6.cc: New This is a lot of new test files for a small-ish QoI feature. Could they be combined into one file? Generally we do want one test file per feature, but I think all of these are arguably testing one feature (just on different constructors). The downside of lots of smaller files is that we have to compile+assemble+link+run each one, which adds several fork()s to launch a new process for each step. On some platforms that can be quite slow. @@ -624,6 +634,7 @@ && (sizeof...(_Elements) >= 1), bool>::type=true> constexpr tuple(_UElements&&... __elements) + noexcept(__and_...>::value) Can this be __nothrow_constructible<_UElements>() ? : _Inherited(std::forward<_UElements>(__elements)...) { } template= 1), bool>::type=false> explicit constexpr tuple(_UElements&&... __elements) +noexcept(__nothrow_constructible<_UElements&&...>()) The && here is redundant, though harmless. is_constructible is exactly equivalent to is_constructible because U means construction from an rvalue of type U and so does U&&. It's fine to leave the && there though. @@ -966,6 +995,7 @@ && !is_same<__remove_cvref_t<_U1>, allocator_arg_t>::value, bool>::type = true> constexpr tuple(_U1&& __a1, _U2&& __a2) +noexcept(__nothrow_constructible<_U1&&,_U2&&>()) There should be a space after the comma here, and all the later additions in the file. Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc === --- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (nonexistent) +++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs.cc (working copy) @@ -0,0 +1,191 @@ +// { dg-options { -std=gnu++2a } } +// { dg-do run { target c++2a } } This new file doesn't use std::is_nothrow_convertible so could just use: { dg-do run { target c++11 } } and no dg-options. For the other new tests that do use is_nothrow_convertible, I'm already planning to add std::is_nothrow_convertible for our internal use in C++11, so they could use that. Alternatively, the test files themselves could define: template struct is_nothrow_convertible : std::integral_constant && is_nothrow_constructible> { }; and then use that. That way we can test the exception specs are correct in C++11 mode, the default C++14 mode, and C++17 mode. Otherwise we're adding code that affects all those modes but only testing it works correctly for the experimental C++2a mode. +// 2019-04-10 Nina Dinka Ranns +// +// Copyright (C) 2017 Free Software Foundation, Inc. Copyright date on new files should be 2019. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this library; see