lmi
[Top][All Lists]
Advanced

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

Re: [lmi] depr.impldec


From: Greg Chicares
Subject: Re: [lmi] depr.impldec
Date: Wed, 13 Jul 2022 20:51:24 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0

On 7/10/22 12:29, Vadim Zeitlin wrote:
[...]
>  FWIW I would omit the move ctors and assignment operators instead of
> deleting them because the situation is much more clear with move-related
> special functions than with the copy-related ones: if they're not
> explicitly defined, they're not available -- i.e. the compiler never
> provides them implicitly, unlike with the copy ones.

That sounds like page 55 here:
  https://howardhinnant.github.io/bloomberg_2016.pdf
| Guideline:
| Never delete the move members
| Deleted move members are at best redundant, and at worst, a bug...

Should I therefore purge the deleted move ctors we have?
If I got the regex right, there are only these two:

$git --no-pager grep '&&.*= *delete' 
path.hpp:    ifstream(ifstream&&) = delete;
path.hpp:    ofstream(ofstream&&) = delete;

so should I purge those two lines? Or comment them out, because,
stepping back first...

What we really need is a global strategy that we can apply to
every class in every file. (So far, we've only addressed the
'-Wdeprecated-copy-with-dtor' issues, in the files identified
by clang.) Let's try to formulate that first, and then see how
it handles the "[move] = delete" question above.

I weakly favor the Rule of Zero, while you more strongly favor
the Rule of Five, so let's start with a universal Rule of Five
(six, counting the default ctor). As a text-template:

    // Rule of six: special member functions.
    Q() = default;
    Q(Q const&) = default;
    Q(Q&&) = default;
    Q& operator=(Q const&) = default;
    Q& operator=(Q&&) = default;
    ~Q() = default;

I suggest we copy and paste that entire block into each class,
then s/Q/name-of-class/g and remove any earlier duplicates.
I think that it should go either at the top of the public or
protected section where it belongs (assuming it all belongs
in one section), or perhaps after ctors with more than zero
arguments.

I've been finding it difficult to move my eye across six
differently-indented words beginning with "de", so I really
want to impose uniform indentation on equal signs.

Not all members are always appropriate. For any unwanted
member, I suggest:
 - change "default" to "delete", only for default ctor and
   copy operations;
 - for move operations, "delete" is potentially harmful, so
   comment those out if unwanted
 - add a very brief comment on any changed line
Thus, for example:

    // Rule of six: special member functions.
    Q()                    = default;
    Q(Q const&)            = default;
//  Q(Q&&)                 = default; // non-moveable by design
    Q& operator=(Q const&) = delete;  // not needed yet
//  Q& operator=(Q&&)      = default; // non-moveable by design
    ~Q()                   = default;

Favoring the Rule of Five means always writing a dtor. Some may
need to be written out of line:

    ~Q()                   ; // Implemented out of line.

and sometimes the default ctor in this block won't be needed:

    Q(foo = foo_default);

//  Q()                    = default; // zero-arg ctor above

Here's a complete example of applying this technique to one class:

--8<----8<----8<----8<----8<----8<----8<----8<--
diff --git a/path.hpp b/path.hpp
index a20cac840..dfdf5741c 100644
--- a/path.hpp
+++ b/path.hpp
@@ -503,8 +503,6 @@ class ifstream final
     :public std::ifstream
 {
   public:
-    ifstream() = default;
-
     explicit ifstream
         (path const& p
         ,std::ios_base::openmode mode = std::ios_base::in
@@ -513,8 +511,13 @@ class ifstream final
         {
         }
 
-    ifstream(ifstream const&) = delete;
-    ifstream(ifstream&&) = delete;
+    // Rule of six: special member functions.
+    ifstream()                           = default;
+    ifstream(ifstream const&)            = delete;  // not yet needed
+//  ifstream(ifstream&&)                 = default; // not yet needed
+    ifstream& operator=(ifstream const&) = delete;  // std: only move=
+//  ifstream& operator=(ifstream&&)      = default; // not yet needed
+    ~ifstream() override                 = default;
 
     void open
         (path const& p
--8<----8<----8<----8<----8<----8<----8<----8<--

What do you think?


reply via email to

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