Code Style: Opinion on returning void

# Daniel Bates (a month ago)

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

Opinions, please.

Dan

Contact us to advertise here
# Tim Horton (a month ago)

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I prefer:

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

+1 to a style guide update

# Ryosuke Niwa (a month ago)

On Thu, Feb 7, 2019 at 12:53 PM Tim Horton <timothy_horton at apple.com> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I much prefer doing this in my own code but stay away from it in WebKit because we tend to have a separate return.

I prefer: >

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

+1 to a style guide update

Yeah, we might as well as codify it in the style guideline for clarity.

  • R. Niwa
# Saam Barati (a month ago)

On Feb 7, 2019, at 12:53 PM, Tim Horton <timothy_horton at apple.com> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

# Geoffrey Garen (a month ago)

FWIW, I’ve always felt conflicted about this case.

I very much prefer early return to if/else chains.

However, “return f()” when f returns void is a bit mind bending.

So, I can’t use my preferred style in functions that return void. Boo.

Geoff

# Alex Christensen (a month ago)

If you search for “return completionHandler” in WebKit you will find over a hundred instances. Most if not all of them return void. It means call the completion handler and return. I probably wrote most of them and obviously think it’s a fabulous idiom.

# Zalan Bujtas (a month ago)

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

Alan.

On Thu, Feb 7, 2019 at 7:31 PM Alex Christensen <achristensen at apple.com>

wrote:

# Chris Dumez (a month ago)

Same here, I used it in PSON code with completion handlers. I liked the more concise code but I also do not feel strongly about it.

The extra return line often would have meant adding curly brackets for if statements leading to early returns.

Chris Dumez

# Keith Miller (a month ago)

I’ve definitely done this in JSC before. As with everyone else, I don’t feel particularly strongly about it.

On Feb 7, 2019, at 8:45 PM, Chris Dumez <cdumez at apple.com> wrote:

Same here, I used it in PSON code with completion handlers. I liked the more concise code but I also do not feel strongly about it.

The extra return line often would have meant adding curly brackets for if statements leading to early returns.

IMO, this is why I think all if/elses should have curlies that way I don’t have to think about whether or not I need them, but ¯_(ツ)_/¯. It would also make refactoring code much easier.

# Daniel Bates (a month ago)

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

Patch posted bugs.webkit.org/show_bug.cgi?id=194848 for

review of language and examples.

Summary of this thread is in that bug, too.

Dan

# Ryosuke Niwa (a month ago)

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff and Alex both expressed preferences for being able to return void, and Saam pointed out that there is a lot of existing code which does this. Zalan also said he does this in his layout code.

  • R. Niwa
# Daniel Bates (a month ago)

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Chris Dumez (a month ago)

I also prefer allowed returning void.

Chris Dumez

# Saam Barati (a month ago)

I prefer it as well.

# Daniel Bates (a month ago)

Okay, you’ve changed your mind from your earlier email of not having a strong opinion. Would have been good to know from the get-go. Better late than never knowing :/

Dan

On Feb 20, 2019, at 6:58 AM, Chris Dumez <cdumez at apple.com> wrote:

I also prefer allowed returning void.

Chris Dumez

On Feb 19, 2019, at 10:35 PM, Daniel Bates <dbates at webkit.org> wrote:

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Daniel Bates (a month ago)

Thanks for the opinion!

Dan

On Feb 20, 2019, at 7:26 AM, Saam Barati <sbarati at apple.com> wrote:

I prefer it as well.

  • Saam

On Feb 20, 2019, at 6:58 AM, Chris Dumez <cdumez at apple.com> wrote:

I also prefer allowed returning void.

Chris Dumez

On Feb 19, 2019, at 10:35 PM, Daniel Bates <dbates at webkit.org> wrote:

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Chris Dumez (a month ago)

On Feb 20, 2019, at 7:48 AM, Daniel Bates <dbates at webkit.org> wrote:

Okay, you’ve changed your mind from your earlier email of not having a strong opinion. Would have been good to know from the get-go. Better late than never knowing :/

I did not change my mind. I said I was using this pattern in my code. So did other people. If we use it in our code, it is because we prefer it. What I meant to say is that if a majority of people felt strongly that we should not allow this pattern, then I would not stand in the way.

I don’t think this mail thread showed that people strongly feel that we should not allow this pattern. Therefore, I was also surprised by your email saying that we’d reached a consensus.

# Simon Fraser (a month ago)

I find it mind bending. It makes me wonder if the author made a coding error.

Simon

# Mark Lam (a month ago)

I also prefer it, and I think some coding patterns may require it e.g. in templates where sometimes we want to specialize into a void function, and other times into a function that returns a value. However, this is rarely needed in practice. Without being able to return void, writing such a template will be a pain if not impossible.

Mark

# Said Abou-Hallawa (a month ago)

On Feb 20, 2019, at 10:35 AM, Mark Lam <mark.lam at apple.com> wrote:

I also prefer it, and I think some coding patterns may require it e.g. in templates where sometimes we want to specialize into a void function, and other times into a function that returns a value. However, this is rarely needed in practice. Without being able to return void, writing such a template will be a pain if not impossible.

But the template and the macro cases are different. The void return in this case is implicit and you do not recognize it unless you see the caller.

template<typename Functor>

auto caller(const Functor& functor) { return functor(); }

Nothing in the return statement above says it will return void. But if you pass it a pointer to a returning void function it will. So outside the scope of macros and templates, I think returning void is not nor really needed.

# Michael Catanzaro (a month ago)

On Wed, Feb 20, 2019 at 12:33 PM, Simon Fraser <simon.fraser at apple.com>

wrote:

I find it mind bending. It makes me wonder if the author made a coding error.

Yeah me too. It does seem to work nicely in Alex's CompletionHandler example, but still, I'd just add braces and return on an extra line if I was writing it.

Anyway, it seems we don't have any consensus here, so no need to create a rule.

Michael

# Maciej Stachowiak (a month ago)

It seems like return foo(); where foo() is a void function can always be replaced with foo(); return; for greater clarity at the cost of one extra line break. For people who prefer the one-line style, can you say why you don’t like the other way?

# Chris Dumez (a month ago)

On Feb 20, 2019, at 1:14 PM, Maciej Stachowiak <mjs at apple.com> wrote:

It seems like return foo(); where foo() is a void function can always be replaced with foo(); return; for greater clarity at the cost of one extra line break. For people who prefer the one-line style, can you say why you don’t like the other way?

We do not allow more than one statement per line so it would be: foo(); return;

Also, since we favor early returns in WebKit, things like: If (!nok) return completionHandler(Failed);

Would become: If (!nok) { completionHandler(Failed); return; }

It is not a big deal but I personally prefer the most concise version. Especially, it is not uncommon to have multiple early returns. I think more concise is better and I personally do not see a readability issue here. It does not really matter what the completion handler is returning.

# Alex Christensen (a month ago)

I like it mostly for its brevity, but I also think it would be strange that changing a return type from bool to void or vice versa would require touching all its call sites.

# Darin Adler (a month ago)

I tried not to weigh in on this, but my view on the materials we should use to build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I don’t like about using “return” with a call to a void-returning function.

  • You can’t use this idiom when you want to ignore the result of a function, only if the function actually returns void. Often functions are designed with ignorable and often ignored return values. For example, it’s common to call something that might fail and have a good reason to ignore whether it failed or not. The idiom we are discussing requires treating those functions differently from ones that return void. If you refactor so that a function now returns an ignorable return value you need to visit all the call sites using return and change them into the two line form.

  • It works for return, but not for break. I’ve seen at least one case where someone filled a switch statement with return statements instead of break statements so they could use this more-terse form. Now if we want to add one line of code after that switch, we need to convert all those cases into multi-line statements with break.

  • Unless it’s mandatory, it’s a case where two programmers can make different style choices. If both forms are acceptable, then it introduces a little bit of disorder into our code.

One thing I like about it is that since “pass along the return value from this inner function to this outer function” can be written this way, the code can then survive refactorings where both the inner and outer functions might gain or lose the return value.

— Darin

# Ryosuke Niwa (a month ago)

On Thu, Feb 21, 2019 at 9:31 AM Darin Adler <darin at apple.com> wrote:

I tried not to weigh in on this, but my view on the materials we should use to build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I don’t like about using “return” with a call to a void-returning function.

  • You can’t use this idiom when you want to ignore the result of a function, only if the function actually returns void. Often functions are designed with ignorable and often ignored return values. For example, it’s common to call something that might fail and have a good reason to ignore whether it failed or not. The idiom we are discussing requires treating those functions differently from ones that return void. If you refactor so that a function now returns an ignorable return value you need to visit all the call sites using return and change them into the two line form.

Perhaps this is the key distinction we want to make. Maybe if the intent is to ignore the function's return value regardless of what it may return in the future, then we should have a separate "return". If the intent is to return whatever that other function returns, then we should return void. But having to think about the intent of code isn't a great thing for coding style guideline.

  • It works for return, but not for break. I’ve seen at least one case where

    someone filled a switch statement with return statements instead of break statements so they could use this more-terse form. Now if we want to add one line of code after that switch, we need to convert all those cases into multi-line statements with break.

This is an orthogonal point but I do prefer switch'es that return directly over one that break's then having a separate return after the switch statement especially if the switch statement is long.

I've gotta say I'm fascinated to learn that many people are bothered by returning void. I guess I was never bothered by it because I see a return statement as a thing that returns the control back to the caller, not a thing that returns a value.

  • R. Niwa
# Daniel Bates (15 days ago)

I just had to change a void function to a non-void function in https://bugs.webkit.org/show_bug.cgi?id=195281. I was very happy that all

of the existing code was written by people that did not return void because I didn't have to change any call sites returning void into two lines. This saved me time, kept the patch minimal, avoided dirtying line history, and avoiding me having to do a double-take whenever I see a return void.

Is there a path forward for this issue? Should I put together an online survey for people to vote? I don't want this issue to fall into a void ;)

Dan

On Feb 21, 2019, at 10:32 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Thu, Feb 21, 2019 at 9:31 AM Darin Adler <darin at apple.com> wrote:

I tried not to weigh in on this, but my view on the materials we should use to build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I don’t like about using “return” with a call to a void-returning function.

  • You can’t use this idiom when you want to ignore the result of a function, only if the function actually returns void. Often functions are designed with ignorable and often ignored return values. For example, it’s common to call something that might fail and have a good reason to ignore whether it failed or not. The idiom we are discussing requires treating those functions differently from ones that return void. If you refactor so that a function now returns an ignorable return value you need to visit all the call sites using return and change them into the two line form.

Perhaps this is the key distinction we want to make. Maybe if the intent is to ignore the function's return value regardless of what it may return in the future, then we should have a separate "return". If the intent is to return whatever that other function returns, then we should return void. But having to think about the intent of code isn't a great thing for coding style guideline.

  • It works for return, but not for break. I’ve seen at least one case where

    someone filled a switch statement with return statements instead of break statements so they could use this more-terse form. Now if we want to add one line of code after that switch, we need to convert all those cases into multi-line statements with break.

This is an orthogonal point but I do prefer switch'es that return directly over one that break's then having a separate return after the switch statement especially if the switch statement is long.

I've gotta say I'm fascinated to learn that many people are bothered by returning void. I guess I was never bothered by it because I see a return statement as a thing that returns the control back to the caller, not a thing that returns a value.

  • R. Niwa
# Nico Weber (16 hours ago)

On Thu, Feb 21, 2019 at 12:31 PM Darin Adler <darin at apple.com> wrote:

I tried not to weigh in on this, but my view on the materials we should use to build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I don’t like about using “return” with a call to a void-returning function.

  • You can’t use this idiom when you want to ignore the result of a function, only if the function actually returns void. Often functions are designed with ignorable and often ignored return values. For example, it’s common to call something that might fail and have a good reason to ignore whether it failed or not. The idiom we are discussing requires treating those functions differently from ones that return void. If you refactor so that a function now returns an ignorable return value you need to visit all the call sites using return and change them into the two line form.

That works, you just have to spell it return (void)non_void_fun(); :D

Want more features?

Request early access to our private beta of readable email premium.