proposal to allow: custom move constructors & move assignment operators

112 views
Skip to first unread message

Michael Spang

unread,
Nov 19, 2015, 1:52:30 PM11/19/15
to cxx
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct, so in practice we'll need to define the ctors & dtor out-of-line:

struct T {
  T(scoped_ptr<A> a, scoped_ptr<B> b);
  T(T&& other);
  T& operator=(T&& other);
  ~T();

  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

T::T(scoped_ptr<A> a, scoped_ptr<B> b) : a(std::move(a)), b(std::move(b)) {}
T::T(T&& other) : a(std::move(other.a)), b(std::move(other.b)) {}
T& T::operator=(T&& other) { a = std::move(other.a); b = std::move(other.b); return *this; }
T::~T() {}

Also, we would use "= default" to define the move ctor if it were implemented correctly in MSVC2013.

Cheers!
Michael


Nico Weber

unread,
Nov 19, 2015, 2:49:13 PM11/19/15
to Michael Spang, cxx
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.
 
, so in practice we'll need to define the ctors & dtor out-of-line:

struct T {
  T(scoped_ptr<A> a, scoped_ptr<B> b);
  T(T&& other);
  T& operator=(T&& other);
  ~T();

  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

T::T(scoped_ptr<A> a, scoped_ptr<B> b) : a(std::move(a)), b(std::move(b)) {}
T::T(T&& other) : a(std::move(other.a)), b(std::move(other.b)) {}
T& T::operator=(T&& other) { a = std::move(other.a); b = std::move(other.b); return *this; }
T::~T() {}

Also, we would use "= default" to define the move ctor if it were implemented correctly in MSVC2013.

Cheers!
Michael


--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAC5F_1kaq3u7XaEhs%2BPn3cHnT66ZdO%3Dnrz2c9xeAdui9JjrnGw%40mail.gmail.com.

Michael Spang

unread,
Nov 19, 2015, 2:56:08 PM11/19/15
to Nico Weber, cxx
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Michael
 
 
, so in practice we'll need to define the ctors & dtor out-of-line:

struct T {
  T(scoped_ptr<A> a, scoped_ptr<B> b);
  T(T&& other);
  T& operator=(T&& other);
  ~T();

  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

T::T(scoped_ptr<A> a, scoped_ptr<B> b) : a(std::move(a)), b(std::move(b)) {}
T::T(T&& other) : a(std::move(other.a)), b(std::move(other.b)) {}
T& T::operator=(T&& other) { a = std::move(other.a); b = std::move(other.b); return *this; }
T::~T() {}

Also, we would use "= default" to define the move ctor if it were implemented correctly in MSVC2013.

Cheers!
Michael

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAC5F_1kaq3u7XaEhs%2BPn3cHnT66ZdO%3Dnrz2c9xeAdui9JjrnGw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

Nico Weber

unread,
Nov 19, 2015, 2:57:15 PM11/19/15
to Michael Spang, cxx
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().

Michael Spang

unread,
Nov 19, 2015, 3:06:42 PM11/19/15
to Nico Weber, cxx
On Thu, Nov 19, 2015 at 2:57 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().
 

Shrug, I guess it's not a big deal if we wait on this due to an excess of caution in allowing new features.

Note that if we allow implicitly defined operators but not explicit ones, one result is that the plugin's heuristic now enforces a semantic restriction (it constraints the user-defined types that are movable to ones that it allows inlining the dtor for). That seems unfortunate. The check is intended to push some code from .h to .cc, nothing else.

IMO move operators that just recurse to members aren't a big risk, though.

Nico Weber

unread,
Nov 19, 2015, 3:14:32 PM11/19/15
to Michael Spang, cxx
On Thu, Nov 19, 2015 at 12:06 PM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:57 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().
 

Shrug, I guess it's not a big deal if we wait on this due to an excess of caution in allowing new features.

I take issue with "an excess of caution". There are people who are super excited about all this new stuff – which is great! But there are also people who choose to invest their time differently. Almost by definition, this list is populated almost exclusively by the former. The idea behind this gradual roll-out process is that new stuff arrives gradually, so that people who don't read up on all the New Stuff on their own initiative can get used to it somewhat more slowly, while people who do know all about the New Stuff still get to use at least some of it immediately. So yes, this is cautious, but given that we have hundreds of engineers working on the project I'd argue that it's not an excess of caution.

Vladimir Levin

unread,
Nov 19, 2015, 3:44:34 PM11/19/15
to Nico Weber, Michael Spang, cxx
On Thu, Nov 19, 2015 at 12:14 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 12:06 PM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:57 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().
 

Shrug, I guess it's not a big deal if we wait on this due to an excess of caution in allowing new features.

I take issue with "an excess of caution". There are people who are super excited about all this new stuff – which is great! But there are also people who choose to invest their time differently. Almost by definition, this list is populated almost exclusively by the former. The idea behind this gradual roll-out process is that new stuff arrives gradually, so that people who don't read up on all the New Stuff on their own initiative can get used to it somewhat more slowly, while people who do know all about the New Stuff still get to use at least some of it immediately. So yes, this is cautious, but given that we have hundreds of engineers working on the project I'd argue that it's not an excess of caution.

I think caution is a good thing with these features, since they tend to be overused when they aren't really needed. Personally, I'm all for waiting, but I'd also like to get a sense for how long we wait. The problem is that if people don't read up on new things now, they aren't too likely to read up on it next week or next month either. 

Basically, what I'm asking is how long do you propose we wait?


 

Michael Spang

unread,
Nov 19, 2015, 4:07:32 PM11/19/15
to Nico Weber, cxx
On Thu, Nov 19, 2015 at 3:14 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 12:06 PM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:57 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().
 

Shrug, I guess it's not a big deal if we wait on this due to an excess of caution in allowing new features.

I take issue with "an excess of caution". There are people who are super excited about all this new stuff – which is great! But there are also people who choose to invest their time differently. Almost by definition, this list is populated almost exclusively by the former. The idea behind this gradual roll-out process is that new stuff arrives gradually, so that people who don't read up on all the New Stuff on their own initiative can get used to it somewhat more slowly, while people who do know all about the New Stuff still get to use at least some of it immediately. So yes, this is cautious, but given that we have hundreds of engineers working on the project I'd argue that it's not an excess of caution.

I apologize, I don't mean to insult to the process. I think the we have the right motivations in rolling out incrementally.

std::move() on STL and base/ types and std::move() on user types are very closely related, so rolling them out as a unit makes sense to me. I'd argue we should roll out both the tools to consume movable types as well as compose them at the same time - the hard part is the concept, not the syntax.

Michael

Dana Jansens

unread,
Nov 19, 2015, 4:26:19 PM11/19/15
to Michael Spang, Nico Weber, cxx
On Thu, Nov 19, 2015 at 1:06 PM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 3:14 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 12:06 PM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:57 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 11:55 AM, Michael Spang <sp...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 2:49 PM, Nico Weber <tha...@chromium.org> wrote:
On Thu, Nov 19, 2015 at 10:51 AM, Michael Spang <sp...@chromium.org> wrote:
I propose to allow custom move constructors and move assignment operators, particularly if they are defined as moves of each member and base class (i.e. equivalent to the default).

For efficiency reasons, we should be able to move types that simply compose other move-only types such as

struct T {
  scoped_ptr<A> a;
  scoped_ptr<B> b;
};

This allows us to to use these types in STL containers without indirecting through a scoped_ptr<>.

As noted in an earlier thread ("default move ctor and [chromium-style] Complex class/struct needs an explicit out-of-line destructor"), our clang plugin requires an out-of-line constructor & destructor for the above struct

Wasn't the conclusion on that thread that we could fix this in the plugin instead? (Sorry, behind on email.)

In general, I'm pretty scared of allowing custom move constructors/assignment operators generally, and it looks like the motivating thing for this proposal could possibly fixed without allowing them.

Dana suggested that if we could not define our own move operators for custom types, we could not rely on implicitly defined ones for those types either. With that interpretation (which we can certainly debate), fixing the plugin is not enough.

Also, the Google C++ style guide allows defining move operations. We should eventually be consistent with that.

Yes, eventually, but in my opinion not 2 days after we started allowing std::move().
 

Shrug, I guess it's not a big deal if we wait on this due to an excess of caution in allowing new features.

I take issue with "an excess of caution". There are people who are super excited about all this new stuff – which is great! But there are also people who choose to invest their time differently. Almost by definition, this list is populated almost exclusively by the former. The idea behind this gradual roll-out process is that new stuff arrives gradually, so that people who don't read up on all the New Stuff on their own initiative can get used to it somewhat more slowly, while people who do know all about the New Stuff still get to use at least some of it immediately. So yes, this is cautious, but given that we have hundreds of engineers working on the project I'd argue that it's not an excess of caution.

I apologize, I don't mean to insult to the process. I think the we have the right motivations in rolling out incrementally.

std::move() on STL and base/ types and std::move() on user types are very closely related, so rolling them out as a unit makes sense to me. I'd argue we should roll out both the tools to consume movable types as well as compose them at the same time - the hard part is the concept, not the syntax.

You can already write move-only types without writing T&& by using the macro in base/move.h: https://code.google.com/p/chromium/codesearch#chromium/src/base/move.h&l=208  This has a side-effect bonus of also working with base::Bind. You could do this for a struct of scoped_ptrs today. The guidance given in the header file is:

// This macro should be used instead of DISALLOW_COPY_AND_ASSIGN to create                                                              
// a "move-only" type.  Unlike DISALLOW_COPY_AND_ASSIGN, this macro should be                                                           
// the first line in a class declaration.                                                                                               

As always, don't make things copyable normally.

Using this macro means writing a move-constructor of sorts, with slightly different syntax. And it means you have to use t.Pass() instead of std::move(t) on those objects. This leaves us with inconsistency that feels awkward to me, sometimes you use move() sometimes you use Pass().

I think making everything movable in our codebase would be a mistake, so I understand caution here. But I also don't see value in doing the same thing (move-only types) with a non-standard syntax any more.

I would like to see us delete the old macro, and have people use the rvalue-ref version instead: https://code.google.com/p/chromium/codesearch#chromium/src/base/move.h&l=222 if they want to make something movable (it could use a better name if it's going to be a long-lived sibling to DISALLOW_COPY_AND_ASSIGN). In other words, allow people to make an object move-only, but not both copyable+movable generally. This would also work with base::Bind in the same ways.

The benefits here aren't around this letting people make move-only things, since you can already, but it would:
- Make syntax consistent and standard/well-known.
- Allow those types to be used in standard containers/etc, the same way scoped_ptr can now.

The downside is it would let people write types that are both movable and copyable, and the performance characteristics get very muddy there, and it's usually extra complexity for no good reason. It feels to me, in those cases, the class shouldn't be copyable in the first place and this is why we strongly encourage DISALLOW_COPY_AND_ASSIGN everywhere.

Does it help to think of this as an option instead of DISALLOW_COPY_AND_ASSIGN instead of as an additional constructor to add everywhere? We can try make this very clear in the Chromium C++ Dos and Don'ts if that helps. (I was surprised DISALLOW_COPY_AND_ASSIGN isn't even mentioned in there.)


bruce...@google.com

unread,
Nov 20, 2015, 1:20:49 PM11/20/15
to cxx, sp...@chromium.org
VS 2015 support is coming along well and it is likely that we will be able to switch in about two weeks. I don't know when we would actually switch, but it is probably soon enough that we should allow for it in our calculations - specifically, let's not compromise our code or coding standards based on a compiler version that we may be dropping soon.
Reply all
Reply to author
Forward
0 new messages