This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: std::vector default default and move constructors


On 15/01/18 22:32 +0100, François Dumont wrote:
On 15/01/2018 13:29, Jonathan Wakely wrote:
On 14/01/18 22:13 +0100, François Dumont wrote:
On 11/01/2018 07:57, Marc Glisse wrote:
On Thu, 11 Jan 2018, François Dumont wrote:

-       void _M_swap_data(_Vector_impl& __x) _GLIBCXX_NOEXCEPT
+       void
+       _M_swap_data(_Vector_impl_data& __x) _GLIBCXX_NOEXCEPT
        {
          std::swap(_M_start, __x._M_start);
          std::swap(_M_finish, __x._M_finish);
          std::swap(_M_end_of_storage, __x._M_end_of_storage);
        }


I don't remember earlier discussions about this patch, but is this piece of code still needed? std::swap(*this, __x) looks like it should work.

This patch has been arround for quite a long time now, initial submission was:

https://gcc.gnu.org/ml/libstdc++/2017-07/msg00060.html

even if I sent several updates since then.

I know, but it doesn't actually fix any bugs so has been low priority
and I've not been able to make time to finish reviewing it.
No problem, it just make the implementation more compiler friendly by leaving as much job as possible to it.

In fact it introduces a serious regression because of this line:

-      vector(vector&& __x) noexcept
-      : _Base(std::move(__x)) { }
+      vector(vector&&) = default;

Consider what happens if we have an allocator that is not
nothrow-move-constructible, which can happen if the allocator has a
noexcept(false) move constructor, or more likely just has no move
constructor but has a noexcept(false) copy constructor:

#include <vector>

template<typename T> struct Alloc : std::allocator<T> {
 Alloc() { }
 Alloc(const Alloc&) { }
 template<typename U> Alloc(const Alloc<U>&) { }
 template<typename U> struct rebind { using other = Alloc<U>; };
};

int main()
{
 static_assert( std::is_nothrow_move_constructible<std::vector<int, Alloc<int>>>::value, "" );
}

The standard says this program must compile, and with current GCC
trunk it does compile.

After your change it fails, because the vector(vector&&) constructor
is not explicitly noexcept, instead it deduces its exception
specification from its base classes, which includes the allocator.

Shouldn't gcc raise a compilation error ? At least if the use explicitely qualified its alloc move constructor with noexcept(true).

I should be more careful when the Standard explicitely qualified constructor, and I might need an up to date version of the Standard too.


The latest draft is always available from https://isocpp.org (on the
left sidebar).

We can still write:

+      vector(vector&&) noexcept = default;

You already pointed to me recently on the istreambuf_iterator copy constructor that I needed to keep the noexcept qualification.

That would still be implicitly noexcept(false) because of the base
class, but now you'd get an error because the explicit exception
specification doesn't match the implicit one.

It can even be added on the _Vector_impl so that vector(vector&&) benefit from it but as you said it is better to make it explicit on the vector move constructor.

One of the move constructors needs to be defined with a function body,
not defaulted, so that it can also be declared noexcept.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]