134620 2014-07-03 17:25:59 -0700 Teach check-webkit-style to suggest WTF::move() when it sees std::move() 2014-07-14 09:17:27 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 dbates dbates andersca bunhere cdumez commit-queue darin ddkilzer glenn gyuyoung.kim sam sergio thorton oldest_to_newest 1020411 0 dbates 2014-07-03 17:25:59 -0700 Following the patch for bug #134500, we should teach check-webkit-style to suggest using WTF::move() instead of std::move(). The former is less error prone to use than the latter. See bug #134500 for more details. 1020413 1 234383 dbates 2014-07-03 17:32:26 -0700 Created attachment 234383 Patch and unit test 1020419 2 234383 joepeck 2014-07-03 17:44:35 -0700 Comment on attachment 234383 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". 1022212 3 dbates 2014-07-14 07:38:09 -0700 (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. 1022220 4 dbates 2014-07-14 09:11:08 -0700 (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. 1022222 5 dbates 2014-07-14 09:17:27 -0700 Committed r171065: <http://trac.webkit.org/changeset/171065> 234383 2014-07-03 17:32:26 -0700 2014-07-03 17:44:34 -0700 Patch and unit test bug-134620-20140703173207.patch text/plain 4806 dbates U3VidmVyc2lvbiBSZXZpc2lvbjogMTcwNzc1CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZDMzNmU4OGUzYjY4NzA0MzkyYTI2NGYxYTA2ZGVhYWE3 NGUxNGQyZS4uZTBjMjBkOWE3MWZlNmUzYzEyMjVmMzFhMGY0ZDBmOTVmNGM5NjAyOCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIw IEBACiAyMDE0LTA3LTAzICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KIAorICAg ICAgICBUZWFjaCBjaGVjay13ZWJraXQtc3R5bGUgdG8gc3VnZ2VzdCBXVEY6Om1vdmUoKSB3aGVu IGl0IHNlZXMgc3RkOjptb3ZlKCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTEzNDYyMAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgICogU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHk6Cisg ICAgICAgIChjaGVja19tYXhfbWluX21hY3Jvcyk6IEZpeCB1cCBjb21tZW50IHNvIHRoYXQgaXQg cmVhZHMgd2VsbC4KKyAgICAgICAgKGNoZWNrX3d0Zl9tb3ZlKTogQWRkZWQuCisgICAgICAgIChj aGVja19zdHlsZSk6IE1vZGlmaWVkIHRvIGNhbGwgY2hlY2tfd3RmX21vdmUoKS4KKyAgICAgICAg KENwcENoZWNrZXIpOiBBZGQgY2F0ZWdvcnkgInJ1bnRpbWUvd3RmX21vdmUiLgorICAgICAgICAq IFNjcmlwdHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvY3BwX3VuaXR0ZXN0LnB5OgorICAgICAg ICAoV2ViS2l0U3R5bGVUZXN0LnRlc3Rfd3RmX21vdmUpOiBBZGRlZC4KKworMjAxNC0wNy0wMyAg RGFuaWVsIEJhdGVzICA8ZGFiYXRlc0BhcHBsZS5jb20+CisKICAgICAgICAgQWRkIFdURjo6bW92 ZSgpCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzQ1 MDAKIApkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9j cHAucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQppbmRl eCAxYjllMGRhNGViYjBhZWI2YmI2YmU2ZDZmY2E5YzIzMjQ1MDA3MDMzLi5mODg3MmYwOTI2MTE1 MWYzNmJhZGMzMmNhZTYxNTY5MTRkNTZiNmJlIDEwMDY0NAotLS0gYS9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5 L3N0eWxlL2NoZWNrZXJzL2NwcC5weQpAQCAtMjI0Myw3ICsyMjQzLDcgQEAgZGVmIGNoZWNrX3Vz aW5nX25hbWVzcGFjZShjbGVhbl9saW5lcywgbGluZV9udW1iZXIsIGZpbGVfZXh0ZW5zaW9uLCBl cnJvcik6CiAgICAgICAgICAgIkRvIG5vdCB1c2UgJ3VzaW5nIG5hbWVzcGFjZSAlczsnLiIgJSBt ZXRob2RfbmFtZSkKIAogZGVmIGNoZWNrX21heF9taW5fbWFjcm9zKGNsZWFuX2xpbmVzLCBsaW5l X251bWJlciwgZmlsZV9zdGF0ZSwgZXJyb3IpOgotICAgICIiIkxvb2tzIHVzZSBvZiBNQVgoKSBh bmQgTUlOKCkgbWFjcm9zIHRoYXQgc2hvdWxkIGJlIHJlcGxhY2VkIHdpdGggc3RkOjptYXgoKSBh bmQgc3RkOjptaW4oKS4KKyAgICAiIiJMb29rcyBmb3IgdXNlIG9mIE1BWCgpIGFuZCBNSU4oKSBt YWNyb3MgdGhhdCBzaG91bGQgYmUgcmVwbGFjZWQgd2l0aCBzdGQ6Om1heCgpIGFuZCBzdGQ6Om1p bigpLgogCiAgICAgQXJnczoKICAgICAgIGNsZWFuX2xpbmVzOiBBIENsZWFuc2VkTGluZXMgaW5z dGFuY2UgY29udGFpbmluZyB0aGUgZmlsZS4KQEAgLTIyNzAsNiArMjI3MCwzMCBAQCBkZWYgY2hl Y2tfbWF4X21pbl9tYWNyb3MoY2xlYW5fbGluZXMsIGxpbmVfbnVtYmVyLCBmaWxlX3N0YXRlLCBl cnJvcik6CiAgICAgICAgICAgJSAobWF4X21pbl9tYWNyb19sb3dlciwgbWF4X21pbl9tYWNyb19s b3dlciwgbWF4X21pbl9tYWNybykpCiAKIAorZGVmIGNoZWNrX3d0Zl9tb3ZlKGNsZWFuX2xpbmVz LCBsaW5lX251bWJlciwgZmlsZV9zdGF0ZSwgZXJyb3IpOgorICAgICIiIkxvb2tzIGZvciB1c2Ug b2YgJ3N0ZDo6bW92ZSgpJyB3aGljaCBzaG91bGQgYmUgcmVwbGFjZWQgd2l0aCAnV1RGOjptb3Zl KCknLgorCisgICAgQXJnczoKKyAgICAgIGNsZWFuX2xpbmVzOiBBIENsZWFuc2VkTGluZXMgaW5z dGFuY2UgY29udGFpbmluZyB0aGUgZmlsZS4KKyAgICAgIGxpbmVfbnVtYmVyOiBUaGUgbnVtYmVy IG9mIHRoZSBsaW5lIHRvIGNoZWNrLgorICAgICAgZmlsZV9zdGF0ZTogQSBfRmlsZVN0YXRlIGlu c3RhbmNlIHdoaWNoIG1haW50YWlucyBpbmZvcm1hdGlvbiBhYm91dAorICAgICAgICAgICAgICAg ICAgdGhlIHN0YXRlIG9mIHRoaW5ncyBpbiB0aGUgZmlsZS4KKyAgICAgIGVycm9yOiBUaGUgZnVu Y3Rpb24gdG8gY2FsbCB3aXRoIGFueSBlcnJvcnMgZm91bmQuCisgICAgIiIiCisKKyAgICAjIFRo aXMgY2hlY2sgZG9lc24ndCBhcHBseSB0byBDIG9yIE9iamVjdGl2ZS1DIGltcGxlbWVudGF0aW9u IGZpbGVzLgorICAgIGlmIGZpbGVfc3RhdGUuaXNfY19vcl9vYmplY3RpdmVfYygpOgorICAgICAg ICByZXR1cm4KKworICAgIGxpbmUgPSBjbGVhbl9saW5lcy5lbGlkZWRbbGluZV9udW1iZXJdICAj IEdldCByaWQgb2YgY29tbWVudHMgYW5kIHN0cmluZ3MuCisKKyAgICB1c2luZ19zdGRfbW92ZSA9 IHNlYXJjaChyJ1xic3RkOjptb3ZlXHMqXCgnLCBsaW5lKQorICAgIGlmIG5vdCB1c2luZ19zdGRf bW92ZToKKyAgICAgICAgcmV0dXJuCisKKyAgICBlcnJvcihsaW5lX251bWJlciwgJ3J1bnRpbWUv d3RmX21vdmUnLCA0LCAiVXNlICdXVEY6Om1vdmUoKScgaW5zdGVhZCBvZiAnc3RkOjptb3ZlKCkn LiIpCisKKwogZGVmIGNoZWNrX2N0eXBlX2Z1bmN0aW9ucyhjbGVhbl9saW5lcywgbGluZV9udW1i ZXIsIGZpbGVfc3RhdGUsIGVycm9yKToKICAgICAiIiJMb29rcyBmb3IgdXNlIG9mIHRoZSBzdGFu ZGFyZCBmdW5jdGlvbnMgaW4gY3R5cGUuaCBhbmQgc3VnZ2VzdCB0aGV5IGJlIHJlcGxhY2VkCiAg ICAgICAgYnkgdXNlIG9mIGVxdWl2aWxlbnQgb25lcyBpbiA8d3RmL0FTQ0lJQ1R5cGUuaD4/LgpA QCAtMjc1OCw2ICsyNzgyLDcgQEAgZGVmIGNoZWNrX3N0eWxlKGNsZWFuX2xpbmVzLCBsaW5lX251 bWJlciwgZmlsZV9leHRlbnNpb24sIGNsYXNzX3N0YXRlLCBmaWxlX3N0YXQKICAgICBjaGVja191 c2luZ19zdGQoY2xlYW5fbGluZXMsIGxpbmVfbnVtYmVyLCBmaWxlX3N0YXRlLCBlcnJvcikKICAg ICBjaGVja191c2luZ19uYW1lc3BhY2UoY2xlYW5fbGluZXMsIGxpbmVfbnVtYmVyLCBmaWxlX2V4 dGVuc2lvbiwgZXJyb3IpCiAgICAgY2hlY2tfbWF4X21pbl9tYWNyb3MoY2xlYW5fbGluZXMsIGxp bmVfbnVtYmVyLCBmaWxlX3N0YXRlLCBlcnJvcikKKyAgICBjaGVja193dGZfbW92ZShjbGVhbl9s aW5lcywgbGluZV9udW1iZXIsIGZpbGVfc3RhdGUsIGVycm9yKQogICAgIGNoZWNrX2N0eXBlX2Z1 bmN0aW9ucyhjbGVhbl9saW5lcywgbGluZV9udW1iZXIsIGZpbGVfc3RhdGUsIGVycm9yKQogICAg IGNoZWNrX3N3aXRjaF9pbmRlbnRhdGlvbihjbGVhbl9saW5lcywgbGluZV9udW1iZXIsIGVycm9y KQogICAgIGNoZWNrX2JyYWNlcyhjbGVhbl9saW5lcywgbGluZV9udW1iZXIsIGVycm9yKQpAQCAt MzgwMCw2ICszODI1LDcgQEAgY2xhc3MgQ3BwQ2hlY2tlcihvYmplY3QpOgogICAgICAgICAncnVu dGltZS90aHJlYWRzYWZlX2ZuJywKICAgICAgICAgJ3J1bnRpbWUvdW5zaWduZWQnLAogICAgICAg ICAncnVudGltZS92aXJ0dWFsJywKKyAgICAgICAgJ3J1bnRpbWUvd3RmX21vdmUnLAogICAgICAg ICAnd2hpdGVzcGFjZS9ibGFua19saW5lJywKICAgICAgICAgJ3doaXRlc3BhY2UvYnJhY2VzJywK ICAgICAgICAgJ3doaXRlc3BhY2UvY29sb24nLApkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkgYi9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcF91bml0dGVzdC5weQppbmRleCA1ZTE0YzRkYjQwNDBm YzRjYzczM2ZhOWFhNzBhNjU0M2NkNzE2M2FjLi4wZTM5ZDQ3ODY3NWY4NTBhMWM0Y2Y4MTdmM2Ji ZTI1ZDY5NTVhMmY5IDEwMDY0NAotLS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2No ZWNrZXJzL2NwcF91bml0dGVzdC5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxl L2NoZWNrZXJzL2NwcF91bml0dGVzdC5weQpAQCAtNDcwMSw2ICs0NzAxLDE4IEBAIGNsYXNzIFdl YktpdFN0eWxlVGVzdChDcHBTdHlsZVRlc3RCYXNlKToKICAgICAgICAgICAgICcgIFtydW50aW1l L21heF9taW5fbWFjcm9zXSBbNF0nLAogICAgICAgICAgICAgJ2Zvby5oJykKIAorICAgIGRlZiB0 ZXN0X3d0Zl9tb3ZlKHNlbGYpOgorICAgICAgICBzZWxmLmFzc2VydF9saW50KAorICAgICAgICAg ICAgICdBIGEgPSBXVEY6Om1vdmUoYik7JywKKyAgICAgICAgICAgICAnJywKKyAgICAgICAgICAg ICAnZm9vLmNwcCcpCisKKyAgICAgICAgc2VsZi5hc3NlcnRfbGludCgKKyAgICAgICAgICAgICdB IGEgPSBzdGQ6Om1vdmUoYik7JywKKyAgICAgICAgICAgICJVc2UgJ1dURjo6bW92ZSgpJyBpbnN0 ZWFkIG9mICdzdGQ6Om1vdmUoKScuIgorICAgICAgICAgICAgIiAgW3J1bnRpbWUvd3RmX21vdmVd IFs0XSIsCisgICAgICAgICAgICAnZm9vLmNwcCcpCisKICAgICBkZWYgdGVzdF9jdHlwZV9mdWNu dGlvbihzZWxmKToKICAgICAgICAgc2VsZi5hc3NlcnRfbGludCgKICAgICAgICAgICAgICdpbnQg aSA9IGlzYXNjaWkoOCk7JywK