[DBus] Explicitly instantiate dbusProperty variant

[DBus] Explicitly instantiate dbusProperty variant

dbus::Property<std::vector<uint8> > isn't instantiated in libdbus in non-ChromeOS build + latest clang.
That causes instantiation of the template in dbus_unittests, and causes link failure.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285475

[tzik, 2014-07-23 04:08:46]

PTL

Let me retry my previous link fix, http://crrev.com/280264, which was reverted
due to build breakage on non-clang build.
Latest Chromium-bundled Clang starts failing with the link failure.

[tzik, 2014-07-23 04:22:06]

On 2014/07/23 04:08:46, tzik wrote:
> PTL
> 
> Let me retry my previous link fix, http://crrev.com/280264, which was reverted
> due to build breakage on non-clang build.
> Latest Chromium-bundled Clang starts failing with the link failure.

Hm, the failure happens only on ASan enabled build.

[Nico, 2014-07-23 05:07:17]

https://codereview.chromium.org/408143012/diff/1/dbus/property.h
File dbus/property.h (right):

https://codereview.chromium.org/408143012/diff/1/dbus/property.h#newcode412
dbus/property.h:412: template class CHROME_DBUS_EXPORT Property<uint8>;
This doesn't look right to me. I think you want a template instantiation
declaration here, and a template instantiation definition in a cc file. See
base/strings/string16.h and cc for an example (look for the "extern template" in
the .h and the corresponding line in the .cc)

That should work with both clang and gcc.

[tzik, 2014-07-23 05:29:02]

https://codereview.chromium.org/408143012/diff/1/dbus/property.h
File dbus/property.h (right):

https://codereview.chromium.org/408143012/diff/1/dbus/property.h#newcode412
dbus/property.h:412: template class CHROME_DBUS_EXPORT Property<uint8>;
On 2014/07/23 05:07:16, Nico (away) wrote:
> This doesn't look right to me. I think you want a template instantiation
> declaration here, and a template instantiation definition in a cc file. See
> base/strings/string16.h and cc for an example (look for the "extern template"
in
> the .h and the corresponding line in the .cc)
> 
> That should work with both clang and gcc.

Ah, right. This shouldn't be instantiated in other than libdbus.a.
Updated.

[Nico, 2014-07-23 15:31:00]

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h
File dbus/property.h (right):

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h#newcode411
dbus/property.h:411: template <> void
Property<uint8>::AppendSetValueToWriter(MessageWriter* writer);
Explicitly instantiating a template class instantiates all its methods, so you
can remove all the method instantiations from this file now, I think

[tzik, 2014-07-23 15:39:42]

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h
File dbus/property.h (right):

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h#newcode411
dbus/property.h:411: template <> void
Property<uint8>::AppendSetValueToWriter(MessageWriter* writer);
On 2014/07/23 15:31:00, Nico (away) wrote:
> Explicitly instantiating a template class instantiates all its methods, so you
> can remove all the method instantiations from this file now, I think

I think this is a specialization rather than an instantiation.
Removing them causes compile error like this:
../../dbus/property.cc:220:18: error: explicit specialization of 'Property'
after instantiation

[Nico, 2014-07-23 15:41:26]

lgtm

This file has quite a confusing mix of polymorphism and templates…

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h
File dbus/property.h (right):

https://codereview.chromium.org/408143012/diff/40001/dbus/property.h#newcode411
dbus/property.h:411: template <> void
Property<uint8>::AppendSetValueToWriter(MessageWriter* writer);
On 2014/07/23 15:39:42, tzik wrote:
> On 2014/07/23 15:31:00, Nico (away) wrote:
> > Explicitly instantiating a template class instantiates all its methods, so
you
> > can remove all the method instantiations from this file now, I think
> 
> I think this is a specialization rather than an instantiation.
> Removing them causes compile error like this:
> ../../dbus/property.cc:220:18: error: explicit specialization of 'Property'
> after instantiation

Ah, right.

[tzik, 2014-07-24 02:19:54]

keybuk@, could you double check as a owner?

[keybuk, 2014-07-24 17:39:02]

lgtm

[tzik, 2014-07-25 00:59:41]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2014-07-25 01:02:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tzik@chromium.org/408143012/40001

[commit-bot: I haz the power, 2014-07-25 04:04:32]

Change committed as 285475