New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 725 - Move constructors are not noexcept
Move constructors are not noexcept
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: General
3.2
All All
: Normal Unknown
Assigned To: Nobody
:
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2014-01-09 18:26 UTC by Martinho Fernandes
Modified: 2016-06-03 12:32 UTC (History)
3 users (show)



Attachments
This patch remedies the issue (5.62 KB, patch)
2014-01-10 12:02 UTC, Marton Danoczy
no flags Details | Diff

Description Martinho Fernandes 2014-01-09 18:26:01 UTC
Eigen types now sport move constructors, but those move constructors are not noexcept in compilers that support it and care about it. This causes problems when containers use `std::move_if_noexcept` and end up doing needless copies.

Please make the move constructors noexcept as appropriate.
Comment 1 Marton Danoczy 2014-01-09 18:38:54 UTC
Test case:

#include <Eigen/Core>
#include <utility>
#include <vector>

using namespace Eigen;

// a type containing an eigen member to showcase
// that providing a noexcept move constructor is needed
struct B
{
    B(const Array2i& x) : x(x) {}

    // [1]: the next line shouldn't be necessary!
    B(B&& b) noexcept : x(b.x) {}

    Array2i x;
};

// a move-only type
struct M
{
    explicit M(int a) : a(a) {}
    M(M&& m) noexcept : a(m.a) {}

    int a;
};


typedef std::pair<M, B> pair;
typedef std::vector<pair, aligned_allocator<pair>> vector;

int main()
{
    vector v;
    v.emplace_back(M(0), B(Array2i(1,2)));
    return 0;
}

std::vector requires noexcept moves in move-only types. Because of M, the pair is also move-only. With noexcept move constructors in eigen types, this should compile fine without the line [1].
Comment 2 Marton Danoczy 2014-01-10 12:02:20 UTC
Created attachment 416 [details]
This patch remedies the issue

This patch qualifies move constructors and move assignment operators (MC/MAO) as noexcept.

Note that the exception safety of the MC/MAO is not trivial to determine. Namely, the call to Base::_set_noalias(other) is problematic for custom scalar types for which assignment may throw. Hence I only marked the MC/MAO of Array and Matrix as noexcept iff std::is_nothrow_move_assignable<Scalar>::value is true. Maybe somebody who is knowledgeable in the inner workings of internal::assign_selector could comment on whether this guess is correct.

The patch applies on top of pull request #40, which is (mysteriously to me) also required to fix this issue.
Comment 3 Marton Danoczy 2014-01-10 12:16:02 UTC
Note that these patches apply nicely on top of 3.2 by first grafting

https://bitbucket.org/eigen/eigen/commits/c280e906ade1689d2f47c14d40e820a8e6cc2de4
Comment 4 Marton Danoczy 2015-01-27 11:04:35 UTC
Reviving this old bug. Could it by chance be merged into the next release? With C++11 gaining more and more acceptance, having Eigen not fully support it is becoming an issue.
Comment 5 Marc Glisse 2016-03-29 19:14:23 UTC
It seems very unlikely that is_nothrow_move_assignable<Scalar> could be the right condition for a move constructor...
Comment 6 Gael Guennebaud 2016-03-29 21:02:51 UTC
Yep, is_nothrow_move_constructible would be more appropriate. With such a change, I'd be OK to apply this patch.
Comment 7 Gael Guennebaud 2016-06-03 12:32:31 UTC
https://bitbucket.org/eigen/eigen/commits/9d4a08d57d0d/
Summary:     Bug 725: make move ctor/assignment noexcept.

Note You need to log in before you can comment on or make changes to this bug.