Fix build failures due to unused functions when building with clang on linux.

Fix build failures due to unused functions when building with clang on linux.

The reason of this failure on recent ubuntu version is that which versinos use high version  of glib, 2.37.5 or upper. In details, glib 2.37.5 includes a new API for instance private data: G_DEFINE_TYPE_WITH_PRIVATE. that is the unused function. I assume google build servers have been using lower version of Ubuntu than mine. but I believe we'll face this break near soon when moving  to next ubuntu LTS, 14.04. So I hope this to be fixed here.
Gaurded gobject style definitions with proper #pragma clang.

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

[changseok, 2014-02-27 15:33:05]

Would you review this? This is my first patch to chromium so that something
might be wrong or missed. Please point it out if so.

[Elliot Glaysher, 2014-02-27 18:22:51]

On 2014/02/27 15:33:05, changseok wrote:
> Would you review this? This is my first patch to chromium so that something
> might be wrong or missed. Please point it out if so.

So I'm building with our clang right now, and this works. I'm not sure if we
support system clang.

(We certainly don't want to add all these pragmas...)

[changseok, 2014-03-04 08:03:34]

On 2014/02/27 18:22:51, Elliot Glaysher wrote:
> On 2014/02/27 15:33:05, changseok wrote:
> > Would you review this? This is my first patch to chromium so that something
> > might be wrong or missed. Please point it out if so.
> 
> So I'm building with our clang right now, and this works. I'm not sure if we
> support system clang.
> 
> (We certainly don't want to add all these pragmas...)

That's odd. I'm still facing same problem on my system. Ubuntu 13.10 which
doesn't have system clang.
The clang I used is ./src/third_party/llvm-build/Release+Asserts/bin/clang, the
version 3.5. Is there something I missed?

[changseok, 2014-04-30 15:23:58]

On 2014/03/04 08:03:34, changseok wrote:
> On 2014/02/27 18:22:51, Elliot Glaysher wrote:
> > On 2014/02/27 15:33:05, changseok wrote:
> > > Would you review this? This is my first patch to chromium so that
something
> > > might be wrong or missed. Please point it out if so.
> > 
> > So I'm building with our clang right now, and this works. I'm not sure if we
> > support system clang.
> > 
> > (We certainly don't want to add all these pragmas...)
> 
> That's odd. I'm still facing same problem on my system. Ubuntu 13.10 which
> doesn't have system clang.
> The clang I used is ./src/third_party/llvm-build/Release+Asserts/bin/clang,
the
> version 3.5. Is there something I missed?

After discussion in bug 347549, I found this is caused by using glib 2.37.5 or
newer. It added a new api and that's the unused function.
It means any ubuntu users using recent ubuntu and clang would face this breaks.
So I hope to fix this.

> glib2.0 (2.37.5-1) experimental; urgency=low
> + add a new API for instance private data: G_DEFINE_TYPE_WITH_PRIVATE

[changseok, 2014-04-30 15:28:52]

Please take a look again.

[Elliot Glaysher, 2014-04-30 17:18:08]

thakis: what's your take here?

[Nico, 2014-04-30 17:20:05]

I'd rather put the G_DEFINE_TYPE in the version ifdef instead of doing
diagnostic push/pops. Is that possible?

[Elliot Glaysher, 2014-04-30 17:21:55]

On 2014/04/30 17:20:05, Nico wrote:
> I'd rather put the G_DEFINE_TYPE in the version ifdef instead of doing
> diagnostic push/pops. Is that possible?

No, because the G_DEFINE_TYPE does several things, only of which one is unused.

[Nico, 2014-04-30 17:28:07]

On Wed, Apr 30, 2014 at 10:21 AM, <erg@chromium.org> wrote:

> On 2014/04/30 17:20:05, Nico wrote:
>
>> I'd rather put the G_DEFINE_TYPE in the version ifdef instead of doing
>> diagnostic push/pops. Is that possible?
>>
>
> No, because the G_DEFINE_TYPE does several things, only of which one is
> unused.
>

Is there a macro that doesn't generate the unused bits? The bug mentions
G_DEFINE_TYPE_WITH_PRIVATE, would doing "if ver < X: use G_DEFINE_TYPE,
else G_DEFINE_TYPE_WITH_PRIVATE" do the trick?


>
> https://codereview.chromium.org/183223002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-04-30 17:50:23]

(The more general point is that we're very interested in getting this fixed, but
adding pragma diagnostics is not the right way. Ideally we can figure out a
"real" fix for this, like the different macros I just suggested. If that's not
possible, we can add -Wno-unused-functions to the gtk2ui target's gyp file.)

(As is, this patch won't build with gcc btw.)

[changseok, 2014-05-01 14:53:21]

On 2014/04/30 17:50:23, Nico wrote:
> (The more general point is that we're very interested in getting this fixed,
but
> adding pragma diagnostics is not the right way. Ideally we can figure out a
> "real" fix for this, like the different macros I just suggested. If that's not
> possible, we can add -Wno-unused-functions to the gtk2ui target's gyp file.)
> 
> (As is, this patch won't build with gcc btw.)

I spent more time to look into this.
G_DEFINE_TYPE -> G_DEFINE_TYPE_EXTENDED -> _G_DEFINE_TYPE_EXTENDED_BEGIN.
The _G_DEFINE_TYPE_EXTENDED_BEGIN includes an inline function
'type_name##_get_instance_private'
Conditional using G_DEFINE_TYPE_WITH_PRIVATE is not helpful here and I couldn't
find an alternative macro.
So let me add -Wno-unused-function to cflags for gtk2ui for linux & clang build.

BTW, the gcc build break is not due to this patch I think. Let me check it
again.

[Nico, 2014-05-01 18:29:00]

That looks better, thanks! l g t m once the CLA question is cleared.

https://codereview.chromium.org/183223002/diff/40001/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/183223002/diff/40001/AUTHORS#newcode63
AUTHORS:63: ChangSeok Oh <changseok.oh@collabora.com>
As far as I can tell, you signed the CLA with <shivamidow@gmail.com> – can you
either use that address here, or resign the CLA with this new address?

https://codereview.chromium.org/183223002/diff/40001/chrome/browser/ui/libgtk...
File chrome/browser/ui/libgtk2ui/libgtk2ui.gyp (right):

https://codereview.chromium.org/183223002/diff/40001/chrome/browser/ui/libgtk...
chrome/browser/ui/libgtk2ui/libgtk2ui.gyp:91: [ 'OS=="linux" and clang==1', {
nit: You don't really need the OS=="linux" bit, this file is only used on linux.

[changseok, 2014-05-02 03:06:25]

https://codereview.chromium.org/183223002/diff/40001/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/183223002/diff/40001/AUTHORS#newcode63
AUTHORS:63: ChangSeok Oh <changseok.oh@collabora.com>
On 2014/05/01 18:29:01, Nico wrote:
> As far as I can tell, you signed the CLA with <mailto:shivamidow@gmail.com>
–&nbsp;can you
> either use that address here, or resign the CLA with this new address?

O.K Let me change it to shivamidow@gmail.com It's not a big deal. :)

https://codereview.chromium.org/183223002/diff/40001/chrome/browser/ui/libgtk...
File chrome/browser/ui/libgtk2ui/libgtk2ui.gyp (right):

https://codereview.chromium.org/183223002/diff/40001/chrome/browser/ui/libgtk...
chrome/browser/ui/libgtk2ui/libgtk2ui.gyp:91: [ 'OS=="linux" and clang==1', {
On 2014/05/01 18:29:01, Nico wrote:
> nit: You don't really need the OS=="linux" bit, this file is only used on
linux.
Done.

[Nico, 2014-05-02 04:34:02]

lgtm, thanks!

[Nico, 2014-05-02 04:34:08]

The CQ bit was checked by thakis@chromium.org

[Nico, 2014-05-02 04:34:12]

The CQ bit was unchecked by thakis@chromium.org

[Nico, 2014-05-02 04:35:10]

…oh, one last comment: Always include the title line as first line in the
description too, because only the description makes it into the commit message.

I've copied "Fix build failures due to unused functions when building with clang
on linux." as the first line into the description for you for this patch set.

[Nico, 2014-05-02 04:35:26]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2014-05-02 04:38:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/shivamidow@gmail.com/183223002/60001

[changseok, 2014-05-02 05:21:36]

On 2014/05/02 04:35:10, Nico wrote:
> …oh, one last comment: Always include the title line as first line in the
> description too, because only the description makes it into the commit
message.
> 
> I've copied "Fix build failures due to unused functions when building with
clang
> on linux." as the first line into the description for you for this patch set.

Oops Sorry. Good to know. Let me keep it in mind. Thanks. :)

[commit-bot: I haz the power, 2014-05-02 17:18:04]

Change committed as 267834