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

Open
kstenerud opened this issue Jun 28, 2019 · 20 comments

Comments

@kstenerud
Copy link

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

@gopherbot gopherbot added the Proposal label Jun 28, 2019

@the

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented Jun 28, 2019

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

This comment has been minimized.

Copy link

commented Jun 28, 2019

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

@andreygrehov

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Author

commented Jun 28, 2019

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

This comment has been minimized.

Copy link

commented Jun 29, 2019

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

This comment has been minimized.

Copy link

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 was marked as disruptive content.

Copy link

commented Jun 29, 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).

+1

I mean yet another person who barely uses Go is actively “improving” it. Not surprised.

@DisposaBoy

This comment has been minimized.

Copy link

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.

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

@sirkon

This comment was marked as disruptive content.

Copy link

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.

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

LOL, another “smart”. Boy, the sooner you understand we humans are naturally apes the better. Please remember Murphy’s law. It actually works.

PS if we weren’t idiots something like Marx described as communism would rule the world.

@ianlancetaylor

This comment was marked as outdated.

Copy link
Contributor

commented Jun 30, 2019

Please keep the conversion polite and on topic. Thanks.

@as

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Jul 4, 2019

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

@fkarakas

This comment has been minimized.

Copy link

commented Jul 7, 2019

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.