194848 2019-02-19 20:45:58 -0800 [Code Style] Functions returning void 2019-02-20 09:44:27 -0800 1 1 1 Unclassified WebKit WebKit Website WebKit Local Build Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 1 dbates dbates darin jond rniwa thorton oldest_to_newest 1507991 0 dbates 2019-02-19 20:45:58 -0800 WebKit Dev thread: <https://lists.webkit.org/pipermail/webkit-dev/2019-February/030439.html> It has been a long standing, unwritten policy, that return statements should be written expression-less and on their own line in functions that return void. And a return statement on its own line in a function returning void should be omitted when its effect is equivalent to falling off the end of the function. From the WebKit dev thread (linked above), I strongly support this policy and so does Tim Horton [1]. Ryosuke Niwa seems supportive, acknowledging that WebKit "tend[s] to have a separate return." and replying to Tim Horton's "+1" with support for codifying this unwritten policy [2]. Although he did not reply to the thread, Darin Adler has acknowledged "our style has been to not return void" in his remarks in bug 194485, comment 5. Alex Christensen seems to be the lone respondent that is strongly opposed to the unwritten policy [3]. All other respondents to the thread to date have expressed no strong opinion. [1] <https://lists.webkit.org/pipermail/webkit-dev/2019-February/030440.html> [2] <https://lists.webkit.org/pipermail/webkit-dev/2019-February/030441.html> [3] <https://lists.webkit.org/pipermail/webkit-dev/2019-February/030444.html> 1507994 1 362479 dbates 2019-02-19 20:50:13 -0800 Created attachment 362479 Patch 1507995 2 dbates 2019-02-19 20:51:36 -0800 When I have a moment I will look to teach the style checker to enforce this rule. I do not feel we need to wait for style checker support to move forward and amend the code style guidelines in light of the support for this policy. 1508003 3 362479 rniwa 2019-02-19 21:43:44 -0800 Comment on attachment 362479 Patch As far as I can tell, there isn't a strong consensus either way. As such, I don't think we should make this coding style guide change right now. r-. 1508007 4 dbates 2019-02-19 22:23:33 -0800 (In reply to Ryosuke Niwa from comment #3) > Comment on attachment 362479 [details] > Patch > > As far as I can tell, there isn't a strong consensus either way. Please read the bug description. > As such, I don't think we should make this coding style guide change right You're remarks are confusing. You wrote, emphasis mine: [[[ I much prefer doing this in my own code **but stay away from it in WebKit because we tend to have a separate return.** ]] AND [[ On Thu, Feb 7, 2019 at 12:53 PM Tim Horton <timothy_horton at apple.com> wrote: > +1 to a style guide update > Yeah, we **might as well as codify it in the style guideline for clarity.** ]] Yeah, we might as well as codify it in the style guideline for clarity. > now. > r-. I'm so confused what you're position is. What is is? 1508010 5 dbates 2019-02-19 22:45:02 -0800 Ryosuke, your argument does not follow from the discussion as far as I can tell. I'm inclined to put this up for review again because 1) your argument does not follow for the discussion 2) your comments seem to contract your own words in <https://lists.webkit.org/pipermail/webkit-dev/2019-February/030441.html> and 3) you have not provided any constructive criticism of the content of the patch or provided guidance on a path forward. Instead you put up a wall and tell me "I don't think we should make this coding style guide change *right now*." 1508099 6 rniwa 2019-02-20 09:44:20 -0800 (In reply to Daniel Bates from comment #5) > Ryosuke, your argument does not follow from the discussion as far as I can tell. It totally does. Just look at the latest reply on webkit-dev. I think you misinterpreted people's responses. 362479 2019-02-19 20:50:13 -0800 2019-02-19 21:43:44 -0800 Patch bug-194848-20190219205012.patch text/plain 2669 dbates U3VidmVyc2lvbiBSZXZpc2lvbjogMjQxNTU5CmRpZmYgLS1naXQgYS9XZWJzaXRlcy93ZWJraXQu b3JnL0NoYW5nZUxvZyBiL1dlYnNpdGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCmluZGV4IDQ3ZTNk MjIwZDA1NDc3MmQwOWU1NmM2Nzg2MTI5MDFhNDNiODUyZmMuLmFlZGNkYTM5OTlmYzE2ZTAxMTg0 NDE5OGNiNzMxMWQxMzUzN2ViOTMgMTAwNjQ0Ci0tLSBhL1dlYnNpdGVzL3dlYmtpdC5vcmcvQ2hh bmdlTG9nCisrKyBiL1dlYnNpdGVzL3dlYmtpdC5vcmcvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkg QEAKKzIwMTktMDItMTkgIERhbmllbCBCYXRlcyAgPGRhYmF0ZXNAYXBwbGUuY29tPgorCisgICAg ICAgIFtDb2RlIFN0eWxlXSBGdW5jdGlvbnMgcmV0dXJuaW5nIHZvaWQKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5NDg0OAorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvZGlmeSBwb2xpY3kgb24gaG93IHRv IHdyaXRlIHJldHVybiBzdGF0ZW1lbnRzIGluIGZ1bmN0aW9ucyByZXR1cm5pbmcgdm9pZC4gSW4g c3VjaCBmdW5jdGlvbnMsCisgICAgICAgIHRoZSByZXR1cm4gc3RhdGVtZW50IHNob3VsZCBiZSBl eHByZXNzaW9uLWxlc3MgYW5kIHdyaXR0ZW4gb24gaXRzIG93biBsaW5lLiBJZiBhZnRlciB3cml0 aW5nIHRoZQorICAgICAgICByZXR1cm4gc3RhdGVtZW50IG9uIGl0cyBvd24gbGluZSBpdHMgZWZm ZWN0IGlzIGVxdWl2YWxlbnQgdG8gZmFsbGluZyBvZmYgdGhlIGVuZCBvZiB0aGUgZnVuY3Rpb24K KyAgICAgICAgdGhlbiBpdCBzaG91bGQgYmUgb21pdHRlZC4KKworICAgICAgICBUaGlzIHBvbGlj eSB3YXMgZGlzY3Vzc2VkIG9uIHdlYmtpdC1kZXYgaW4gdGhlIHRocmVhZCB0aGF0IHN0YXJ0cyBh dCA8aHR0cHM6Ly9saXN0cy53ZWJraXQub3JnL3BpcGVybWFpbC93ZWJraXQtZGV2LzIwMTktRmVi cnVhcnkvMDMwNDM5Lmh0bWw+LgorCisgICAgICAgICogY29kZS1zdHlsZS5tZDoKKwogMjAxOS0w Mi0wNyAgSm9uIERhdmlzICA8am9uZEBhcHBsZS5jb20+CiAKICAgICAgICAgRml4ZWQgYW5pbWF0 aW9uIHBvc2l0aW9uaW5nIHdoZW4gaG9tZXBhZ2UgYW5pbWF0aW9uIGNvbXBsZXRlcwpkaWZmIC0t Z2l0IGEvV2Vic2l0ZXMvd2Via2l0Lm9yZy9jb2RlLXN0eWxlLm1kIGIvV2Vic2l0ZXMvd2Via2l0 Lm9yZy9jb2RlLXN0eWxlLm1kCmluZGV4IDBiNzFlOGQ2NjY2ZDRjZWEwOGFjOTUxOTgzYjZjMDFh ODU0M2U1YjAuLjM3ZWYwODhjNDAzOWExNzZmZWNiZWRlMGY2ZGE3NjFjN2M1NDU5ZTggMTAwNjQ0 Ci0tLSBhL1dlYnNpdGVzL3dlYmtpdC5vcmcvY29kZS1zdHlsZS5tZAorKysgYi9XZWJzaXRlcy93 ZWJraXQub3JnL2NvZGUtc3R5bGUubWQKQEAgLTM2Miw2ICszNjIsNjIgQEAgaWYgKGNvbmRpdGlv bikgewogfQogYGBgCiAKK1tdKCNsaW5lYnJlYWtpbmctcmV0dXJuLXZvaWQpIEluIGZ1bmN0aW9u cyB0aGF0IHJldHVybiB2b2lkLCB0aGUgYHJldHVybmAgc3RhdGVtZW50IHNob3VsZCBub3QgaGF2 ZSBhbiBleHByZXNzaW9uIGFuZCBzaG91bGQgYmUgd3JpdHRlbiBvbiBpdHMgb3duIGxpbmUuIEEg YHJldHVybmAgc3RhdGVtZW50IG9uIGl0cyBvd24gbGluZSBzaG91bGQgYmUgb21pdHRlZCBlbnRp cmVseSBpZiBpdCBoYXMgdGhlIHNhbWUgZWZmZWN0IGFzIGZhbGxpbmcgb2ZmIHRoZSBlbmQgb2Yg dGhlIGZ1bmN0aW9uLgorCisjIyMjIyMgUmlnaHQ6CisKK2BgYGNwcAordm9pZCBQb2xpY3lDaGVj a2VyOjpjaGVja05ld1dpbmRvd1BvbGljeSgpCit7CisgICAgaWYgKGNvbmRpdGlvbikgeworICAg ICAgICBmKFNob3VsZENvbnRpbnVlOjpObyk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisgICAg ZihTaG91bGRDb250aW51ZTo6WWVzKTsKK30KK2BgYAorCisjIyMjIyMgV3Jvbmc6CisKK2BgYGNw cAordm9pZCBQb2xpY3lDaGVja2VyOjpjaGVja05ld1dpbmRvd1BvbGljeSgpCit7CisgICAgaWYg KGNvbmRpdGlvbikKKyAgICAgICAgcmV0dXJuIGYoU2hvdWxkQ29udGludWU6Ok5vKTsKKyAgICBy ZXR1cm4gZihTaG91bGRDb250aW51ZTo6WWVzKTsKK30KK2BgYAorCisjIyMjIyMgUmlnaHQ6CisK K2BgYGNwcAordm9pZCBQb2xpY3lDaGVja2VyOjpjaGVja05hdmlnYXRpb25Qb2xpY3koKQorewor ICAgIHN3aXRjaCAoY29uZGl0aW9uKSB7CisgICAgY2FzZSBmb29Db25kaXRpb246CisgICAgY2Fz ZSBiYXJDb25kaXRpb246CisgICAgICAgIGYoU2hvdWxkQ29udGludWU6OlllcykKKyAgICAgICAg cmV0dXJuOworICAgIH0KKyAgICBmKFNob3VsZENvbnRpbnVlOjpObyk7Cit9CitgYGAKKworIyMj IyMjIFdyb25nOgorCitgYGBjcHAKK3ZvaWQgUG9saWN5Q2hlY2tlcjo6Y2hlY2tOYXZpZ2F0aW9u UG9saWN5KCkKK3sKKyAgICBzd2l0Y2ggKGNvbmRpdGlvbikgeworICAgIGNhc2UgZm9vQ29uZGl0 aW9uOgorICAgIGNhc2UgYmFyQ29uZGl0aW9uOgorICAgICAgICByZXR1cm4gZihTaG91bGRDb250 aW51ZTo6WWVzKQorICAgIH0KKyAgICBmKFNob3VsZENvbnRpbnVlOjpObyk7CisgICAgcmV0dXJu OworfQorYGBgCisKICMjIyBCcmFjZXMKIAogW10oI2JyYWNlcy1mdW5jdGlvbikgRnVuY3Rpb24g ZGVmaW5pdGlvbnM6IHBsYWNlIGVhY2ggYnJhY2Ugb24gaXRzIG93biBsaW5lLgo=