rewrite_to_chrome_style: making globals great again.

rewrite_to_chrome_style: making globals great again.

Prefix globals at translation unit / namespace scope to reduce the
chance of a naming collision.

BUG=598175
Committed: https://crrev.com/71dbc98da81b7d8a758a674b1971672bf1573556
Cr-Commit-Position: refs/heads/master@{#383556}

[dcheng, 2016-03-27 07:35:17]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org

[dcheng, 2016-03-27 07:35:18]

This is a mild recommendation in the style guide [1], and might save us from
other collisions down the road.

[1] http://google.github.io/styleguide/cppguide.html#Variable_Names

[danakj, 2016-03-28 18:18:46]

https://codereview.chromium.org/1839573002/diff/20001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):

https://codereview.chromium.org/1839573002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:384:
(decl_context->isTranslationUnit() || decl_context->isNamespace()))
Why isTranslationUnit? I thought the matcher would only match things inside
blink/wtf namespace?

[dcheng, 2016-03-28 18:22:23]

https://codereview.chromium.org/1839573002/diff/20001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):

https://codereview.chromium.org/1839573002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:384:
(decl_context->isTranslationUnit() || decl_context->isNamespace()))
On 2016/03/28 at 18:18:46, danakj wrote:
> Why isTranslationUnit? I thought the matcher would only match things inside
blink/wtf namespace?

Hmm. I guess I was thinking if people had a static at global scope, but then we
wouldn't rewrite them at all. Done.

[danakj, 2016-03-28 18:28:33]

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:379: // Non-const
variables with static storage duration at translation unit or
update this then

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:384:
decl_context->isNamespace())
OK can you have global storage and not be isNamespace?

[dcheng, 2016-03-28 19:50:54]

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:379: // Non-const
variables with static storage duration at translation unit or
On 2016/03/28 at 18:28:33, danakj wrote:
> update this then

Done.

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:384:
decl_context->isNamespace())
On 2016/03/28 at 18:28:33, danakj wrote:
> OK can you have global storage and not be isNamespace?

Yeah, static locals inside functions.

[danakj, 2016-03-28 19:54:06]

LGTM

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):

https://codereview.chromium.org/1839573002/diff/40001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:384:
decl_context->isNamespace())
On 2016/03/28 19:50:54, dcheng wrote:
> On 2016/03/28 at 18:28:33, danakj wrote:
> > OK can you have global storage and not be isNamespace?
> 
> Yeah, static locals inside functions.

ooh, cool

[dcheng, 2016-03-28 19:55:21]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-03-28 19:55:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1839573002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1839573002/60001

[commit-bot: I haz the power, 2016-03-28 20:46:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-28 20:47:54]

Description was changed from

==========
rewrite_to_chrome_style: making globals great again.

Prefix globals at translation unit / namespace scope to reduce the
chance of a naming collision.

BUG=598175
==========

to

==========
rewrite_to_chrome_style: making globals great again.

Prefix globals at translation unit / namespace scope to reduce the
chance of a naming collision.

BUG=598175

Committed: https://crrev.com/71dbc98da81b7d8a758a674b1971672bf1573556
Cr-Commit-Position: refs/heads/master@{#383556}
==========

[commit-bot: I haz the power, 2016-03-28 20:47:55]

Patchset 4 (id:??) landed as
https://crrev.com/71dbc98da81b7d8a758a674b1971672bf1573556
Cr-Commit-Position: refs/heads/master@{#383556}