New issue
Advanced search Search tips

Issue 598177 link

Starred by 2 users
Status: Fixed
Owner:
danakj@chromium.org
Closed: Jan 2017
Cc:
danakj@chromium.org
lukasza@chromium.org
dcheng@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style: Static const bool used for template traits not renamed.

Project Member Reported by dcheng@chromium.org, Mar 26 2016 
After a rename in WTF:

$ git gs safeToCompareToEmptyOrDeleted
is 42.

But:
$ git gs kSafeToCompareToEmptyOrDeleted| wc -l                      
is 5.

Almost all the rewritten references are inside HashTable.h itself: the various hash function helpers exposing this trait largely aren't rewritten, with the exception of StringHash.h.

Comment 1 by dcheng@chromium.org, Mar 26 2016 

There are two problems:
1) Stuff in namespace WTF doesn't always get rewritten. Maybe this is related to the CXXDependentScopeMember stuff.
2) Stuff outside namespace WTF (obviously) won't get renamed. But we can probably manually fix those up with sed on the day of the WTF rewrite.

Comment 2 by lukasza@chromium.org, Sep 2 2016

Owner: lukasza@chromium.org
Ooops.  I assume that it is undesirable to rename |safeToCompareToEmptyOrDeleted| (because this is a field of a type trait).  I guess I've regressed this in https://codereview.chromium.org/2307643002 and https://codereview.chromium.org/2276813003 :-(

Comment 3 by danakj@chromium.org, Sep 2 2016 

safeToCompareToEmptyOrDeleted should be safe_to_compare_to_empty_or_deleted if its a type trait.

Comment 4 by danakj@chromium.org, Jan 18 2017 

I think this is better now, or at least more consistent thanks to other stuff. It appears to be always kSafeToCompareToEmptyOrDeleted now. It's not determining it is a trait tho.

The only safeToCompareToEmptyOrDeleted I see are:
mojo/public/cpp/bindings/lib/wtf_hash_util.h:68:  static const bool safeToCompareToEmptyOrDeleted = false;
mojo/public/cpp/bindings/lib/wtf_hash_util.h:80:  static const bool safeToCompareToEmptyOrDeleted = false;


Which I guess should be renamed as it's a traits for WTF stuff but it is not in the WTF namespace so uh hm.

Comment 5 by danakj@chromium.org, Jan 18 2017 

The whole structs mojo::internal::StructPtrHashFn and mojo::internal::InlinedStructPtrHashFn need to be rewritten. Maybe we need to whitelist these.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5974c5c41238e402ecf1d0918a5e629e95536eb0

commit 5974c5c41238e402ecf1d0918a5e629e95536eb0
Author: danakj <danakj@chromium.org>
Date: Tue Jan 24 17:41:30 2017

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}

[modify] https://crrev.com/5974c5c41238e402ecf1d0918a5e629e95536eb0/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
[modify] https://crrev.com/5974c5c41238e402ecf1d0918a5e629e95536eb0/tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc
[modify] https://crrev.com/5974c5c41238e402ecf1d0918a5e629e95536eb0/tools/clang/rewrite_to_chrome_style/tests/fields-original.cc

Comment 7 by danakj@chromium.org, Jan 24 2017

Cc: lukasza@chromium.org
Owner: danakj@chromium.org
Status: Fixed (was: Available)
►

Sign in to add a comment