Bug #2966
closedMove constructors and assignment operators should be declared noexcept
100%
Description
While looking at some code I noticed that we have move constructors that are not declared noexcept, which in general is a bad thing.
From a quick search in ndn-cxx I've found:
src/util/signal-scoped-connection.hpp: ScopedConnection(ScopedConnection&& other);
src/util/scheduler-scoped-event-id.hpp: ScopedEventId(ScopedEventId&& other);
There might be more.
From 12.5.4 Declare noexcept the move constructor and move assignment operator:
If the move constructor for an element type in a container is not
noexceptthen the container will use the copy constructor rather than the move constructor.
This loses most benefits of defining move constructor and move assignment operator.
Updated by Junxiao Shi over 10 years ago
- Estimated time set to 1.50 h
Agreed.
We need to make this a code-style rule, so that there won't be violations in the future.
From 12.5.4 Declare noexcept the move constructor and move assignment operator:
If the move constructor for an element type in a container is not
noexceptthen the container will use the copy constructor rather than the move constructor.
This loses most benefits of defining move constructor and move assignment operator.
Updated by Davide Pesavento over 10 years ago
Junxiao Shi wrote:
This loses most benefits of defining move constructor and move assignment operator.
Yep, this is the main reason why I said lack of noexcept is bad, in turn caused by the widespread usage of move_if_noexcept in the standard library.
Updated by Junxiao Shi over 10 years ago
- Category set to Utils
- Assignee set to Junxiao Shi
- Estimated time changed from 1.50 h to 1.00 h
Updated by Junxiao Shi over 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
http://gerrit.named-data.net/2179
I have upgraded std::is_move_constructible and std::is_move_assignable to std::is_nothrow_move_constructible and std::is_nothrow_move_assignable.
For signal::ScopedConnection and scheduler::ScopedEventId, noexcept is added to move constructor, but they are designed to be not MoveAssignable.
Updated by Junxiao Shi over 10 years ago
- Status changed from Code review to Closed
Updated by Davide Pesavento over 10 years ago
Have you checked that there aren't other instances of missing noexcept in the codebase?
Updated by Junxiao Shi over 10 years ago
Have you checked that there aren't other instances of missing
noexceptin the codebase?
I do not have the necessary tool to perform an exhaustive search.
Please reopen this issue if you find any.