Question about [chromium-style] Complex class/struct needs an explicit out-of-line constructor / destructos

[Zijie He]

Hi, Chromium-Devs,

I cannot quite tell the reason of this style rule. As far as I can tell, it actually removes move constructors and move assignments silently and implicitly without any notifications in lots of our code.

Move constructor and move assignment operator are deleted if a destructor is provided.

Though by using = default, we can still get compiler generated move constructor and assignment (Correct me if I made a mistake here), we do not always use = default to replace {}. (https://goo.gl/pZBsn1) And what's the point if I define a constructor or destructor just for = default?

        .Hzj_jie

[Daniel Cheng]

On Tue, Nov 8, 2016 at 4:05 PM 'Zijie He' via Chromium-dev <chromi...@chromium.org> wrote:

Hi, Chromium-Devs,

I cannot quite tell the reason of this style rule. As far as I can tell, it actually removes move constructors and move assignments silently and implicitly without any notifications in lots of our code.

Compiler-generated constructors and destructors are inlined. This can often result in surprisingly large amounts of code being emitted, since the destructors of your classes fields are also inlined, etc.

When the checks were originally written, we only checked for constructors / destructors / copy constructors (notably, not copy assignment operators). I'm not sure if we do it for move constructors / operators, but making the check consistent across these is a long-term goal of mine.

 

Move constructor and move assignment operator are deleted if a destructor is provided.

Though by using = default, we can still get compiler generated move constructor and assignment (Correct me if I made a mistake here), we do not always use = default to replace {}. (https://goo.gl/pZBsn1) And what's the point if I define a constructor or destructor just for = default?

You can use = default, but the plugin will complain if you = default in the header file =)

As for the use of { }, that's because of lot of the codebase predated C++11.

Daniel

 

        .Hzj_jie

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Zijie He]

        .Hzj_jie

On Tue, Nov 8, 2016 at 4:12 PM, Daniel Cheng <dch...@chromium.org> wrote:

On Tue, Nov 8, 2016 at 4:05 PM 'Zijie He' via Chromium-dev <chromi...@chromium.org> wrote:

Hi, Chromium-Devs,

I cannot quite tell the reason of this style rule. As far as I can tell, it actually removes move constructors and move assignments silently and implicitly without any notifications in lots of our code.

Compiler-generated constructors and destructors are inlined. This can often result in surprisingly large amounts of code being emitted, since the destructors of your classes fields are also inlined, etc.

This is an interesting point. I have never considered it. 

When the checks were originally written, we only checked for constructors / destructors / copy constructors (notably, not copy assignment operators). I'm not sure if we do it for move constructors / operators, but making the check consistent across these is a long-term goal of mine.

 

Move constructor and move assignment operator are deleted if a destructor is provided.

Though by using = default, we can still get compiler generated move constructor and assignment (Correct me if I made a mistake here), we do not always use = default to replace {}. (https://goo.gl/pZBsn1) And what's the point if I define a constructor or destructor just for = default?

You can use = default, but the plugin will complain if you = default in the header file =)

That's the trouble, I need to extraly write the type name for at least 6 times, and "= default" for at least 2 times. I have no concern about moving "= default" out of header files, but indeed I thought we should not write them at all, before I realized your comment about the binary size. 

As for the use of { }, that's because of lot of the codebase predated C++11.

Yes, I understand. But do we have plan to change them through entire codebase? Or at least update the coding style to discourage using {}? 

[Zijie He]

Sorry, I have made a mistake in my previous mail thread. Even a destructor = default, compiler will also ignore move constructor and move assignment operator. http://cpp.sh/7jxo vs http://cpp.sh/4sdld vs http://cpp.sh/84vhg.

        .Hzj_jie