Capitalize WTF-based type traits as lower_case

Capitalize WTF-based type traits as lower_case

This uses a whitelist to consistently name type traits defined
for WTF structures globally throughout chromium.

R=dcheng
BUG=598177
Review-Url: https://codereview.chromium.org/2649933003
Cr-Commit-Position: refs/heads/master@{#445755}
Committed: https://chromium.googlesource.com/chromium/src/+/5974c5c41238e402ecf1d0918a5e629e95536eb0

[danakj, 2017-01-23 22:31:53]

Description was changed from

==========
traits

R=dcheng
BUG=
==========

to

==========
Capitalize WTF-based type traits as lower_case

This uses a whitelist to consistently name type traits defined
for WTF structures globally throughout chromium.

R=dcheng
BUG=598177
==========

[danakj, 2017-01-23 22:32:30]

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

[danakj, 2017-01-23 22:33:33]

This isn't the prettiest but best I could think of.

[commit-bot: I haz the power, 2017-01-23 22:33:55]

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-23 22:45:52]

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

[commit-bot: I haz the power, 2017-01-23 22:45:53]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-24 15:52:15]

bump

[dcheng, 2017-01-24 16:20:30]

On 2017/01/24 15:52:15, danakj (slow) wrote:
> bump

Sorry. I've been thinking about whether there's an alternate way other than
adding for_type_traits to each GetNameDecl overload.
It /almost/ feel like the MatchCallback subclasses should have a RenamePolicy
enum to help specify the way to rename things (e.g. RenamePolicy::HACKER_CASE,
RenamePolicy::UPPER_CAMEL,
RenamePolicy::HACKER_CASE_WITH_TRAILING_UNDERSCORE)... but that requires a lot
more moving thing around (things like CanBeEvaluatedAtCompileTime would need to
become a matcher, the matchers would have to be refactored out of
GetNameForDecl(clang::FunctionDecl*, std::string& new_name), etc...

IDK, any other ideas?

[danakj, 2017-01-24 16:25:49]

On 2017/01/24 16:20:30, dcheng wrote:
> On 2017/01/24 15:52:15, danakj (slow) wrote:
> > bump
> 
> Sorry. I've been thinking about whether there's an alternate way other than
> adding for_type_traits to each GetNameDecl overload.
> It /almost/ feel like the MatchCallback subclasses should have a RenamePolicy
> enum to help specify the way to rename things (e.g. RenamePolicy::HACKER_CASE,
> RenamePolicy::UPPER_CAMEL,
> RenamePolicy::HACKER_CASE_WITH_TRAILING_UNDERSCORE)... but that requires a lot
> more moving thing around (things like CanBeEvaluatedAtCompileTime would need
to
> become a matcher, the matchers would have to be refactored out of
> GetNameForDecl(clang::FunctionDecl*, std::string& new_name), etc...

Yeh.. that feels like a rewrite of everything.

> IDK, any other ideas?

I considered making a separate subclass for typetraits instead of using
VarDeclMatcher's class. It's a bit of duplicated code around getting the
original name if it exists though, but it could CamelCaseToUnderscoreCase
directly.

[dcheng, 2017-01-24 17:34:45]

On 2017/01/24 16:25:49, danakj (slow) wrote:
> On 2017/01/24 16:20:30, dcheng wrote:
> > On 2017/01/24 15:52:15, danakj (slow) wrote:
> > > bump
> > 
> > Sorry. I've been thinking about whether there's an alternate way other than
> > adding for_type_traits to each GetNameDecl overload.
> > It /almost/ feel like the MatchCallback subclasses should have a
RenamePolicy
> > enum to help specify the way to rename things (e.g.
RenamePolicy::HACKER_CASE,
> > RenamePolicy::UPPER_CAMEL,
> > RenamePolicy::HACKER_CASE_WITH_TRAILING_UNDERSCORE)... but that requires a
lot
> > more moving thing around (things like CanBeEvaluatedAtCompileTime would need
> to
> > become a matcher, the matchers would have to be refactored out of
> > GetNameForDecl(clang::FunctionDecl*, std::string& new_name), etc...
> 
> Yeh.. that feels like a rewrite of everything.

Arguably, we should have been doing this to begin with. But then we added too
much matching logic into GetNameForDecl().

> 
> > IDK, any other ideas?
> 
> I considered making a separate subclass for typetraits instead of using
> VarDeclMatcher's class. It's a bit of duplicated code around getting the
> original name if it exists though, but it could CamelCaseToUnderscoreCase
> directly.

I think the right approach here is what's mentioned above; perhaps I'll take a
stab at it later (in which case, the current CL will be easier to refactor than
duplicating code).

So... meh. LGTM =)

[danakj, 2017-01-24 17:35:12]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2017-01-24 17:35:36]

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

[danakj, 2017-01-24 17:36:20]

On Tue, Jan 24, 2017 at 12:34 PM, <dcheng@chromium.org> wrote:

> On 2017/01/24 16:25:49, danakj (slow) wrote:
> > On 2017/01/24 16:20:30, dcheng wrote:
> > > On 2017/01/24 15:52:15, danakj (slow) wrote:
> > > > bump
> > >
> > > Sorry. I've been thinking about whether there's an alternate way other
> than
> > > adding for_type_traits to each GetNameDecl overload.
> > > It /almost/ feel like the MatchCallback subclasses should have a
> RenamePolicy
> > > enum to help specify the way to rename things (e.g.
> RenamePolicy::HACKER_CASE,
> > > RenamePolicy::UPPER_CAMEL,
> > > RenamePolicy::HACKER_CASE_WITH_TRAILING_UNDERSCORE)... but that
> requires a
> lot
> > > more moving thing around (things like CanBeEvaluatedAtCompileTime
> would need
> > to
> > > become a matcher, the matchers would have to be refactored out of
> > > GetNameForDecl(clang::FunctionDecl*, std::string& new_name), etc...
> >
> > Yeh.. that feels like a rewrite of everything.
>
> Arguably, we should have been doing this to begin with. But then we added
> too
> much matching logic into GetNameForDecl().
>
> >
> > > IDK, any other ideas?
> >
> > I considered making a separate subclass for typetraits instead of using
> > VarDeclMatcher's class. It's a bit of duplicated code around getting the
> > original name if it exists though, but it could CamelCaseToUnderscoreCase
> > directly.
>
> I think the right approach here is what's mentioned above; perhaps I'll
> take a
> stab at it later (in which case, the current CL will be easier to refactor
> than
> duplicating code).
>

FWIW I do question if it'd be worth the time, unless it outlived this one
project.


>
> So... meh. LGTM =)
>

SG


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

-- 
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.

[dcheng, 2017-01-24 17:41:05]

On 2017/01/24 17:36:20, danakj (slow) wrote:
> On Tue, Jan 24, 2017 at 12:34 PM, <mailto:dcheng@chromium.org> wrote:
> 
> > On 2017/01/24 16:25:49, danakj (slow) wrote:
> > > On 2017/01/24 16:20:30, dcheng wrote:
> > > > On 2017/01/24 15:52:15, danakj (slow) wrote:
> > > > > bump
> > > >
> > > > Sorry. I've been thinking about whether there's an alternate way other
> > than
> > > > adding for_type_traits to each GetNameDecl overload.
> > > > It /almost/ feel like the MatchCallback subclasses should have a
> > RenamePolicy
> > > > enum to help specify the way to rename things (e.g.
> > RenamePolicy::HACKER_CASE,
> > > > RenamePolicy::UPPER_CAMEL,
> > > > RenamePolicy::HACKER_CASE_WITH_TRAILING_UNDERSCORE)... but that
> > requires a
> > lot
> > > > more moving thing around (things like CanBeEvaluatedAtCompileTime
> > would need
> > > to
> > > > become a matcher, the matchers would have to be refactored out of
> > > > GetNameForDecl(clang::FunctionDecl*, std::string& new_name), etc...
> > >
> > > Yeh.. that feels like a rewrite of everything.
> >
> > Arguably, we should have been doing this to begin with. But then we added
> > too
> > much matching logic into GetNameForDecl().
> >
> > >
> > > > IDK, any other ideas?
> > >
> > > I considered making a separate subclass for typetraits instead of using
> > > VarDeclMatcher's class. It's a bit of duplicated code around getting the
> > > original name if it exists though, but it could CamelCaseToUnderscoreCase
> > > directly.
> >
> > I think the right approach here is what's mentioned above; perhaps I'll
> > take a
> > stab at it later (in which case, the current CL will be easier to refactor
> > than
> > duplicating code).
> >
> 
> FWIW I do question if it'd be worth the time, unless it outlived this one
> project.

Sure; at the same time, it "shouldn't" be very hard and would help with some of
the maintenance burden. =)

> 
> 
> >
> > So... meh. LGTM =)
> >
> 
> SG
> 
> 
> >
> > https://codereview.chromium.org/2649933003/
> >
> 
> -- 
> 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 mailto:chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2017-01-24 17:41:34]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485279312902600,
"parent_rev": "57d39edccd4352d933347b9742010740cef62ce5", "commit_rev":
"5974c5c41238e402ecf1d0918a5e629e95536eb0"}

[commit-bot: I haz the power, 2017-01-24 17:42:05]

Description was changed from

==========
Capitalize WTF-based type traits as lower_case

This uses a whitelist to consistently name type traits defined
for WTF structures globally throughout chromium.

R=dcheng
BUG=598177
==========

to

==========
Capitalize WTF-based type traits as lower_case

This uses a whitelist to consistently name type traits defined
for WTF structures globally throughout chromium.

R=dcheng
BUG=598177

Review-Url: https://codereview.chromium.org/2649933003
Cr-Commit-Position: refs/heads/master@{#445755}
Committed:
https://chromium.googlesource.com/chromium/src/+/5974c5c41238e402ecf1d0918a5e...
==========

[commit-bot: I haz the power, 2017-01-24 17:42:06]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5974c5c41238e402ecf1d0918a5e...