Re: [lmi] [io]fstream move ctors

[Vadim Zeitlin]

On Sat, 1 May 2021 00:57:36 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> On 4/30/21 9:48 PM, Vadim Zeitlin wrote:
GC> > On Fri, 30 Apr 2021 20:11:37 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> > 
GC> > GC> Vadim--Please help me understand why clang considers the [io]fstream
GC> > GC> move ctors to be deleted (cf. the [squashed] commit below).
GC> > 
GC> >  First of all, let me start by showing the problem in a simple test case:
GC> > 
GC> > ---------------------------------- >8 
--------------------------------------
GC> > % cat -n fstest.cpp
GC> >      1  #include <fstream>
GC> >      2
GC> >      3  struct my_stream final : public std::ifstream
GC> >      4  {
GC> >      5      my_stream() = default;
GC> >      6      my_stream(my_stream const&) = delete;
GC> >      7      my_stream(my_stream&&) = default;
GC> >      8  };
GC> > % clang++-12 -std=c++20 -stdlib=libc++ -fsyntax-only -Wall fstest.cpp
GC> > fstest.cpp:7:5: warning: explicitly defaulted move constructor is 
implicitly deleted [-Wdefaulted-function-deleted]
GC> >     my_stream(my_stream&&) = default;
GC> >     ^
GC> > /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
GC> >     : virtual public basic_ios<_CharT, _Traits>
GC> >       ^
GC> > /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
GC> >     : public ios_base
GC> >       ^
GC> > 1 warning generated.
GC> > ---------------------------------- >8 
--------------------------------------
GC> 
GC> Suppose we add to that example (so that gcc also reports errors):
GC> 
GC>   int main(int, char*[])
GC>   {
GC>     std::ifstream a;                // 1 okay
GC>     std::ifstream b(a);             // 2 error
GC>     std::ifstream c(std::move(a));  // 3 okay
GC> 
GC>     my_stream s;                    // 4 okay
GC>     my_stream t(s);                 // 5 error
GC>     my_stream u(std::move(s));      // 6 error
GC>   }
GC> 
GC> In the first set, using std::ifstream:
GC> 
GC>  - Somehow #2 doesn't surprise me

 And it shouldn't, copying a file input stream doesn't really make sense,
does it? Such stream is associated with a file, so what would it mean to
copy it?
 
GC>  - #3 kind of surprises me.

 And I think this shouldn't be surprising neither. Any object with an empty
state (and we know that ifstream has one, because it's the state in which
it's left by its default ctor) should be expected to be movable and I can't
think of any reasonable exception to this. IOW the only "objects" (in the
usual, and hence intentionally imprecise, meaning of this word) for which
it makes sense not to support move semantics are those that are always
"valid", and so can't be moved from. But if an object has a reasonable
empty state, I'd always expect it to support move semantics, either in
addition to or instead of, copying.

GC>    If #3 and #6 were both errors, that would seem natural enough to me.
GC>    Given that one is an error and the other is not, it's #3 that seems
GC>    surprising: moving works when copying doesn't.

 Again, this seems all but surprising to me: moving is a much more
fundamental operation than copying and there are plenty of examples of
objects that support moving but not copying, but no non-pathological
examples of the copyable-but-not-movable objects.

GC>    What I see through a glass darkly satisfies my curiosity well
GC>    enough, because after all why should we care about moving or copying
GC>    fstream objects?

 We would care about it as soon as we start treating them as "objects".
E.g. as soon as you start storing them in a vector or any other container.
In our typical use, we don't really care that they're objects at all, even
though we create a C++ fstream object in our code, its object-ness doesn't
matter as long as it's just a local variable on the stack.

 Which is why I indeed agree that we don't care about neither moving nor
copying them right now: we don't use them as objects at all. But it's not
an unreasonable question more generally speaking.

GC> >  So we have the choice between just deleting the move ctor, as I did
GC> > because I thought we didn't need it anyhow and it was the simplest
GC> > solution, or defining it explicitly.
GC> 
GC> I look with favor upon a third choice: use the rule of zero,

 I don't know if you've had the patience to read my post

   https://lists.nongnu.org/archive/html/lmi/2021-04/msg00042.html

until the end, but I still stand by its conclusion and IMO it applies even
more here than in the original situation discussed there.

GC> and don't try to specify, ourselves, what should be deleted
GC> or defaulted. If we'd get those special member functions
GC> automatically, then there's no reason to type "=default".

 There is indeed no reason to use "= default" because, as we've discovered,
it doesn't work as intended. But there is an excellent reason to write
"= delete" here: without it, it's very natural to assume that fs::ifstream
does support move semantics because we know that its base class does and
this class is clearly just a trivial wrapper for it. As you've seen, I've
totally missed the fact that this class isn't, in fact, movable due to the
non-movable virtual base class existence myself, until your question made
me really look into this. And I don't think it was an especially egregious
mistake to make -- I believe that the vast majority of other people would
make exactly the same assumption, and would be just as puzzled to discover
that it was wrong. So being upfront and clear about it is IMO very
important.

GC> Otherwise, the compiler can't generate them; que será será.
GC> In that case, isn't writing "=delete" just redundant?

 It is strictly less redundant than writing a comment and we still write
comments, even if they are not, strictly speaking, required.

 To justify this statement, consider that:

1. Writing "= delete" is at least as useful as writing a comment saying
   that this class is not movable and I believe that such a comment would
   absolutely have to be written if we removed the deleted move ctor to
   avoid misleading people reading the code from assuming that the class is
   movable.

2. It is strictly more useful than a comment because actually deleting the
   move ctor results in better error messages if the compiler tries to use
   it.

GC> And if we never even try to use them (as is likely), then
GC> we don't have to think about any of this.

 Until the moment when you get multiple pages of error messages because you
assumed the class was movable (quite likely implicitly, e.g. by passing it
to some function which requires its argument to be either copyable or
movable) and it turned out that it wasn't.


 Again, I don't want to repeat the same arguments I've already given in the
post linked above, but IMO all C++11 classes must explicitly declare their
copy/move special functions as being either defaulted or deleted. This
clearly indicates the intention of the class author and advantageously
replaces the comments that we had to write to explain it before. The extra
clarity on its own is already well worth writing them explicitly, but this
example shows how doing this can also have other benefits: if we hadn't
written an explicitly defaulted move ctor in the initial version, we
wouldn't have even realized that the class isn't, in fact, movable.

 I really need to stop saying the same thing again and again because I
realize that if I haven't convinced you by now, I'm not going to do it by
repeating it a few (dozens) more times, but I wish I could understand _why_
do you disagree with this.

 Regards,
VZ

pgpMME4oPBjsv.pgp
Description: PGP signature