Bug 158001 - Use auto for some of our lambda function parameters
Summary: Use auto for some of our lambda function parameters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-23 15:26 PDT by Chris Dumez
Modified: 2016-05-24 09:49 PDT (History)
8 users (show)

See Also:


Attachments
Patch (54.92 KB, patch)
2016-05-23 15:28 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (55.14 KB, patch)
2016-05-23 15:30 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (54.19 KB, patch)
2016-05-23 15:35 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (53.45 KB, patch)
2016-05-23 15:46 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (52.90 KB, patch)
2016-05-23 16:05 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (52.29 KB, patch)
2016-05-23 16:22 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (52.43 KB, patch)
2016-05-23 16:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (52.21 KB, patch)
2016-05-24 09:07 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (53.66 KB, patch)
2016-05-24 09:24 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (53.67 KB, patch)
2016-05-24 09:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (53.67 KB, patch)
2016-05-24 09:43 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (53.66 KB, patch)
2016-05-24 09:48 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-05-23 15:26:38 PDT
Use auto for some of our lambda function parameters now that we build with c++14.
Comment 1 Chris Dumez 2016-05-23 15:28:23 PDT
Created attachment 279593 [details]
Patch
Comment 2 Chris Dumez 2016-05-23 15:30:55 PDT
Created attachment 279595 [details]
Patch
Comment 3 WebKit Commit Bot 2016-05-23 15:33:40 PDT
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.
Comment 4 Chris Dumez 2016-05-23 15:35:18 PDT
Created attachment 279597 [details]
Patch
Comment 5 WebKit Commit Bot 2016-05-23 15:36:27 PDT
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.
Comment 6 Chris Dumez 2016-05-23 15:42:03 PDT
Hmm, GTK seems to have issues with this but not EFL, weird.
Comment 7 Chris Dumez 2016-05-23 15:46:42 PDT
Created attachment 279599 [details]
Patch
Comment 8 WebKit Commit Bot 2016-05-23 15:47:58 PDT
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.
Comment 9 Chris Dumez 2016-05-23 16:05:22 PDT
Created attachment 279601 [details]
Patch
Comment 10 Chris Dumez 2016-05-23 16:05:59 PDT
Trying to scale down the patch until it builds on GTK.
Comment 11 WebKit Commit Bot 2016-05-23 16:07:45 PDT
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.
Comment 12 Chris Dumez 2016-05-23 16:22:05 PDT
Created attachment 279604 [details]
Patch
Comment 13 Chris Dumez 2016-05-23 16:32:49 PDT
Created attachment 279606 [details]
Patch
Comment 14 WebKit Commit Bot 2016-05-23 16:35:19 PDT
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 15 Chris Dumez 2016-05-23 16:46:15 PDT
Comment on attachment 279606 [details]
Patch

GTK is building!
Comment 16 Darin Adler 2016-05-24 08:56:04 PDT
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.
Comment 17 Brady Eidson 2016-05-24 08:59:58 PDT
(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.
Comment 18 Brady Eidson 2016-05-24 09:02:02 PDT
(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.
Comment 19 Chris Dumez 2016-05-24 09:07:55 PDT
Created attachment 279655 [details]
Patch
Comment 20 Chris Dumez 2016-05-24 09:09:14 PDT
(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.
Comment 21 Brady Eidson 2016-05-24 09:10:27 PDT
(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.
Comment 22 WebKit Commit Bot 2016-05-24 09:10:40 PDT
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.
Comment 23 Chris Dumez 2016-05-24 09:24:40 PDT
Created attachment 279657 [details]
Patch
Comment 24 Chris Dumez 2016-05-24 09:25:20 PDT
I tweaked the style checker to stop warning in this case. It is probably not perfect but it seems to work.
Comment 25 Chris Dumez 2016-05-24 09:32:43 PDT
Created attachment 279660 [details]
Patch
Comment 26 WebKit Commit Bot 2016-05-24 09:40:49 PDT
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
Comment 27 Chris Dumez 2016-05-24 09:43:39 PDT
Created attachment 279662 [details]
Patch
Comment 28 WebKit Commit Bot 2016-05-24 09:45:18 PDT
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
Comment 29 Chris Dumez 2016-05-24 09:48:01 PDT
Created attachment 279664 [details]
Patch
Comment 30 Chris Dumez 2016-05-24 09:49:20 PDT
Comment on attachment 279664 [details]
Patch

Clearing flags on attachment: 279664

Committed r201333: <http://trac.webkit.org/changeset/201333>
Comment 31 Chris Dumez 2016-05-24 09:49:25 PDT
All reviewed patches have been landed.  Closing bug.