lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] [io]fstream move ctors


From: Vadim Zeitlin
Subject: Re: [lmi] [io]fstream move ctors
Date: Fri, 30 Apr 2021 23:48:20 +0200

On Fri, 30 Apr 2021 20:11:37 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> Vadim--Please help me understand why clang considers the [io]fstream
GC> move ctors to be deleted (cf. the [squashed] commit below).

 First of all, let me start by showing the problem in a simple test case:

---------------------------------- >8 --------------------------------------
% cat -n fstest.cpp
     1  #include <fstream>
     2
     3  struct my_stream final : public std::ifstream
     4  {
     5      my_stream() = default;
     6      my_stream(my_stream const&) = delete;
     7      my_stream(my_stream&&) = default;
     8  };
% clang++-12 -std=c++20 -stdlib=libc++ -fsyntax-only -Wall fstest.cpp
fstest.cpp:7:5: warning: explicitly defaulted move constructor is implicitly 
deleted [-Wdefaulted-function-deleted]
    my_stream(my_stream&&) = default;
    ^
/usr/lib/llvm-12/bin/../include/c++/v1/istream:177:7: note: move constructor of 
'my_stream' is implicitly deleted because base class 'basic_ios<char>' has a 
deleted move constructor
    : virtual public basic_ios<_CharT, _Traits>
      ^
/usr/lib/llvm-12/bin/../include/c++/v1/ios:603:7: note: copy constructor of 
'basic_ios<char>' is implicitly deleted because base class 'std::ios_base' has 
an inaccessible copy constructor
    : public ios_base
      ^
1 warning generated.
---------------------------------- >8 --------------------------------------

GC> C++20 (N4861) [ifstream] says:
GC>   // 29.9.3.1, constructors
GC>   basic_ifstream(basic_ifstream&& rhs);
GC> I assume that it wouldn't be mentioned there if it were deleted;
GC> otherwise, clang would issue a warning for the specification in the
GC> standard itself.
GC> 
GC> Isn't that the "corresponding base class" of our own fs::ifstream?

 So the "base class" clang means here is std::ios_base and the reason it
comes into play here is due to the use of virtual inheritance: as all
virtual base classes sub-objects are initialized by the most derived class
ctors, and not by any intermediate base classes ctors, as would be the case
without virtual inheritance, we need to be able to initialize basic_ios
subobject here, but we don't do it.

 To make thins more clear, an even simpler example like this could be used:
---------------------------------- >8 --------------------------------------
struct vbase {
    vbase() = default;
    vbase(vbase const&) = delete;
};

struct base : virtual vbase {
    base() = default;
    base(base&&) {}
};

struct my_stream final : public base {
    my_stream() = default;
    my_stream(my_stream const&) = delete;
    my_stream(my_stream&&) = default;
};
---------------------------------- >8 --------------------------------------

This is still sufficient to reproduce the same warning/problem, but
removing "virtual" makes it disappear, as expected.


 IOW, the problem is not really clang warning, which is actually very
helpful, as I would have had even more trouble understanding what exactly
is wrong here without it, but the fact that my_stream does _not_ have a
default move ctor because its virtual base class basic_ios doesn't have
one.

 So we have the choice between just deleting the move ctor, as I did
because I thought we didn't need it anyhow and it was the simplest
solution, or defining it explicitly. Except that I'm not even 100% sure
what is the right way to do it, i.e. whether the naive

    my_stream(my_stream&& s) : std::ifstream(std::move(s)) {}

is actually guaranteed to work. I do believe that

    my_stream(my_stream&& s) { std::swap(s); }

should work, but I don't know if this implementation is optimal.

 
 To summarize, it still seems like the simplest thing to do is to keep the
current version, without any move ctor at all, and even though the original
commit description was misleading. But if we want to define this move ctor,
we could do it, probably using swap().

 Please let me know if you think it's worth doing this,
VZ

Attachment: pgp75Bd18wEhh.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]