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
noexcept
then 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 9 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
noexcept
then 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 9 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 9 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 9 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 9 years ago
- Status changed from Code review to Closed
Updated by Davide Pesavento over 9 years ago
Have you checked that there aren't other instances of missing noexcept
in the codebase?
Updated by Junxiao Shi over 9 years ago
Have you checked that there aren't other instances of missing
noexcept
in the codebase?
I do not have the necessary tool to perform an exhaustive search.
Please reopen this issue if you find any.