chrome: Add out-of-line copy ctors for complex classes.

chrome: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=dcheng@chromium.org, thakis@chromium.org

Committed: https://crrev.com/b8aacbe5510e9a7aaccb5646904f350bca8e3a7c
Cr-Commit-Position: refs/heads/master@{#377767}

[vmpstr, 2016-02-24 00:15:42]

Please take a look.

[tapted, 2016-02-24 00:36:13]

[drive-by] Sorry to put a spanner in the works. I'm only OWNER for one of these,
but a similar thing might apply to other cases.

https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/app_list_prefs.h (right):

https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/app_list_prefs.h:46: AppListInfo(const AppListInfo&
other);
Pretty sure this can be `= delete` -- only the assignment operator is used for
`AppListInfo`

[vmpstr, 2016-02-24 00:43:06]

https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/app_list_prefs.h (right):

https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/app_list_prefs.h:46: AppListInfo(const AppListInfo&
other);
On 2016/02/24 00:36:13, tapted wrote:
> Pretty sure this can be `= delete` -- only the assignment operator is used for
> `AppListInfo`

If I delete it, then there's a compiler error on this line:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....

Here's the error:

/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_pair.h:104:21:
error: call to deleted constructor of 'app_list::AppListPrefs::AppListInfo'
      : first(__a), second(__b) { }
                    ^      ~~~
/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_map.h:453:29:
note: in instantiation of member function 'std::pair<const
std::basic_string<char>, app_list::AppListPrefs::AppListInfo>::pair' requested
here
          __i = insert(__i, value_type(__k, mapped_type()));
                            ^
../../chrome/browser/ui/app_list/app_list_prefs.cc:120:11: note: in
instantiation of member function 'std::map<std::basic_string<char>,
app_list::AppListPrefs::AppListInfo, std::less<std::basic_string<char> >,
std::allocator<std::pair<const std::basic_string<char>,
app_list::AppListPrefs::AppListInfo> > >::operator[]' requested here
    (*out)[it.key()] = *AppListInfo::CreateAppListInfoFromDict(item_dict);
          ^
../../chrome/browser/ui/app_list/app_list_prefs.h:46:5: note: 'AppListInfo' has
been explicitly marked deleted here
    AppListInfo(const AppListInfo& other) = delete;
    ^

I think it happens because [] operator creates an empty value and then inserts
it via a copy constructor, but I'm not too sure.

[tapted, 2016-02-24 01:38:47]

On 2016/02/24 00:43:06, vmpstr wrote:
>
https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
> File chrome/browser/ui/app_list/app_list_prefs.h (right):
> 
>
https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
> chrome/browser/ui/app_list/app_list_prefs.h:46: AppListInfo(const AppListInfo&
> other);
> On 2016/02/24 00:36:13, tapted wrote:
> > Pretty sure this can be `= delete` -- only the assignment operator is used
for
> > `AppListInfo`
> 
> If I delete it, then there's a compiler error on this line:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> 
> Here's the error:
> 
>
/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_pair.h:104:21:
> error: call to deleted constructor of 'app_list::AppListPrefs::AppListInfo'
>       : first(__a), second(__b) { }
>                     ^      ~~~
>
/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_map.h:453:29:
> note: in instantiation of member function 'std::pair<const
> std::basic_string<char>, app_list::AppListPrefs::AppListInfo>::pair' requested
> here
>           __i = insert(__i, value_type(__k, mapped_type()));
>                             ^
> ../../chrome/browser/ui/app_list/app_list_prefs.cc:120:11: note: in
> instantiation of member function 'std::map<std::basic_string<char>,
> app_list::AppListPrefs::AppListInfo, std::less<std::basic_string<char> >,
> std::allocator<std::pair<const std::basic_string<char>,
> app_list::AppListPrefs::AppListInfo> > >::operator[]' requested here
>     (*out)[it.key()] = *AppListInfo::CreateAppListInfoFromDict(item_dict);
>           ^
> ../../chrome/browser/ui/app_list/app_list_prefs.h:46:5: note: 'AppListInfo'
has
> been explicitly marked deleted here
>     AppListInfo(const AppListInfo& other) = delete;
>     ^
> 
> I think it happens because [] operator creates an empty value and then inserts
> it via a copy constructor, but I'm not too sure.

Ugh. It works fine with libc++. Looks like this is a GCC bug not fixed until
4.8. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44436

[Nico, 2016-02-24 01:39:35]

lgtm

[vmpstr, 2016-02-24 23:50:40]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2016-02-24 23:50:56]

The CQ bit was unchecked by vmpstr@chromium.org

[vmpstr, 2016-02-24 23:53:01]

On 2016/02/24 01:38:47, tapted wrote:
> On 2016/02/24 00:43:06, vmpstr wrote:
> >
>
https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
> > File chrome/browser/ui/app_list/app_list_prefs.h (right):
> > 
> >
>
https://codereview.chromium.org/1731483003/diff/1/chrome/browser/ui/app_list/...
> > chrome/browser/ui/app_list/app_list_prefs.h:46: AppListInfo(const
AppListInfo&
> > other);
> > On 2016/02/24 00:36:13, tapted wrote:
> > > Pretty sure this can be `= delete` -- only the assignment operator is used
> for
> > > `AppListInfo`
> > 
> > If I delete it, then there's a compiler error on this line:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> > 
> > Here's the error:
> > 
> >
>
/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_pair.h:104:21:
> > error: call to deleted constructor of 'app_list::AppListPrefs::AppListInfo'
> >       : first(__a), second(__b) { }
> >                     ^      ~~~
> >
>
/work/src/build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_map.h:453:29:
> > note: in instantiation of member function 'std::pair<const
> > std::basic_string<char>, app_list::AppListPrefs::AppListInfo>::pair'
requested
> > here
> >           __i = insert(__i, value_type(__k, mapped_type()));
> >                             ^
> > ../../chrome/browser/ui/app_list/app_list_prefs.cc:120:11: note: in
> > instantiation of member function 'std::map<std::basic_string<char>,
> > app_list::AppListPrefs::AppListInfo, std::less<std::basic_string<char> >,
> > std::allocator<std::pair<const std::basic_string<char>,
> > app_list::AppListPrefs::AppListInfo> > >::operator[]' requested here
> >     (*out)[it.key()] = *AppListInfo::CreateAppListInfoFromDict(item_dict);
> >           ^
> > ../../chrome/browser/ui/app_list/app_list_prefs.h:46:5: note: 'AppListInfo'
> has
> > been explicitly marked deleted here
> >     AppListInfo(const AppListInfo& other) = delete;
> >     ^
> > 
> > I think it happens because [] operator creates an empty value and then
inserts
> > it via a copy constructor, but I'm not too sure.
> 
> Ugh. It works fine with libc++. Looks like this is a GCC bug not fixed until
> 4.8. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44436

tapted@, are you OK with this change as is or did you want me to change
something?

[vmpstr, 2016-02-26 00:04:31]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-02-26 00:14:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1731483003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1731483003/1

[commit-bot: I haz the power, 2016-02-26 02:01:00]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-26 02:02:37]

Description was changed from

==========
chrome: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=dcheng@chromium.org, thakis@chromium.org
==========

to

==========
chrome: Add out-of-line copy ctors for complex classes.

This patch adds out of line copy constructors for classes that our
clang-plugin considers heavy. This is an effort to enable copy
constructor checks by default.

BUG=436357
R=dcheng@chromium.org, thakis@chromium.org

Committed: https://crrev.com/b8aacbe5510e9a7aaccb5646904f350bca8e3a7c
Cr-Commit-Position: refs/heads/master@{#377767}
==========

[commit-bot: I haz the power, 2016-02-26 02:02:39]

Patchset 1 (id:??) landed as
https://crrev.com/b8aacbe5510e9a7aaccb5646904f350bca8e3a7c
Cr-Commit-Position: refs/heads/master@{#377767}