Summary: | Use auto for some of our lambda function parameters | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
Severity: | Normal | CC: | andersca, beidson, cgarcia, commit-queue, koivisto, mcatanzaro, sam, zan | ||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2016-05-23 15:26:38 PDT
Created attachment 279593 [details]
Patch
Created attachment 279595 [details]
Patch
Attachment 279595 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 46 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 279597 [details]
Patch
Attachment 279597 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Hmm, GTK seems to have issues with this but not EFL, weird. Created attachment 279599 [details]
Patch
Attachment 279599 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 279601 [details]
Patch
Trying to scale down the patch until it builds on GTK. Attachment 279601 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 279604 [details]
Patch
Created attachment 279606 [details]
Patch
Attachment 279606 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 279606 [details]
Patch
GTK is building!
Comment on attachment 279606 [details]
Patch
Could you make the style bot happy so the next person who modifies these won’t see a complaint?
Many of those const auto& could just be auto& instead; I don’t think the const helps.
(In reply to comment #16) > Comment on attachment 279606 [details] > Patch > > Could you make the style bot happy so the next person who modifies these > won’t see a complaint? This is a *long* standing deficiency in the style bot, around ever since we started using lambdas and std::functions https://bugs.webkit.org/show_bug.cgi?id=124730 https://bugs.webkit.org/show_bug.cgi?id=125616 I think the reason it remains unfixed is because nobody knows how to distinguish between function definitions and lambdas in the current style bot infrastructure. (In reply to comment #17) > (In reply to comment #16) > > Comment on attachment 279606 [details] > > Patch > > > > Could you make the style bot happy so the next person who modifies these > > won’t see a complaint? > > This is a *long* standing deficiency in the style bot, around ever since we > started using lambdas and std::functions > > https://bugs.webkit.org/show_bug.cgi?id=124730 > https://bugs.webkit.org/show_bug.cgi?id=125616 > > I think the reason it remains unfixed is because nobody knows how to > distinguish between function definitions and lambdas in the current style > bot infrastructure. And, of course, I might be completely wrong about that, because I see in one of those I suggested the fix and in the other I (maybe) partially implemented the fix. Maybe it's solvable. Created attachment 279655 [details]
Patch
(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > Comment on attachment 279606 [details] > > > Patch > > > > > > Could you make the style bot happy so the next person who modifies these > > > won’t see a complaint? > > > > This is a *long* standing deficiency in the style bot, around ever since we > > started using lambdas and std::functions > > > > https://bugs.webkit.org/show_bug.cgi?id=124730 > > https://bugs.webkit.org/show_bug.cgi?id=125616 > > > > I think the reason it remains unfixed is because nobody knows how to > > distinguish between function definitions and lambdas in the current style > > bot infrastructure. > > And, of course, I might be completely wrong about that, because I see in one > of those I suggested the fix and in the other I (maybe) partially > implemented the fix. > > Maybe it's solvable. I do think this is a style script bug. putting the curly bracket on the next line does not look right in such cases. (In reply to comment #20) > (In reply to comment #18) > > (In reply to comment #17) > > > (In reply to comment #16) > > > > Comment on attachment 279606 [details] > > > > Patch > > > > > > > > Could you make the style bot happy so the next person who modifies these > > > > won’t see a complaint? > > > > > > This is a *long* standing deficiency in the style bot, around ever since we > > > started using lambdas and std::functions > > > > > > https://bugs.webkit.org/show_bug.cgi?id=124730 > > > https://bugs.webkit.org/show_bug.cgi?id=125616 > > > > > > I think the reason it remains unfixed is because nobody knows how to > > > distinguish between function definitions and lambdas in the current style > > > bot infrastructure. > > > > And, of course, I might be completely wrong about that, because I see in one > > of those I suggested the fix and in the other I (maybe) partially > > implemented the fix. > > > > Maybe it's solvable. > > I do think this is a style script bug. putting the curly bracket on the next > line does not look right in such cases. There's no debate that the style checker is getting this wrong - It is. It's just that nobody has fixed it to get it right and then update the style guidelines on the site. Attachment 279655 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3248: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:213: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm:217: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WTF/wtf/BubbleSort.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 4 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 279657 [details]
Patch
I tweaked the style checker to stop warning in this case. It is probably not perfect but it seems to work. Created attachment 279660 [details]
Patch
Comment on attachment 279660 [details] Patch Rejecting attachment 279660 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 279660, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WTF/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/1375417 Created attachment 279662 [details]
Patch
Comment on attachment 279662 [details] Patch Rejecting attachment 279662 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 279662, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Tools/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/1375435 Created attachment 279664 [details]
Patch
Comment on attachment 279664 [details] Patch Clearing flags on attachment: 279664 Committed r201333: <http://trac.webkit.org/changeset/201333> All reviewed patches have been landed. Closing bug. |