ios: Enable -Wunused-functions.

ios: Enable -Wunused-functions.

BUG=315884
TBR=net,sql owners

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

[Nico, 2013-11-13 01:38:07]

Not sure if you guys want this, but all other (clang-using) platforms build with
it now and it allowed us to delete > 1.5 kloc

[stuartmorgan, 2013-11-13 19:42:24]

LGTM.

+blundell as it may require some tweaks (or temporary downstream removal) in the
next trunk merge, due to the current state of forking.

[commit-bot: I haz the power, 2013-11-13 19:52:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/64273005/110001

[commit-bot: I haz the power, 2013-11-13 20:16:34]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-11-13 20:25:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/64273005/110001

[blundell, 2013-11-13 20:44:03]

On 2013/11/13 20:25:59, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/thakis%40chromium.org/64273005/110001

I'm not sure this is a good idea in the current state of the world. Downstream
we use little bits of //chrome all over the place; I'm afraid that we'd have to
introduce a lot of forking just to get things to compile with this flag, which
would increase the chances of merge conflicts needlessly. Moreover, we'd have to
satisfy this flag anew at every merge.

We could of course leave it turned off downstream and on upstream; I'm not sure
if there's value to that.

Nico, is there an easy way for me to see how many warnings this would generate
for us downstream right now so I can see what the damage is?

[Nico, 2013-11-13 20:45:11]

On Wed, Nov 13, 2013 at 12:44 PM, <blundell@chromium.org> wrote:

> On 2013/11/13 20:25:59, I haz the power (commit-bot) wrote:
>
>> CQ is trying da patch. Follow status at
>> https://chromium-status.appspot.com/cq/thakis%
>> 40chromium.org/64273005/110001
>>
>
> I'm not sure this is a good idea in the current state of the world.
> Downstream
> we use little bits of //chrome all over the place; I'm afraid that we'd
> have to
> introduce a lot of forking just to get things to compile with this flag,
> which
> would increase the chances of merge conflicts needlessly. Moreover, we'd
> have to
> satisfy this flag anew at every merge.
>
> We could of course leave it turned off downstream and on upstream; I'm not
> sure
> if there's value to that.
>
> Nico, is there an easy way for me to see how many warnings this would
> generate
> for us downstream right now so I can see what the damage is?
>

Patch this in and build?

Uncheck CQ if you think it's not even worth giving it a try.


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

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

[stuartmorgan, 2013-11-13 21:41:50]

On 2013/11/13 20:44:03, blundell wrote:
> We could of course leave it turned off downstream and on upstream; I'm not
sure
> if there's value to that.

I think there is (which is why I approved it :) ). The state we want to be in is
that we have the same warnings as everyone else, and this reflects that. If it's
currently impractical downstream due to forking, then (temporarily) changing the
setting in the forked repo, as part of the forking, makes sense.

(I'm not sure that it will be impractical; we already build with the 'lighter'
version of the warning and it hasn't been a major problem.)

[commit-bot: I haz the power, 2013-11-14 01:46:19]

Change committed as 235014

[blundell, 2013-11-15 12:24:12]

For some reason I stopped seeing email updates on this CL; hence my delayed
reply.

OK, SGTM. +noyau@ because this CL didn't get picked up in today's merge.

[noyau (Ping after 24h), 2013-11-15 12:42:48]

On 2013/11/15 12:24:12, blundell wrote:
> For some reason I stopped seeing email updates on this CL; hence my delayed
> reply.
> 
> OK, SGTM. +noyau@ because this CL didn't get picked up in today's merge.

Where is the CL removing the 1.5kloc?

There is some code that iOS uses that is not used by any other platform. Was all
that code removed? I see the end goal you are pursuing, but until everything is
component based this is only going to make our merges way harder.

This could also be the case for the code you ifdefed out in this CL, maybe it's
used downstream by us, I haven't checked yet...