Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: "onerr return" #32848

Closed
kstenerud opened this issue Jun 28, 2019 · 21 comments
Closed

proposal: Go 2: "onerr return" #32848

kstenerud opened this issue Jun 28, 2019 · 21 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Milestone

Comments

@kstenerud
Copy link

kstenerud commented Jun 28, 2019

Proposal: "onerr return"

Edit: Originally this was or return, but I like onerr return better since it's more explicit about what we're doing and why.

Current syntax:

func (this *ReaderThing) readBytesFromFile(filename string, maxBytes int) (bytesRead int, err error) {
    f, err := os.Open(filename)
    if err != nil {
        return 0, err
    }
    ...
}

Proposed syntax:

func (this *ReaderThing) readBytesFromFile(filename string, maxBytes int) (bytesRead int, err error) {
    f, err := os.Open(filename) onerr return 0, err
    ...
}

Basically, if the last thing returned by a function is an error type, onerr checks if it's null, and if not, runs the statement return 0, err.

Specifically:

  • The compiler makes sure that onerr can only follow a function that returns type error as its last return value.
  • The result of os.Open() is stored to f, err as normal.
  • onerr examines the last returned value of the function call (which must be type error), and executes the subclause if it is not nil.
  • The return statement executed with the onerr clause follows standard scoping rules, which means that err is visible to it (as are f and filename and maxBytes).
  • err has already been assigned to by the time the onerr clause executes, and is visible due to scoping rules, so it's perfectly valid to access. There's nothing new or magical here.

Edit:

As an alternative (but only if we can be sure this won't break things or induce too much cognitive load) Allow block statements:

err := DoSomething() onerr {
    fmt.PrintF("We got error %v\n", err)
    do other stuff...
}
@gopherbot gopherbot added this to the Proposal milestone Jun 28, 2019
@the
Copy link

the commented Jun 28, 2019

I like how readable the error handling becomes with this proposal, especially by re-using return. I think using “otherwise” instead of “or” might fit even better, albeit be a bit on the long side for a keyword. Is the idea to only allow return or other statements, too (calling another function, panic, ...)? It reminds me of Perl, is that where the idea came from?

@kstenerud
Copy link
Author

kstenerud commented Jun 28, 2019

Hmm, I'd originally thought of it as orelse but then shortened it to or.

Actually, I suppose technically it could be used as a general error handling keyword:

err := DoSomething() or {
    fmt.PrintF("We got error %n",err)
    do other stuff...
}

But I won't push that far in this proposal if there's resistance to it. It's enough to get rid of the if err != nil ... return boilerplate for now.

@andreygrehov
Copy link

andreygrehov commented Jun 28, 2019

I am not a fan of this syntax. The fact that or return block knows the result of os.Open puts me off. It doesn't feel like a natural flow.

@kstenerud
Copy link
Author

kstenerud commented Jun 28, 2019

err exists in the outside block. The or clause gets executed if it was not nil, and follows normal scoping rules, which makes err visible to it (and f and filename and maxBytes for that matter).

@kstenerud
Copy link
Author

We could also call it onerr:

func (this *ReaderThing) readBytesFromFile(filename string, maxBytes int) (bytesRead int, err error) {
    f, err := os.Open(filename) onerr return 0, err
    ...
}

@andrewrothman
Copy link

I like the idea a lot, but I don't think or is the best keyword. What about reusing else?

@andreygrehov
Copy link

andreygrehov commented Jun 28, 2019

I understand the flow, it just doesn't look natural. Will the or clause allow you to do anything other than return? It may open a can of worms like:

func (this *ReaderThing) readBytesFromFile(filename string, maxBytes int) (bytesRead int, err error) {
    f, err := os.Open(filename) or doSomethingElse() && return 0, err
    ...
}

Which imho adds cognitive load.

@kstenerud
Copy link
Author

No, this isn't supposed to allow chaining. It just means:

func (this *ReaderThing) readBytesFromFile(filename string, maxBytes int) (bytesRead int, err error) {
    f, err := os.Open(filename) [but if err != nil] [do this]
    ...
}

In the strictest version of this proposal, it only accepts return after the or. In the more relaxed version (if we should decide it's worthwhile), it allows a statement, but not an assignment, meaning that you couldn't chain doSomethingElse() and return 0, err because doSomethingElse() has nothing to the left of it to assign to. (the f, err at the beginning are from os.Open() and not usable for assignment by doSomethingElse()).

@kstenerud kstenerud changed the title Proposal: "or return" Proposal: "onerr return" Jun 28, 2019
@owlwalks
Copy link

Very good proposal indeed, tackle exactly what we try to achieve - syntactic sugar for if err != nil { ... }, this is the only proposal that don't make me think how to use it because it’s dead simple, also very minimal impact or might be none at all to current go codebase

@sirkon
Copy link

sirkon commented Jun 29, 2019

Well, this is much better than flawed try they seriously considered to adapt:

stat := try(try(os.Open(fileName)).Stat())

File descriptor leak here — the example taken from comment in a popular widely supported issue where someone proposed to leave everything as is. There’s no RAII in Go to handle this.

@sirkon

This comment has been minimized.

@DisposaBoy
Copy link

Well, this is much better than flawed try they seriously considered to adapt:

stat := try(try(os.Open(fileName)).Stat())

File descriptor leak here — the example taken from comment in a popular widely supported issue where someone proposed to leave everything as is. There’s no RAII in Go to handle this.

No-one (except maybe complete newbies) writes code like this.

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jun 29, 2019
@sirkon

This comment has been minimized.

@ianlancetaylor

This comment has been minimized.

@as
Copy link
Contributor

as commented Jul 4, 2019

stat := try(try(os.Open(fileName)).Stat())
@sirkon

This isn't an optimal example because the file descriptor actually will get cleaned up in the current implementation of Go with this indulgent finalizer:

https://github.com/golang/go/blob/master/src/os/file_windows.go#L59

This undocumented behavior (which certainly shouldn't be part of any compatibility agreement) presents somewhat of a false sense of security to me (not sure about what other people think). However, it would be of benefit to think of other counter-examples for this reason.

@sirkon
Copy link

sirkon commented Jul 4, 2019

@as it can easily be some custom resource handler that has no finalizers

@fkarakas
Copy link

fkarakas commented Jul 7, 2019

would you like to talk english this way too ? "eat apple i like"

@ianlancetaylor
Copy link
Contributor

I kind of like the way that this pushes error handling over to the right, where it is easier to skip when skimming the code.

But I think we need more clarity on what can follow onerr. If the onerr does not return, I guess we just keep execution? But that seems potentially confusing.

Also it's a bit odd to have to write the err over on the left, so that it gets, and then execution jumps over to the right.

@ianlancetaylor
Copy link
Contributor

The return statement executed with the onerr clause follows standard scoping rules, which means that err is visible to it (as are f and filename and maxBytes).

This does not seem true to me. With standard scoping rules the variables declared in an assignment are only visible after the assignment is complete.

    x := 1
    {
        x := x + 1
        fmt.Println(x)
    }

This will print 2: the x in the expression x + 1 is not the variable x being declared in the statement.

I think you are suggesting that the onerr clause introduces a new scope point that occurs after the variables on the left-hand-side have been defined.

@ianlancetaylor
Copy link
Contributor

This proposal does not have strong supported based on votes for the initial comment.

The scoping issue is not resolved. In a case like err := f() onerr return err the second err is not in the scope of the first err with current scoping rules. Any change to those rules to make this work would be quite subtle.

Based on these comments, this looks like a likely decline. Leaving open for a month for final comments.

@ianlancetaylor ianlancetaylor changed the title Proposal: "onerr return" proposal: Go 2: "onerr return" Sep 18, 2019
@ianlancetaylor
Copy link
Contributor

There were no further comments.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests