This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 725 - Move constructors are not noexcept
Summary: Move constructors are not noexcept
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: General (show other bugs)
Version: 3.2
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2014-01-09 18:26 UTC by Martinho Fernandes
Modified: 2019-12-04 12:56 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.
Comment 8 Nobody 2019-12-04 12:56:15 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/725.

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