3

I've been using Google's DISALLOW_COPY_AND_ASSIGN macro from their C++ Style Guide for a couple of months now, but it recently occurred to me that it would be useful to additionally disable the move constructor and move assignment.

I haven't written any real macros before (in fact, I've been trying to stay away from them as much as possible), so I'd like to get some feedback from the rest of you on whether I've implemented it correctly.

// Original Version
#define DISALLOW_COPY_AND_ASSIGN(TypeName) \
TypeName(const TypeName&);                 \
void operator=(const TypeName&)

// Modified Version (no move semantics)
#define DISALLOW_COPY_MOVE_AND_ASSIGN(TypeName) \
TypeName(const TypeName&);                      \
void operator=(const TypeName&);                \
TypeName(TypeName&&);                           \
void operator=(const TypeName&&)

Suggestions and criticism are very welcome.

Thanks for your time!

spitfire88
  • 1,596
  • 3
  • 19
  • 31
SamuelMS
  • 1,116
  • 1
  • 11
  • 22
  • 1
    Why would you want to follow that guide in the first place? – Cubbi Nov 17 '13 at 02:33
  • 2
    google's style guide reads like someone who believes that C++ should have all the expressive power of C and all the elegance of Java: it is written by someone who either hates the language, or learned it in 97 and thinks learning anything new is unreasonable. – Yakk - Adam Nevraumont Nov 17 '13 at 02:57
  • @Yakk: I think I know who is primarily responsible for this style guide and he is certainly a rather experienced C++ developer knowing the modern C++ techniques. I also don't think that he hates C++. Creating a programming style which work well in large organization with lots of developers who are mostly ignorant of the details of the underlying system is an interesting challenge. The results tend to be rather constraining and certainly always contain rules I, at least, dislike. Often, some of the rules lead to inferior code. – Dietmar Kühl Nov 17 '13 at 03:04
  • 1
    @DietmarKühl Ok, "as if it is written". If google hires developers who cannot handle *copy* and *move* constructors, they need to hire less incompetent developers. "Prefer `CopyFrom` to a copy constructor" -- really? "Do not use lambda expressions" -- really? "do not use `std::function`" -- really? "We do not allow default function parameters" -- really? "Do not use rvalue references" -- boggle! – Yakk - Adam Nevraumont Nov 17 '13 at 03:16
  • @Yakk: The first rule has a nearly trivial explanation: they don't allow exceptions! I don't agree with this approach as it basically clobbers the language (and I don't buy into any reasons I have ever seen). However, once you don't allow exceptions you have to use two-phase initialization because you can't deal with errors otherwise. Note that I'm not defending or endorsing these coding guidelines. The place I work at has similar guidelines and I'm one of the more vocal critic of these. When criticizing these rules it helps to understand why they are thought to be good. – Dietmar Kühl Nov 17 '13 at 03:25
  • @Yakk: lambdas and `std::function` are probably be ruled out due to no throwing exceptions, too. In the case of lambdas this is actually rather problematic because calling an inline function through a lambda is a lot cheaper than calling it through a pointer. ... which is still cheaper than calling it through a `std::function` which may be another reason to object to `std::function`. – Dietmar Kühl Nov 17 '13 at 03:28
  • @Yakk lots of those things make sense. There's plenty of ways that things go ... unexpected with features like default function parameters. Consider what happens if someone overrides a default parameter in a derived class. The value used is tied to the static type of the object at call time, but someone may be expecting the dynamic type to be used. The reason for blocking copy/assign operators is preventing performance issues when accidentally does something like `const Type f = SomeFunc()` instead of `const Type& f = SomeFunc()`. `=` looks innocent, `f.CopyFrom(...)` doesn't hide the intent. – Charlie Nov 17 '13 at 04:30
  • 1
    @DietmarKühl I think what upsets people here is not so much that the guide bans RAII and much of the language along with it for dubious reasons, it's how it goes about it. Some passages certainly give off the FQA vibe. Too bad they reworded my favorite line "the compiler will do it for you, badly" a couple months ago. – Cubbi Nov 17 '13 at 05:02
  • 1
    @DietmarKühl: It just isn't true that two-phase construction is the only alternative to exceptions. Consider `std::fstream`. When the constructor fails to complete the desired initialization, it doesn't throw. It internally marks the object as having a failed state. – Ben Voigt Jan 13 '14 at 20:52
  • @Charlie `=` looks innocent to a Java programmer, but should ring lots of bells for a c++ programmer. – lmat - Reinstate Monica Mar 03 '14 at 21:01

1 Answers1

15

There is no need to disable the move constructor or the move assignment if there is already a declared copy constructor or copy assignment: the move constructor and the move assignment are only implicitly declared as defaulted if there is neither copy constructor nor copy assignment assignment declared for the class. It seems safe to leave the macro alone for that purpose.

A potential change, however, could be to declare the copy constructor and copy assignment explicitly as deleted:

#define DISALLOW_COPY_AND_ASSIGN(TypeName) \
TypeName(TypeName&) = delete;              \
void operator=(TypeName) = delete;

This way the copy constructor and the copy assignment can neither be called from members of the class nor can they they be defined.

In C++ (starting with the 2011 revision) I can't see any point in this macro: it seems easy enough to just delete the constructors explicitly.

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • You're right, and I would have gladly used C++11's function deletion had I could at the time of asking; VS2012 didn't support this feature. Ironically, a few hours after asking this, I upgraded to VS2013 -- and hence now have this option. Thanks for the answer. – SamuelMS Nov 17 '13 at 07:59