Bug 69766 - Style guide should mandate use of pass-by-reference for out arguments
Summary: Style guide should mandate use of pass-by-reference for out arguments
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-10 10:30 PDT by Ryosuke Niwa
Modified: 2011-10-10 17:28 PDT (History)
5 users (show)

See Also:

Attachments
Patch (2.46 KB, patch)
2011-10-10 10:35 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
'get' prefix only applies to getters (2.48 KB, patch)
2011-10-10 10:42 PDT, Ryosuke Niwa 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-10 10:30:47 PDT 
Per discussion on https://lists.webkit.org/pipermail/webkit-dev/2011-October/018143.html, we should mandate use of pass-by-reference for non-optional out arguments.
Comment 1 Ryosuke Niwa 2011-10-10 10:35:07 PDT 
Created attachment 110367 [details]
Patch
Comment 2 Ryosuke Niwa 2011-10-10 10:42:23 PDT 
Created attachment 110368 [details]
'get' prefix only applies to getters
Comment 3 David Levin 2011-10-10 10:56:31 PDT 
Comment on attachment 110368 [details]
'get' prefix only applies to getters

Per comment on webkit-dev.
Comment 4 Ryosuke Niwa 2011-10-10 11:02:44 PDT 
Comment on attachment 110368 [details]
'get' prefix only applies to getters

Ask for review again as this change is nothing to do with RefPtr or PassRefPtr as pointed out on webkit-dev.
Comment 5 David Levin 2011-10-10 11:05:18 PDT 
(In reply to comment #4)
> (From update of attachment 110368 [details])
> Ask for review again as this change is nothing to do with RefPtr or PassRefPtr as pointed out on webkit-dev.

Sorry about that. I just had remember this whole thing about RefPtr<>& and then continued to read this whole thing in that context.
Comment 6 Darin Adler 2011-10-10 11:08:03 PDT 
Comment on attachment 110368 [details]
'get' prefix only applies to getters

View in context: https://bugs.webkit.org/attachment.cgi?id=110368&action=review

Looks great to me.

> Websites/webkit.org/coding/coding-style.html:578
> +<li>Precede getters that return values through out arguments with the word "get".</li>

I don’t think the phrase “precede getters” is quite right. A getter is a function, not a function’s name, and the “get” is part of the name, so doesn’t precede it.

But since the earlier item says “precede setters”, it seems we can fix both together later.

I’d also use curly quotes instead of straight quotes around "get".
Comment 7 Ryosuke Niwa 2011-10-10 11:08:21 PDT 
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 110368 [details] [details])
> > Ask for review again as this change is nothing to do with RefPtr or PassRefPtr as pointed out on webkit-dev.
> 
> Sorry about that. I just had remember this whole thing about RefPtr<>& and then continued to read this whole thing in that context.

Yeah, I figured. No worries :)
Comment 8 Ryosuke Niwa 2011-10-10 11:10:05 PDT 
(In reply to comment #6)
> (From update of attachment 110368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110368&action=review
> > Websites/webkit.org/coding/coding-style.html:578
> > +<li>Precede getters that return values through out arguments with the word "get".</li>
> 
> I don’t think the phrase “precede getters” is quite right. A getter is a function, not a function’s name, and the “get” is part of the name, so doesn’t precede it.
> 
> But since the earlier item says “precede setters”, it seems we can fix both together later.

Agreed.
Comment 9 Ryosuke Niwa 2011-10-10 11:11:24 PDT 
Committed r97070: <http://trac.webkit.org/changeset/97070>
Comment 10 Ryosuke Niwa 2011-10-10 17:28:56 PDT 
Fixed a typo in http://trac.webkit.org/changeset/97111.