Summary: | Teach check-webkit-style to suggest WTF::move() when it sees std::move() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, bunhere, cdumez, commit-queue, darin, ddkilzer, glenn, gyuyoung.kim, sam, sergio, thorton | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Daniel Bates
2014-07-03 17:25:59 PDT
Created attachment 234383 [details]
Patch and unit test
Comment on attachment 234383 [details] Patch and unit test View in context: https://bugs.webkit.org/attachment.cgi?id=234383&action=review Nice! r=me > Tools/Scripts/webkitpy/style/checkers/cpp.py:2286 > + # This check doesn't apply to C or Objective-C implementation files. > + if file_state.is_c_or_objective_c(): > + return We want this for ObjC++. This "is_c_or_objective_c" to too generic for that. A quick search found: network/ios/QuickLook.mm 431: return WTF::move(quickLookHandle); 445: return WTF::move(quickLookHandle); 463: return WTF::move(quickLookHandle); Maybe we would want an "is_cpp". (In reply to comment #2) > (From update of attachment 234383 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234383&action=review > > Nice! r=me > > > Tools/Scripts/webkitpy/style/checkers/cpp.py:2286 > > + # This check doesn't apply to C or Objective-C implementation files. > > + if file_state.is_c_or_objective_c(): > > + return > > We want this for ObjC++. This "is_c_or_objective_c" to too generic for that. A quick search found: > > network/ios/QuickLook.mm > 431: return WTF::move(quickLookHandle); > 445: return WTF::move(quickLookHandle); > 463: return WTF::move(quickLookHandle); > > Maybe we would want an "is_cpp". It seems sufficient to be able to identify a file as Objective-C/Objective-C++ from a C file instead of adding an is_cpp method (see remark (*)). Then we can add a new method, say _FileState.is_objective_c_or_objective_cpp, that returns true for files that end in ".m" or ".mm" or header files that contain Objective-C directives and repurpose _FileState.is_c_or_objective_c() to only return true for files that end in ".c" or ".m" or header files that contain 'extern "C"' or Objective-C directives. (*) From my understanding the C++ style checker module of check-style assumes that its input is a file that contains C++ code and the purpose of the _FileState methods are to detect when the input file may also contain C or Objective-C code with the exception that files that end in ".c" or ".m" are assumed to contain C and Objective-C code, respectively. (In reply to comment #3) > (In reply to comment #2) > > [...] > > We want this for ObjC++. This "is_c_or_objective_c" to too generic for that. > > [...] > > Maybe we would want an "is_cpp". > > It seems sufficient to be able to identify a file as Objective-C/Objective-C++ from a C file instead of adding an is_cpp method (see remark (*)). Then we can add a new method, say _FileState.is_objective_c_or_objective_cpp, that returns true for files that end in ".m" or ".mm" or header files that contain Objective-C directives and repurpose _FileState.is_c_or_objective_c() to only return true for files that end in ".c" or ".m" or header files that contain 'extern "C"' or Objective-C directives. > > [...] I filed bug #134884 to teach check-webkit-style to apply C++ rules to Objective-C++ files if applicable. Committed r171065: <http://trac.webkit.org/changeset/171065> |