Skip to content

fix move special functions in mem in and outfile#258

Merged
aous72 merged 1 commit intoaous72:masterfrom
geo-ant:fix/move-special-functions-for-mem-outfile
Feb 25, 2026
Merged

fix move special functions in mem in and outfile#258
aous72 merged 1 commit intoaous72:masterfrom
geo-ant:fix/move-special-functions-for-mem-outfile

Conversation

@geo-ant
Copy link
Contributor

@geo-ant geo-ant commented Feb 25, 2026

Dear Aous,

I'm sorry I have to bother you with this again. Some changes were made to my last pull request that introduced a hard to see, but critical bug. In the move constructors a call to std::swap is performed which leads to an infinite recursion and stack overflow. The problem is that currently std::swap is now used with an argument of type mem_outfile in the mem_outfile move constructor. This will lead to infinite recursion, because std::swap itself uses the move constructor and move assignment of the mem_outfile class itself. Allow me to explain:

As an example, the version of the gnu standard library that I'm using defines std::move as:

  template<typename _Tp>
    _GLIBCXX20_CONSTEXPR
    inline
#if __cplusplus >= 201103L
    typename enable_if<__and_<__not_<__is_tuple_like<_Tp>>,
			      is_move_constructible<_Tp>,
			      is_move_assignable<_Tp>>::value>::type
#else
    void
#endif
    swap(_Tp& __a, _Tp& __b)
    _GLIBCXX_NOEXCEPT_IF(__and_<is_nothrow_move_constructible<_Tp>,
				is_nothrow_move_assignable<_Tp>>::value)
    {
#if __cplusplus < 201103L
      // concept requirements
      __glibcxx_function_requires(_SGIAssignableConcept<_Tp>)
#endif
      _Tp __tmp = _GLIBCXX_MOVE(__a);
      __a = _GLIBCXX_MOVE(__b);
      __b = _GLIBCXX_MOVE(__tmp);
    }

which roughly simplifies to

template<typename T>
void move(T& lhs, T& rhs) {
  T tmp = std::move(lhs);
  lhs = std::move(rhs);
  rhs = std::move(tmp);
}

From this, we can see that calling std::swap like is currently done is a problem. It's of course fine to call std::swap on the member fields themselves, but not on the instance of the class itself.

I'm pretty sure this is the reason that the late C++ expert Rainer Grimm writes here

If you use the Copy-and-Swap Idiom to implement the copy assignment and the move assignment operator, you must define your own swap — either as a member function or as a friend

We can also infer from the documentation on std::swap on cppreference, that the use of the move constructor and assignment is not particular to glibcxx, but is actually part of the standard:

This overload participates in overload resolution only if std::is_move_constructible_v && std::is_move_assignable_v is true.

  1. Swaps the values a and b.
    This overload participates in overload resolution only if std::is_move_constructible_v && std::is_move_assignable_v is true.

This isn't well-defined unless the move constructor is defined without the use of std::swap.

@aous72
Copy link
Owner

aous72 commented Feb 25, 2026

Thank you putting an eye on the code, and for correcting it -- the code has to be correct.

@aous72 aous72 merged commit 79f040c into aous72:master Feb 25, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants