69766 2011-10-10 10:30:47 -0700 Style guide should mandate use of pass-by-reference for out arguments 2011-10-10 17:28:56 -0700 1 1 1 Unclassified WebKit WebKit Website 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 rniwa rniwa darin jamesr levin morrita pkasting oldest_to_newest 480994 0 rniwa 2011-10-10 10:30:47 -0700 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. 480997 1 110367 rniwa 2011-10-10 10:35:07 -0700 Created attachment 110367 Patch 480999 2 110368 rniwa 2011-10-10 10:42:23 -0700 Created attachment 110368 'get' prefix only applies to getters 481000 3 110368 levin 2011-10-10 10:56:31 -0700 Comment on attachment 110368 'get' prefix only applies to getters Per comment on webkit-dev. 481004 4 110368 rniwa 2011-10-10 11:02:44 -0700 Comment on attachment 110368 '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. 481005 5 levin 2011-10-10 11:05:18 -0700 (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. 481006 6 110368 darin 2011-10-10 11:08:03 -0700 Comment on attachment 110368 '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". 481007 7 rniwa 2011-10-10 11:08:21 -0700 (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 :) 481008 8 rniwa 2011-10-10 11:10:05 -0700 (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. 481012 9 rniwa 2011-10-10 11:11:24 -0700 Committed r97070: <http://trac.webkit.org/changeset/97070> 481340 10 rniwa 2011-10-10 17:28:56 -0700 Fixed a typo in http://trac.webkit.org/changeset/97111. 110367 2011-10-10 10:35:07 -0700 2011-10-10 10:42:01 -0700 Patch bug-69766-20111010103506.patch text/plain 2517 rniwa SW5kZXg6IFdlYnNpdGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYnNp dGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCShyZXZpc2lvbiA5NzA2OCkKKysrIFdlYnNpdGVzL3dl YmtpdC5vcmcvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEt MTAtMTAgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CisKKyAgICAgICAgU3R5bGUg Z3VpZGUgc2hvdWxkIG1hbmRhdGUgdXNlIG9mIHBhc3MtYnktcmVmZXJlbmNlIGZvciBvdXQgYXJn dW1lbnRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02 OTc2NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRo aXMgbWF0Y2hlcyB0aGUgY29udmVudGlvbiB1c2VkIHRocm91Z2hvdXQgV2ViQ29yZS4KKworICAg ICAgICAqIGNvZGluZy9jb2Rpbmctc3R5bGUuaHRtbDoKKwogMjAxMS0xMC0wNiAgQW1ydXRoIFJh aiAgPGFtcnV0aHJhakBtb3Rvcm9sYS5jb20+CiAKICAgICAgICAgQWRkIE1vdG9yb2xhIE1vYmls aXR5IHRvIHRlYW0uaHRtbApJbmRleDogV2Vic2l0ZXMvd2Via2l0Lm9yZy9jb2RpbmcvY29kaW5n LXN0eWxlLmh0bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2Vic2l0ZXMvd2Via2l0Lm9yZy9jb2RpbmcvY29k aW5nLXN0eWxlLmh0bWwJKHJldmlzaW9uIDk3MDA0KQorKysgV2Vic2l0ZXMvd2Via2l0Lm9yZy9j b2RpbmcvY29kaW5nLXN0eWxlLmh0bWwJKHdvcmtpbmcgY29weSkKQEAgLTU3NSw2ICs1NzUsMTcg QEAgc2l6ZV90IGdldENvdW50KCk7CiA8L3ByZT4KIDwvbGk+CiAKKzxsaT5QcmVjZWRlIGZ1bmN0 aW9ucyB3aXRoIGFuIG91dCBhcmd1bWVudCB3aXRoIHRoZSB3b3JkICJnZXQiLjwvbGk+Cis8aDQg Y2xhc3M9InJpZ2h0Ij5SaWdodDo8L2g0PgorPHByZSBjbGFzcz0iY29kZSI+Cit2b2lkIGdldElu bGluZUJveEFuZE9mZnNldChJbmxpbmVCb3gqJmFtcDssIGludCZhbXA7IGNhcmV0T2Zmc2V0KSBj b25zdDsKKzwvcHJlPgorCis8aDQgY2xhc3M9Indyb25nIj5Xcm9uZzo8L2g0PgorPHByZSBjbGFz cz0iY29kZSI+Cit2b2lkIGlubGluZUJveEFuZE9mZnNldChJbmxpbmVCb3gqJmFtcDssIGludCZh bXA7IGNhcmV0T2Zmc2V0KSBjb25zdDsKKzwvcHJlPgorCiA8bGk+VXNlIGRlc2NyaXB0aXZlIHZl cmJzIGluIGZ1bmN0aW9uIG5hbWVzLgogPGg0IGNsYXNzPSJyaWdodCI+UmlnaHQ6PC9oND4KIDxw cmUgY2xhc3M9ImNvZGUiPgpAQCAtNjk3LDYgKzcwOCwxMSBAQCBNeUNsYXNzOjpNeUNsYXNzKERv Y3VtZW50KiBkb2MpIDogTXlTdXBlCiAKIE15T3RoZXJDbGFzczo6TXlPdGhlckNsYXNzKCkgOiBN eVN1cGVyQ2xhc3MoKSB7fQogPC9wcmU+Cis8L29sPgorCis8aDM+UG9pbnRlcnMgYW5kIFJlZmVy ZW5jZXM8L2gzPgorCis8b2w+CiAKIDxsaT5Qb2ludGVyIHR5cGVzIGluIG5vbi1DKysgY29kZSAm bWRhc2g7IFBvaW50ZXIgdHlwZXMgc2hvdWxkIGJlIHdyaXR0ZW4gd2l0aCBhIHNwYWNlIGJldHdl ZW4gdGhlCiB0eXBlIGFuZCB0aGUgKiAoc28gdGhlICogaXMgYWRqYWNlbnQgdG8gdGhlIGZvbGxv d2luZyBpZGVudGlmaWVyIGlmIGFueSkuCkBAIC03MjAsNiArNzM2LDMyIEBAIEltYWdlICpTVkdT dHlsZWRFbGVtZW50Ojpkb1NvbWV0aGluZyhQYWkKICAgICBjb25zdCBLQ0Rhc2hBcnJheSAmYW1w O2Rhc2hlcyA9IGRhc2hBcnJheSgpOwogPC9wcmU+CiAKKzxsaT5BbiBvdXQgYXJndW1lbnQgb2Yg YSBmdW5jdGlvbiBzaG91bGQgYmUgcGFzc2VkIGJ5IHJlZmVyZW5jZSBleGNlcHQgcmFyZSBjYXNl cyB3aGVyZQoraXQgaXMgb3B0aW9uYWwgaW4gd2hpY2ggY2FzZSBpdCBzaG91bGQgYmUgcGFzc2Vk IGJ5IHBvaW50ZXIuCisKKzxoNCBjbGFzcz0icmlnaHQiPlJpZ2h0OjwvaDQ+Cis8cHJlIGNsYXNz PSJjb2RlIj4KK3ZvaWQgTXlDbGFzczo6Z2V0U29tZVZhbHVlKE91dEFyZ3VtZW50VHlwZSZhbXA7 IG91dEFyZ3VtZW50KSBjb25zdAoreworICAgIG91dEFyZ3VtZW50ID0gbV92YWx1ZTsKK30KKwor dm9pZCBNeUNsYXNzOjpkb1NvbWV0aGluZyhPdXRBcmd1bWVudFR5cGUqIG91dEFyZ3VtZW50KSBj b25zdAoreworICAgIGRvU29tZXRoaW5nKCk7CisgICAgaWYgKG91dEFyZ3VtZW50KQorICAgICAg ICBvdXRBcmd1bWVudCA9IG1fdmFsdWU7Cit9Cis8L3ByZT4KKworPGg0IGNsYXNzPSJ3cm9uZyI+ V3Jvbmc6PC9oND4KKzxwcmUgY2xhc3M9ImNvZGUiPgordm9pZCBNeUNsYXNzOjpnZXRTb21lVmFs dWUoT3V0QXJndW1lbnRUeXBlKiBvdXRBcmd1bWVudCkgY29uc3QKK3sKKyAgICAqb3V0QXJndW1l bnQgPSBtX3ZhbHVlOworfQorPC9wcmU+CisKIDwvb2w+CiAKIDxoMz4jaW5jbHVkZSBTdGF0ZW1l bnRzPC9oMz4K 110368 2011-10-10 10:42:23 -0700 2011-10-10 11:08:03 -0700 'get' prefix only applies to getters bug-69766-20111010104222.patch text/plain 2535 rniwa SW5kZXg6IFdlYnNpdGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYnNp dGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCShyZXZpc2lvbiA5NzA2OCkKKysrIFdlYnNpdGVzL3dl YmtpdC5vcmcvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEt MTAtMTAgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CisKKyAgICAgICAgU3R5bGUg Z3VpZGUgc2hvdWxkIG1hbmRhdGUgdXNlIG9mIHBhc3MtYnktcmVmZXJlbmNlIGZvciBvdXQgYXJn dW1lbnRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02 OTc2NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRo aXMgbWF0Y2hlcyB0aGUgY29udmVudGlvbiB1c2VkIHRocm91Z2hvdXQgV2ViQ29yZS4KKworICAg ICAgICAqIGNvZGluZy9jb2Rpbmctc3R5bGUuaHRtbDoKKwogMjAxMS0xMC0wNiAgQW1ydXRoIFJh aiAgPGFtcnV0aHJhakBtb3Rvcm9sYS5jb20+CiAKICAgICAgICAgQWRkIE1vdG9yb2xhIE1vYmls aXR5IHRvIHRlYW0uaHRtbApJbmRleDogV2Vic2l0ZXMvd2Via2l0Lm9yZy9jb2RpbmcvY29kaW5n LXN0eWxlLmh0bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2Vic2l0ZXMvd2Via2l0Lm9yZy9jb2RpbmcvY29k aW5nLXN0eWxlLmh0bWwJKHJldmlzaW9uIDk3MDA0KQorKysgV2Vic2l0ZXMvd2Via2l0Lm9yZy9j b2RpbmcvY29kaW5nLXN0eWxlLmh0bWwJKHdvcmtpbmcgY29weSkKQEAgLTU3NSw2ICs1NzUsMTcg QEAgc2l6ZV90IGdldENvdW50KCk7CiA8L3ByZT4KIDwvbGk+CiAKKzxsaT5QcmVjZWRlIGdldHRl cnMgdGhhdCByZXR1cm4gdmFsdWVzIHRocm91Z2ggb3V0IGFyZ3VtZW50cyB3aXRoIHRoZSB3b3Jk ICJnZXQiLjwvbGk+Cis8aDQgY2xhc3M9InJpZ2h0Ij5SaWdodDo8L2g0PgorPHByZSBjbGFzcz0i Y29kZSI+Cit2b2lkIGdldElubGluZUJveEFuZE9mZnNldChJbmxpbmVCb3gqJmFtcDssIGludCZh bXA7IGNhcmV0T2Zmc2V0KSBjb25zdDsKKzwvcHJlPgorCis8aDQgY2xhc3M9Indyb25nIj5Xcm9u Zzo8L2g0PgorPHByZSBjbGFzcz0iY29kZSI+Cit2b2lkIGlubGluZUJveEFuZE9mZnNldChJbmxp bmVCb3gqJmFtcDssIGludCZhbXA7IGNhcmV0T2Zmc2V0KSBjb25zdDsKKzwvcHJlPgorCiA8bGk+ VXNlIGRlc2NyaXB0aXZlIHZlcmJzIGluIGZ1bmN0aW9uIG5hbWVzLgogPGg0IGNsYXNzPSJyaWdo dCI+UmlnaHQ6PC9oND4KIDxwcmUgY2xhc3M9ImNvZGUiPgpAQCAtNjk3LDYgKzcwOCwxMSBAQCBN eUNsYXNzOjpNeUNsYXNzKERvY3VtZW50KiBkb2MpIDogTXlTdXBlCiAKIE15T3RoZXJDbGFzczo6 TXlPdGhlckNsYXNzKCkgOiBNeVN1cGVyQ2xhc3MoKSB7fQogPC9wcmU+Cis8L29sPgorCis8aDM+ UG9pbnRlcnMgYW5kIFJlZmVyZW5jZXM8L2gzPgorCis8b2w+CiAKIDxsaT5Qb2ludGVyIHR5cGVz IGluIG5vbi1DKysgY29kZSAmbWRhc2g7IFBvaW50ZXIgdHlwZXMgc2hvdWxkIGJlIHdyaXR0ZW4g d2l0aCBhIHNwYWNlIGJldHdlZW4gdGhlCiB0eXBlIGFuZCB0aGUgKiAoc28gdGhlICogaXMgYWRq YWNlbnQgdG8gdGhlIGZvbGxvd2luZyBpZGVudGlmaWVyIGlmIGFueSkuCkBAIC03MjAsNiArNzM2 LDMyIEBAIEltYWdlICpTVkdTdHlsZWRFbGVtZW50Ojpkb1NvbWV0aGluZyhQYWkKICAgICBjb25z dCBLQ0Rhc2hBcnJheSAmYW1wO2Rhc2hlcyA9IGRhc2hBcnJheSgpOwogPC9wcmU+CiAKKzxsaT5B biBvdXQgYXJndW1lbnQgb2YgYSBmdW5jdGlvbiBzaG91bGQgYmUgcGFzc2VkIGJ5IHJlZmVyZW5j ZSBleGNlcHQgcmFyZSBjYXNlcyB3aGVyZQoraXQgaXMgb3B0aW9uYWwgaW4gd2hpY2ggY2FzZSBp dCBzaG91bGQgYmUgcGFzc2VkIGJ5IHBvaW50ZXIuCisKKzxoNCBjbGFzcz0icmlnaHQiPlJpZ2h0 OjwvaDQ+Cis8cHJlIGNsYXNzPSJjb2RlIj4KK3ZvaWQgTXlDbGFzczo6Z2V0U29tZVZhbHVlKE91 dEFyZ3VtZW50VHlwZSZhbXA7IG91dEFyZ3VtZW50KSBjb25zdAoreworICAgIG91dEFyZ3VtZW50 ID0gbV92YWx1ZTsKK30KKwordm9pZCBNeUNsYXNzOjpkb1NvbWV0aGluZyhPdXRBcmd1bWVudFR5 cGUqIG91dEFyZ3VtZW50KSBjb25zdAoreworICAgIGRvU29tZXRoaW5nKCk7CisgICAgaWYgKG91 dEFyZ3VtZW50KQorICAgICAgICBvdXRBcmd1bWVudCA9IG1fdmFsdWU7Cit9Cis8L3ByZT4KKwor PGg0IGNsYXNzPSJ3cm9uZyI+V3Jvbmc6PC9oND4KKzxwcmUgY2xhc3M9ImNvZGUiPgordm9pZCBN eUNsYXNzOjpnZXRTb21lVmFsdWUoT3V0QXJndW1lbnRUeXBlKiBvdXRBcmd1bWVudCkgY29uc3QK K3sKKyAgICAqb3V0QXJndW1lbnQgPSBtX3ZhbHVlOworfQorPC9wcmU+CisKIDwvb2w+CiAKIDxo Mz4jaW5jbHVkZSBTdGF0ZW1lbnRzPC9oMz4K