Re: [lmi] Various C++ m12ns

[Greg Chicares]

On 2017-02-01 21:58, Vadim Zeitlin wrote:
> On Wed, 1 Feb 2017 21:47:34 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Vadim, what do you think about...
> GC> 
> GC> (1) Changing '> >' to '>>' to close template-parameter-lists?
[...]
> So yes, I'd definitely remove the
> spaces, if only because there is no reason to have them in the new code and
> consistency is good.

Done:

commit ba9c81247a467ed242d821e47b17c6acc8b5c738
    Remove stray spaces in template instantiations

> GC> (2) Replacing
> GC> 
> GC>   class foo : private lmi::uncopyable<foo>
> GC> 
> GC> with
> GC> 
> GC>   class foo
> GC>   ...
> GC>       foo(foo const&) = delete;
> GC>       foo& operator=(foo const&) = delete;
> GC> 
> GC> ? I'm not convinced that such a change should be made. If C++11 let us
> GC> write it the way we can write 'final':
> GC>   class foo : uncopyable
> GC> then that would be best. But declaring two extra member functions
> GC> requires writing the class's name four times, and typically those
> GC> declarations would be somewhere in the middle of the class declaration,
> GC> after public members and before other private members. It just seems
> GC> more verbose and less readable to me.

Looking back--over the last few days I've done this in about 175 files,
and I didn't delete the header name manually 175 times, much less write a
class name 700 times. Instead, I did it once with

  sed -e'/include.*uncopyable_lmi.hpp/d'
  sed -e'/lmi::uncopyable/s/^.*lmi::uncopyable *< *\(.*\) *>.*$/&\n    \1(\1 
const\&) = delete;\n    \1\& operator=(\1 const\&) = delete;\n/

and then separately, in another directory, with a couple of vim macros,
watching what the macros did carefully and making some layout changes.
Then, after cleaning up the sed work manually, and testing both separately,
I compared them and resolved the differences, which were few and slight.
This would have been unpleasant with less capable tools.

>  I understand this point of view, but I don't agree with it mainly because
> I have a rule, predating C++11, consisting in always doing something about
> the compiler-generated functions because forgetting to do it is so common
> in C++ code, that it's very nice to have an explicit reminder that the
> author did think about it. So even if the default behaviour is appropriate,
> I would always write a comment like this
> 
>       // Compiler-generated copy ctor and assignment operator are ok.

Me too:

/srv/chroot/cross-lmi/opt/lmi/src/lmi[0]$grep 'Implicitly-declared special 
member functions do the right thing' *.?pp |wc -l   
47

> in C++98 code. And in C++11 code I use
> 
>       foo(foo const&) = default;
>       foo& operator=(foo const&) = default;
> 
> to say this in code instead of comments. So from my point of view, it's
> natural to have the same with "= delete" if the compiler-generated
> functions are not appropriate and, in fact, jarring not to see these 2
> lines in the code.
> 
>  This is, of course, a question of habit, but I believe it's a good habit
> to have.

Thanks for saying this. I found some special member functions with full
implementations that were purely redundant, so, instead of eradicating
them utterly as was my first impulse, I kept the declarations (only):

commit 6234aec54070cf4c596e10ee00ccfe51328671e0

    Replace copy functions with explicitly-defaulted equivalents
    
    The removed implementations of the copy ctor and assignment operator
    were equivalent to those the compiler would have written anyway. Their
    declarations were retained with explicit '= default' as documentation.

Let's see if we can write a guideline for this....

Declare these special member functions by default in every class X:
    X(X const&) = delete;
    X& operator=(X const&) = delete;
If X should be Copyable, then use '= default' instead. Write implementations
in '{}' only if the default does not do the right thing.

Rationale: In C++98, a comment such as
  Implicitly-declared special member functions do the right thing
was appropriate, with the declarations elided. But in C++11, it is
preferable to say that in code. This is better than deriving from
a noncopyable class because it is clearer and optimizes better.

Already I have a question about the guideline. Would you also
always declare a destructor, with '= default' by default?

And another question. At first I was going to recommend either
  public:
    X(X const&) = default;
    X& operator=(X const&) = default;
or
  private:
    X(X const&) = delete;
    X& operator=(X const&) = delete;
but then I figured that calling a deleted public function ought
to produce as clear a diagnostic as calling a deleted private one;
so I think it's okay to declare these preferably in the neighborhood
of other special-member-function declarations, but that seems to
require no guideline. A private '= delete' declaration is never wrong,
and I won't move the ones I've already written, but in the future I'd
probably write all special member functions together in the typical
case.