Re: [lmi] depr.impldec

[Vadim Zeitlin]

On Wed, 13 Jul 2022 20:51:24 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 7/10/22 12:29, Vadim Zeitlin wrote:
GC> [...]
GC> >  FWIW I would omit the move ctors and assignment operators instead of
GC> > deleting them because the situation is much more clear with move-related
GC> > special functions than with the copy-related ones: if they're not
GC> > explicitly defined, they're not available -- i.e. the compiler never
GC> > provides them implicitly, unlike with the copy ones.
GC> 
GC> That sounds like page 55 here:
GC>   https://howardhinnant.github.io/bloomberg_2016.pdf
GC> | Guideline:
GC> | Never delete the move members
GC> | Deleted move members are at best redundant, and at worst, a bug...

 What can I say, Howard does know this stuff better than I (perhaps not
surprising considering he designed it) and is completely right, of course.
I vaguely remembered that there was some reason to not delete move special
functions, but completely forgot that it was a pretty good reason.

GC> Should I therefore purge the deleted move ctors we have?
GC> If I got the regex right, there are only these two:
GC> 
GC> $git --no-pager grep '&&.*= *delete' 
GC> path.hpp:    ifstream(ifstream&&) = delete;
GC> path.hpp:    ofstream(ofstream&&) = delete;
GC>
GC> so should I purge those two lines?

 In this particular case they do no harm because the class is not copyable
neither and is not supposed to be returned, but it's still better to remove
them for consistency.

GC> Or comment them out

 I wouldn't want to keep useless commented out lines of code.

GC> Not all members are always appropriate. For any unwanted
GC> member, I suggest:
GC>  - change "default" to "delete", only for default ctor and
GC>    copy operations;
 
 Yes.

GC>  - for move operations, "delete" is potentially harmful, so
GC>    comment those out if unwanted

 Please let's just remove them instead.

GC>  - add a very brief comment on any changed line

 I'm not sure that even this is needed -- if you delete the copy ctor, it
seems to say pretty clearly that you don't want the class to be copyable
and what else could the comment say?

GC> Here's a complete example of applying this technique to one class:
GC> 
GC> --8<----8<----8<----8<----8<----8<----8<----8<--
GC> diff --git a/path.hpp b/path.hpp
GC> index a20cac840..dfdf5741c 100644
GC> --- a/path.hpp
GC> +++ b/path.hpp
GC> @@ -503,8 +503,6 @@ class ifstream final
GC>      :public std::ifstream
GC>  {
GC>    public:
GC> -    ifstream() = default;
GC> -
GC>      explicit ifstream
GC>          (path const& p
GC>          ,std::ios_base::openmode mode = std::ios_base::in
GC> @@ -513,8 +511,13 @@ class ifstream final
GC>          {
GC>          }
GC>  
GC> -    ifstream(ifstream const&) = delete;
GC> -    ifstream(ifstream&&) = delete;
GC> +    // Rule of six: special member functions.
GC> +    ifstream()                           = default;
GC> +    ifstream(ifstream const&)            = delete;  // not yet needed
GC> +//  ifstream(ifstream&&)                 = default; // not yet needed
GC> +    ifstream& operator=(ifstream const&) = delete;  // std: only move=
GC> +//  ifstream& operator=(ifstream&&)      = default; // not yet needed
GC> +    ~ifstream() override                 = default;
GC>  
GC>      void open
GC>          (path const& p
GC> --8<----8<----8<----8<----8<----8<----8<----8<--
GC> 
GC> What do you think?


 I think it's much too verbose and would prefer just

        ifstream()                           = default;
        ifstream(ifstream const&)            = delete;
        ifstream& operator=(ifstream const&) = delete;
        ~ifstream() override                 = default;

without redundant comments.

 I'd also prefer to keep the default constructor where it is now but I
admit that I don't have any good rationale for this -- it's just that I'm
used to having it before the other ctors and not after them.

 Regards,
VZ

pgpiD5vzCBPJZ.pgp
Description: PGP signature