Fix build warnings with gcc4.8

Fix build warnings with gcc4.8

There are a lot of warnings when build chromium with gcc4.8, which are
in "typedef ‘foo’ locally defined but not used" pattern. Add
"-Wno-unused-local-typedefs" to suppress these warnings.

BUG=227506
TEST=build with gcc4.8 and check no above warnings appear.

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

[qiankun, 2013-04-10 06:30:37]

Fix build warnings with gcc 4.8. All these warnings are treated as errors which
block chromium build. Please help to review.

[Torne, 2013-04-10 09:36:57]

Should we have a tracking bug for actually fixing these warnings for real and
then removing this flag?

Eventually we will build with gcc 4.8 on other platforms too, since toolchain
versions march forwards inexorably, and it sounds like this might actually
describe possible code cleanups..

[qiankun, 2013-04-10 09:47:55]

It makes sense. There is already a bug on this. Updated this issue.
On 2013/04/10 09:36:57, Torne wrote:
> Should we have a tracking bug for actually fixing these warnings for real and
> then removing this flag?
> 
> Eventually we will build with gcc 4.8 on other platforms too, since toolchain
> versions march forwards inexorably, and it sounds like this might actually
> describe possible code cleanups..

[Torne, 2013-04-10 10:38:10]

On 2013/04/10 09:47:55, qiankun.miao wrote:
> It makes sense. There is already a bug on this. Updated this issue.

I meant, the comment in common.gypi should be a TODO which references the bug to
clean up the warnings. That bug doesn't really have any real information. We
should repurpose it, or file a new one, to actually discuss whether we care
about fixing these warnings or not, and if we do, actually reference it from the
file.

> On 2013/04/10 09:36:57, Torne wrote:
> > Should we have a tracking bug for actually fixing these warnings for real
and
> > then removing this flag?
> > 
> > Eventually we will build with gcc 4.8 on other platforms too, since
toolchain
> > versions march forwards inexorably, and it sounds like this might actually
> > describe possible code cleanups..

[qiankun, 2013-04-12 01:56:22]

On 2013/04/10 10:38:10, Torne wrote:
> On 2013/04/10 09:47:55, qiankun.miao wrote:
> > It makes sense. There is already a bug on this. Updated this issue.
> 
> I meant, the comment in common.gypi should be a TODO which references the bug
to
> clean up the warnings. That bug doesn't really have any real information. We
> should repurpose it, or file a new one, to actually discuss whether we care
> about fixing these warnings or not, and if we do, actually reference it from
the
> file.
Added a TODO in the comment, also updated the bug with a comment. I am not sure
it's the right method. If not just correct me, thanks.
> > On 2013/04/10 09:36:57, Torne wrote:
> > > Should we have a tracking bug for actually fixing these warnings for real
> and
> > > then removing this flag?
> > > 
> > > Eventually we will build with gcc 4.8 on other platforms too, since
> toolchain
> > > versions march forwards inexorably, and it sounds like this might actually
> > > describe possible code cleanups..

[Torne, 2013-04-12 09:48:32]

lgtm

[commit-bot: I haz the power, 2013-04-12 12:03:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qiankun.miao@intel.com/13811009/6001

[commit-bot: I haz the power, 2013-04-12 12:12:06]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Raphael Kubo da Costa (rakuco), 2013-04-12 14:16:39]

On 2013/04/12 12:12:06, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on linux_clang.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

Since clang barfs at this unknown compiler flag, it probably makes sense to
check for gcc_version's value (clang will report 4.2.1) and only set it for >=
'48'.

[qiankun, 2013-04-13 01:21:30]

https://codereview.chromium.org/13811009/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/13811009/diff/20001/build/common.gypi#newcode...
build/common.gypi:2561: 'conditions': [
  Add a conditions section to check if gcc_version>=48. But it seems this
conditions section doesn't work. Because this flag doesn't appear in the
generated *.mk files. 
  If use 'target_conditions' instead of 'conditions', got the following error
"gyp: name 'gcc_version' is not defined while evaluating condition
'gcc_version>=48' in src/v8/tools/gyp/v8.gyp".
  If use the following code, it works.
        'target_conditions': [
          # Don't warn about the "typedef 'foo' locally defined but not used"
          # for gcc 4.8.
          # TODO: remove this flag once all builds work. See crbug.com/227506
          [ '1==1', {
            'cflags': [
              '-Wno-unused-local-typedefs',
            ],
          }],
        ],
   Could you give some comments on these problems? What's the correct method to
check gcc_version here?

[Raphael Kubo da Costa (rakuco), 2013-04-15 10:57:07]

https://codereview.chromium.org/13811009/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/13811009/diff/20001/build/common.gypi#newcode...
build/common.gypi:2561: 'conditions': [
On 2013/04/13 01:21:30, qiankun.miao wrote:
>   Add a conditions section to check if gcc_version>=48. But it seems this
> conditions section doesn't work. Because this flag doesn't appear in the
> generated *.mk files.
>   If use 'target_conditions' instead of 'conditions', got the following error
> "gyp: name 'gcc_version' is not defined while evaluating condition
> [...]
>    Could you give some comments on these problems? What's the correct method
to
> check gcc_version here?

Have you tried running `gyp_chromium' with the CHROMIUM_GYP_SYNTAX_CHECK
environment variable set? I think the problem with putting this section here is
that there's also a `conditions' section defined later (around line 2706). I
don't really know what's wrong with the `target_conditions' part.

In any case, it should all work if you just add this check to the existing
`conditions' section.

[qiankun, 2013-04-15 11:29:45]

On 2013/04/15 10:57:07, Raphael Kubo da Costa (rakuco) wrote:
> https://codereview.chromium.org/13811009/diff/20001/build/common.gypi
> File build/common.gypi (right):
> 
>
https://codereview.chromium.org/13811009/diff/20001/build/common.gypi#newcode...
> build/common.gypi:2561: 'conditions': [
> On 2013/04/13 01:21:30, qiankun.miao wrote:
> >   Add a conditions section to check if gcc_version>=48. But it seems this
> > conditions section doesn't work. Because this flag doesn't appear in the
> > generated *.mk files.
> >   If use 'target_conditions' instead of 'conditions', got the following
error
> > "gyp: name 'gcc_version' is not defined while evaluating condition
> > [...]
> >    Could you give some comments on these problems? What's the correct method
> to
> > check gcc_version here?
> 
> Have you tried running `gyp_chromium' with the CHROMIUM_GYP_SYNTAX_CHECK
> environment variable set? I think the problem with putting this section here
is
> that there's also a `conditions' section defined later (around line 2706). I
> don't really know what's wrong with the `target_conditions' part.
> 
> In any case, it should all work if you just add this check to the existing
> `conditions' section.

Thanks for your hits. I ran 'gyp_chromium' with the CHROMIUM_GYP_SYNTAX_CHECK
environment variable set and got 'conditions' repeated error. So merged
gcc_version checking code into the 'conditions' section which is already
contained in 'target_defaults' section. It works now. 

Torne, could you help to re-review it?

[qiankun, 2013-04-15 11:32:30]

> Thanks for your hits. 
Sorry, I meant hints here, just a typo:)

[Torne, 2013-04-15 11:32:49]

LGTM, thanks for bearing with us :)

[commit-bot: I haz the power, 2013-04-17 23:24:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qiankun.miao@intel.com/13811009/3002

[commit-bot: I haz the power, 2013-04-18 01:18:53]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-04-18 04:29:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qiankun.miao@intel.com/13811009/3002

[commit-bot: I haz the power, 2013-04-18 05:31:10]

Change committed as 194779