Project

General

Profile

Actions

Bug #2966

closed

Move constructors and assignment operators should be declared noexcept

Added by Davide Pesavento almost 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Utils
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
1.00 h

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.

Actions #1

Updated by Davide Pesavento almost 9 years ago

  • Description updated (diff)
Actions #2

Updated by Junxiao Shi almost 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.

Actions #3

Updated by Davide Pesavento almost 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.

Actions #4

Updated by Junxiao Shi almost 9 years ago

  • Category set to Utils
  • Assignee set to Junxiao Shi
  • Estimated time changed from 1.50 h to 1.00 h
Actions #5

Updated by Junxiao Shi almost 9 years ago

  • Status changed from New to In Progress
Actions #6

Updated by Junxiao Shi almost 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.

Actions #7

Updated by Junxiao Shi almost 9 years ago

  • Status changed from Code review to Closed
Actions #8

Updated by Davide Pesavento almost 9 years ago

Have you checked that there aren't other instances of missing noexcept in the codebase?

Actions #9

Updated by Junxiao Shi almost 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.

Actions #10

Updated by Alex Afanasyev almost 9 years ago

  • Description updated (diff)
Actions

Also available in: Atom PDF