Adding noexcept-specification on tuple constructors (LWG 2899)
Nina Dinka Ranns
dinka.ranns@gmail.com
Tue Apr 16 17:06:00 GMT 2019
On Tue, 16 Apr 2019 at 15:18, Jonathan Wakely <jwakely@redhat.com> 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 <dinka.ranns@gmail.com>
> >
> > 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_<is_nothrow_constructible<_Elements,_UElements&&>...>::value)
>
> Can this be __nothrow_constructible<_UElements>() ?
It should have been that in the first place. Apologies. Fixed.
>
> > : _Inherited(std::forward<_UElements>(__elements)...) { }
> >
> > template<typename... _UElements, typename
> >@@ -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<T,U&&> is exactly equivalent to is_constructible<T,U>
> 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<typename From, typename To>
> struct is_nothrow_convertible
> : std::integral_constant<bool,
> is_convertible<From, To> && is_nothrow_constructible<Fo, From>>
> { };
>
> 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 only C++2a mode. The semantics of
copy construction changed between C++14 and C++17, the effect of which
is that the is_nothrow_convertible (or its equivalent work around) in
certain cases doesn't evaluate the same before and after C++17. After
discussing this with Ville, we decided to only test C++2a because
that's the target of the library issue and because C++2a provided
std::is_nothrow_convertible so I could do away with my copy conversion
helper functions.
Extending the tests to earlier standards would mean having two sets of
test expectations (one for pre C++17 copy conversion semantics, and
one for C++17 onwards). Would you like me to do that ?
I will address other tests comments when I have an agreement on what
to do with the above issue.
>
>
> >+// 2019-04-10 Nina Dinka Ranns <dinka.ranns@gmail.com>
> >+//
> >+// 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 the file COPYING3. If not see
> >+// <http://www.gnu.org/licenses/>.
> >+
> >+#include <tuple>
> >+#include <testsuite_tr1.h>
> >+#include <utility>
> >+
> >+/* DefaultConstructionTests */
> >+
> >+using namespace __gnu_test;
> >+
> >+bool throwing_ctor_called = false;
> >+
> >+
> >+template<typename T>
> >+bool checkDefaultThrowConstruct()
> >+{
> >+ throwing_ctor_called = false;
> >+ bool deduced_nothrow = std::is_nothrow_constructible<T>::value;
> >+ T t{};
> >+ return throwing_ctor_called != deduced_nothrow;
> >+}
> >+
> >+
> >+typedef std::tuple<int> IT;
> >+typedef std::tuple<const int> CIT;
> >+typedef std::tuple<int&&> RVIT;
> >+typedef std::tuple<int, int> IIT;
> >+typedef std::pair<int, int> IIP;
> >+typedef std::tuple<int, int, int> IIIT;
> >+
> >+namespace DefaultConstructionTests
> >+{
> >+ struct NoexceptDC
> >+ {
> >+ NoexceptDC() noexcept(true){}
> >+ };
> >+
> >+ struct ExceptDC
> >+ {
> >+ ExceptDC() noexcept(false)
> >+ { throwing_ctor_called = true; }
> >+ };
> >+
> >+ struct ExplicitNoexceptDC
> >+ {
> >+ explicit ExplicitNoexceptDC() noexcept(true)
> >+ {}
> >+ };
> >+
> >+ struct ExplicitExceptDC
> >+ {
> >+ explicit ExplicitExceptDC() noexcept(false)
> >+ { throwing_ctor_called = true; }
> >+ };
> >+
> >+ typedef std::tuple<NoexceptDC> NDT;
> >+ typedef std::tuple<ExceptDC> EDT;
> >+ typedef std::tuple<ExplicitNoexceptDC> X_NDT;
> >+ typedef std::tuple<ExplicitExceptDC> X_EDT;
> >+
> >+ typedef std::tuple<NoexceptDC,NoexceptDC> NNDT;
> >+ typedef std::tuple<ExceptDC,ExceptDC> EEDT;
> >+ typedef std::tuple<ExceptDC,NoexceptDC> ENDT;
> >+ typedef std::tuple<ExplicitNoexceptDC,NoexceptDC> X_NNDT;
> >+ typedef std::tuple<ExplicitExceptDC,ExceptDC> X_EEDT;
> >+ typedef std::tuple<ExceptDC,ExplicitNoexceptDC> X_ENDT;
> >+
> >+ typedef std::tuple<long, NoexceptDC, NoexceptDC> LNDNDT;
> >+ typedef std::tuple<long, NoexceptDC, ExceptDC> LNDEDT;
> >+ typedef std::tuple<long, ExplicitNoexceptDC, NoexceptDC> X_LNEDNDT;
> >+ typedef std::tuple<long, ExplicitNoexceptDC, ExceptDC> X_LNEDEDT;
> >+ typedef std::tuple<long, ExplicitExceptDC, ExceptDC> X_LEEDEDT;
> >+
> >+
> >+ /* if it has E in the name, it contains a type that throws when default constructed */
> >+ static_assert(std::is_nothrow_constructible<IT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<NDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<EDT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<X_NDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<X_EDT>::value, "");
> >+
> >+ static_assert(std::is_nothrow_constructible<IIT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<NNDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<EEDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<ENDT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<X_NNDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<X_EEDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<X_ENDT>::value, "");
> >+
> >+ static_assert(std::is_nothrow_constructible<IIIT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<LNDNDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<LNDEDT>::value, "");
> >+ static_assert(std::is_nothrow_constructible<X_LNEDNDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<X_LNEDEDT>::value, "");
> >+ static_assert(!std::is_nothrow_constructible<X_LEEDEDT>::value, "");
> >+
> >+ void Run()
> >+ {
> >+ VERIFY( checkDefaultThrowConstruct<IT>() );
> >+ VERIFY( checkDefaultThrowConstruct<NDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<EDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_NDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_EDT>() );
> >+
> >+ VERIFY( checkDefaultThrowConstruct<IIT>() );
> >+ VERIFY( checkDefaultThrowConstruct<NNDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<EEDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<ENDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_NNDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_EEDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_ENDT>() );
> >+
> >+ VERIFY( checkDefaultThrowConstruct<IIIT>() );
> >+ VERIFY( checkDefaultThrowConstruct<LNDNDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<LNDEDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_LNEDNDT>() );
> >+ VERIFY( checkDefaultThrowConstruct<X_LNEDEDT>() );
> >+ }
> >+}
> >+
> >+
> >+int main()
> >+{
> >+
> >+ DefaultConstructionTests::Run();
> >+
> >+}
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
> >+
>
> These blank lines should go.
>
> >Index: libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc
> >===================================================================
> >--- libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc (nonexistent)
> >+++ libstdc++-v3/testsuite/20_util/tuple/cons/noexcept_specs2.cc (working copy)
> >@@ -0,0 +1,129 @@
> >+// { dg-options { -std=gnu++2a } }
> >+// { dg-do run { target c++2a } }
>
> This one has an empty main() function, so could be a { dg-do compile }
> test instead (and then it doesn't need a main() function). Although if
> you combine all the new test files into one file then it will have a
> non-empty main(), and will need to be { dg-do run }, so this comment
> is just FYI really.
>
>
More information about the Gcc-patches
mailing list