Add a space between string literals and PRIuS

Add a space between string literals and PRIuS

This is required to build on iOS (with the Xcode 4.6 toolchain) in
C++11 mode, to avoid the error: "invalid suffix on literal; C++11
requires a space between literal and identifier"

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

[stuartmorgan, 2013-04-05 10:08:54]

akalin for review
CC'ing thakis since I'm curious why Mac didn't have this problem. Was this check
removed from later versions of clang?

[Nico, 2013-04-05 17:05:36]

Hi Stuart,

we don't need this in chromium/mac because we build with
-Wno-reserved-user-defined-literal (also see the comment above that flag in
build/common.gypi).

NaCl has hundreds of violations, so we haven't cleaned this up yet. I don't know
if it's going to be necessary if we want to build in c++11 mode with gcc.

https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_...
File chrome/browser/sync/glue/tab_node_pool.cc (right):

https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_...
chrome/browser/sync/glue/tab_node_pool.cc:35: return base::StringPrintf("%s %"
PRIuS "", machine_tag.c_str(), tab_node_id);
(I think you only need the space in front of PRIus, not after it)

[stuartmorgan, 2013-04-05 18:48:07]

On 2013/04/05 17:05:36, Nico wrote:
> we don't need this in chromium/mac because we build with
> -Wno-reserved-user-defined-literal (also see the comment above that flag in
> build/common.gypi).

Ah, totally missed that. So do you think I should abandon this CL, or is there
interest in working toward fixing it?

https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_...
File chrome/browser/sync/glue/tab_node_pool.cc (right):

https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_...
chrome/browser/sync/glue/tab_node_pool.cc:35: return base::StringPrintf("%s %"
PRIuS "", machine_tag.c_str(), tab_node_id);
On 2013/04/05 17:05:37, Nico wrote:
> (I think you only need the space in front of PRIus, not after it)

Yeah, but I thought that looked really strange since the PRIuS is logically
linked to the %, not the part after, so this seemed clearer. I can change it if
you prefer though.

[Nico, 2013-04-05 18:51:54]

On Fri, Apr 5, 2013 at 11:48 AM, <stuartmorgan@chromium.org> wrote:

> On 2013/04/05 17:05:36, Nico wrote:
>
>> we don't need this in chromium/mac because we build with
>> -Wno-reserved-user-defined-**literal (also see the comment above that
>> flag in
>> build/common.gypi).
>>
>
> Ah, totally missed that. So do you think I should abandon this CL, or is
> there
> interest in working toward fixing it?


I'm not sure. I supposed we might want to fix it eventually? In any case,
it's probably better if your compiler flags are as similar as possible, so
I'd probably just add that flag for ios and delay making this decision.


>
>
>
> https://codereview.chromium.**org/13721002/diff/1/chrome/**
>
browser/sync/glue/tab_node_**pool.cc<https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_node_pool.cc>
> File chrome/browser/sync/glue/tab_**node_pool.cc (right):
>
> https://codereview.chromium.**org/13721002/diff/1/chrome/**
>
browser/sync/glue/tab_node_**pool.cc#newcode35<https://codereview.chromium.org/13721002/diff/1/chrome/browser/sync/glue/tab_node_pool.cc#newcode35>
> chrome/browser/sync/glue/tab_**node_pool.cc:35: return
> base::StringPrintf("%s %" PRIuS "", machine_tag.c_str(), tab_node_id);
> On 2013/04/05 17:05:37, Nico wrote:
>
>> (I think you only need the space in front of PRIus, not after it)
>>
>
> Yeah, but I thought that looked really strange since the PRIuS is
> logically linked to the %, not the part after, so this seemed clearer. I
> can change it if you prefer though.
>
>
https://codereview.chromium.**org/13721002/<https://codereview.chromium.org/1...
>

[stuartmorgan, 2013-04-05 18:54:30]

On 2013/04/05 18:51:54, Nico wrote:
> In any case, it's probably better if your compiler flags are as
> similar as possible, so I'd probably just add that flag for ios

Yep, already added that to my TODO list.

I'd vote for going ahead and landing this patch anyway since it doesn't hurt, we
might want it someday, and it's already done, but I'll leave the call to akalin.

[Nico, 2013-04-05 18:56:20]

On Fri, Apr 5, 2013 at 11:54 AM, <stuartmorgan@chromium.org> wrote:

> On 2013/04/05 18:51:54, Nico wrote:
>
>> In any case, it's probably better if your compiler flags are as
>> similar as possible, so I'd probably just add that flag for ios
>>
>
> Yep, already added that to my TODO list.
>
> I'd vote for going ahead and landing this patch anyway since it doesn't
> hurt, we
> might want it someday, and it's already done, but I'll leave the call to
> akalin.
>

Works for me :-)


>
>
https://codereview.chromium.**org/13721002/<https://codereview.chromium.org/1...
>

[akalin, 2013-04-10 00:16:55]

LGTM

[commit-bot: I haz the power, 2013-04-10 00:17:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stuartmorgan@chromium.org/13721002/1

[commit-bot: I haz the power, 2013-04-10 03:55:20]

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-10 06:50:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stuartmorgan@chromium.org/13721002/1

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

Change committed as 193361