Force the safeToCompareToEmptyOrDeleted trait to be renamed correctly

Force the safeToCompareToEmptyOrDeleted trait to be renamed correctly

This simply renames all varDecl and varDeclRefs with that name to
safe_to_compare_to_empty_or_deleted. It leaves matchers for outside
of the blink namespace still.

R=dcheng@chromium.org
BUG=685264
Review-Url: https://codereview.chromium.org/2663043002
Cr-Commit-Position: refs/heads/master@{#447136}
Committed: https://chromium.googlesource.com/chromium/src/+/bc7ab72c78b36430b6c77078d2ad9790d1bf79c0

[danakj, 2017-01-30 22:57:34]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 22:58:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2017-01-30 22:59:04]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 22:59:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 23:12:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 23:12:17]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-01-31 00:01:06]

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

https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
Is it OK to skip is_type_trait_value here now?

[danakj, 2017-01-31 00:02:32]

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

https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
On 2017/01/31 00:01:05, dcheng wrote:
> Is it OK to skip is_type_trait_value here now?

Yeh, we'll just do it for all names that match now no matter what, in any
namespace, whatever const-ness, etc.

[danakj, 2017-01-31 00:03:06]

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

https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
On 2017/01/31 00:02:32, danakj (slow) wrote:
> On 2017/01/31 00:01:05, dcheng wrote:
> > Is it OK to skip is_type_trait_value here now?
> 
> Yeh, we'll just do it for all names that match now no matter what, in any
> namespace, whatever const-ness, etc.

It will match any vardecl in the codebase, but it has to have that name, and
it's unique to this.

[dcheng, 2017-01-31 00:08:31]

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

https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
On 2017/01/31 00:03:06, danakj (slow) wrote:
> On 2017/01/31 00:02:32, danakj (slow) wrote:
> > On 2017/01/31 00:01:05, dcheng wrote:
> > > Is it OK to skip is_type_trait_value here now?
> > 
> > Yeh, we'll just do it for all names that match now no matter what, in any
> > namespace, whatever const-ness, etc.
> 
> It will match any vardecl in the codebase, but it has to have that name, and
> it's unique to this.

I'm probably missing something, but don't we need is_type_trait_value here to
camel case the other type traits too? Or are they already renamed correctly, and
it's just this one that the rewriter struggles with?

[danakj, 2017-01-31 00:09:58]

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

https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
On 2017/01/31 00:08:31, dcheng wrote:
> On 2017/01/31 00:03:06, danakj (slow) wrote:
> > On 2017/01/31 00:02:32, danakj (slow) wrote:
> > > On 2017/01/31 00:01:05, dcheng wrote:
> > > > Is it OK to skip is_type_trait_value here now?
> > > 
> > > Yeh, we'll just do it for all names that match now no matter what, in any
> > > namespace, whatever const-ness, etc.
> > 
> > It will match any vardecl in the codebase, but it has to have that name, and
> > it's unique to this.
> 
> I'm probably missing something, but don't we need is_type_trait_value here to
> camel case the other type traits too? Or are they already renamed correctly,
and
> it's just this one that the rewriter struggles with?

Right, the other ones are all named value and are covered above as such.

[dcheng, 2017-01-31 00:18:58]

On 2017/01/31 00:09:58, danakj (slow) wrote:
>
https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
> File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp (right):
> 
>
https://codereview.chromium.org/2663043002/diff/20001/tools/clang/rewrite_to_...
> tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
> type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
> On 2017/01/31 00:08:31, dcheng wrote:
> > On 2017/01/31 00:03:06, danakj (slow) wrote:
> > > On 2017/01/31 00:02:32, danakj (slow) wrote:
> > > > On 2017/01/31 00:01:05, dcheng wrote:
> > > > > Is it OK to skip is_type_trait_value here now?
> > > > 
> > > > Yeh, we'll just do it for all names that match now no matter what, in
any
> > > > namespace, whatever const-ness, etc.
> > > 
> > > It will match any vardecl in the codebase, but it has to have that name,
and
> > > it's unique to this.
> > 
> > I'm probably missing something, but don't we need is_type_trait_value here
to
> > camel case the other type traits too? Or are they already renamed correctly,
> and
> > it's just this one that the rewriter struggles with?
> 
> Right, the other ones are all named value and are covered above as such.

It seems a bit subtle to me, and possibly worth pointing out (as a reader, this
is a question I would wonder) in a comment or some other way. If it doesn't
hurt, maybe the type trait matcher should just be included, even if it's a
no-op?

[danakj, 2017-01-31 00:21:36]

On Mon, Jan 30, 2017 at 7:18 PM, <dcheng@chromium.org> wrote:

> On 2017/01/31 00:09:58, danakj (slow) wrote:
> >
> https://codereview.chromium.org/2663043002/diff/20001/
> tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
> > File tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
> (right):
> >
> >
> https://codereview.chromium.org/2663043002/diff/20001/
> tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp#newcode1491
> > tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp:1491: auto
> > type_trait_decl_matcher = id("decl", varDecl(isKnownTraitName()));
> > On 2017/01/31 00:08:31, dcheng wrote:
> > > On 2017/01/31 00:03:06, danakj (slow) wrote:
> > > > On 2017/01/31 00:02:32, danakj (slow) wrote:
> > > > > On 2017/01/31 00:01:05, dcheng wrote:
> > > > > > Is it OK to skip is_type_trait_value here now?
> > > > >
> > > > > Yeh, we'll just do it for all names that match now no matter what,
> in
> any
> > > > > namespace, whatever const-ness, etc.
> > > >
> > > > It will match any vardecl in the codebase, but it has to have that
> name,
> and
> > > > it's unique to this.
> > >
> > > I'm probably missing something, but don't we need is_type_trait_value
> here
> to
> > > camel case the other type traits too? Or are they already renamed
> correctly,
> > and
> > > it's just this one that the rewriter struggles with?
> >
> > Right, the other ones are all named value and are covered above as such.
>
> It seems a bit subtle to me, and possibly worth pointing out (as a reader,
> this
> is a question I would wonder) in a comment or some other way. If it doesn't
> hurt, maybe the type trait matcher should just be included, even if it's a
> no-op?
>

I think it's what was wrong before. It was still missing things, creating
conflicting edits. I could continue to track down all the reasons why but
this seems better for now.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2017-01-31 00:22:14]

comment added

[danakj, 2017-01-31 00:23:20]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 00:23:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2017-01-31 00:30:41]

LGTM

[commit-bot: I haz the power, 2017-01-31 00:34:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 00:34:26]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-31 00:37:17]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2017-01-31 00:37:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 00:43:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485823037926100,
"parent_rev": "4547e3a30bc0665e903fbae2338d39980560b68e", "commit_rev":
"bc7ab72c78b36430b6c77078d2ad9790d1bf79c0"}

[commit-bot: I haz the power, 2017-01-31 00:43:43]

Description was changed from

==========
Force the safeToCompareToEmptyOrDeleted trait to be renamed correctly

This simply renames all varDecl and varDeclRefs with that name to
safe_to_compare_to_empty_or_deleted. It leaves matchers for outside
of the blink namespace still.

R=dcheng@chromium.org
BUG=685264
==========

to

==========
Force the safeToCompareToEmptyOrDeleted trait to be renamed correctly

This simply renames all varDecl and varDeclRefs with that name to
safe_to_compare_to_empty_or_deleted. It leaves matchers for outside
of the blink namespace still.

R=dcheng@chromium.org
BUG=685264

Review-Url: https://codereview.chromium.org/2663043002
Cr-Commit-Position: refs/heads/master@{#447136}
Committed:
https://chromium.googlesource.com/chromium/src/+/bc7ab72c78b36430b6c77078d2ad...
==========

[commit-bot: I haz the power, 2017-01-31 00:43:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bc7ab72c78b36430b6c77078d2ad...