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: builtin: must function #32219

Closed
pjebs opened this issue May 24, 2019 · 20 comments
Closed

proposal: Go 2: builtin: must function #32219

pjebs opened this issue May 24, 2019 · 20 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@pjebs
Copy link
Contributor

pjebs commented May 24, 2019

It would be great if there was a generic must function in "builtin" package.

It would be equivalent to this:


eg. 
Say we have an arbitrary function that looks like this:
x, y, z, err := function()

x, y, z is of arbitrary type T1, T2, T3 respectively.

the proposed must( function() ) function will return x of type T1 if err == nil and panic(err) if err != nil

The function is better suited for functions which return 2 values, the last of which is type error. But it can be generalized to any number of return values provided the final return value is of type error.

@gopherbot gopherbot added this to the Proposal milestone May 24, 2019
@mvdan
Copy link
Member

mvdan commented May 24, 2019

Have you seen https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md? The overlap is significant.

@pjebs
Copy link
Contributor Author

pjebs commented May 24, 2019

This proposal is not about error handling (albeit weakly related). The emphasis is syntax sugar and flexibility (eg arbitrary function signatures).

It's about the many occasions when you don't care about the error but just want to use the first result of a function directly in places without creating temporary variables. On other occasions, you do care about the error but want it converted to a panic so you catch it during development (i.e. the error is due to developer error), but during production, you assume no error is possible and you just want to first value directly. The standard library uses the convention MustX(...) in various places for the second scenario.

Due to lack of generics, wrappers are used everywhere in the standard library and people's code due to different data types:

Examples
https://golang.org/src/html/template/template.go#L370 (https://godoc.org/html/template#Must)

This could all be simplified with a generics-compatible builtin "must" function. (NB: builtin already has precedent of generics-compatible functions such as append( ))

Another example where it's useful:

An example from godoc encoding/json.Marshal

type ColorGroup struct {
		ID     int
		Name   string
		Colors []string
	}
	group := ColorGroup{
		ID:     1,
		Name:   "Reds",
		Colors: []string{"Crimson", "Red", "Ruby", "Maroon"},
	}
	b, err := json.Marshal(group)
	if err != nil {
		fmt.Println("error:", err)
	}
	os.Stdout.Write(b)

Sometimes you know that no error is returned because the struct is clearly json-compatible.

It can be simplified to:

type ColorGroup struct {
		ID     int
		Name   string
		Colors []string
	}
	group := ColorGroup{
		ID:     1,
		Name:   "Reds",
		Colors: []string{"Crimson", "Red", "Ruby", "Maroon"},
	}
	os.Stdout.Write(must(json.Marshal(group)))

@pjebs
Copy link
Contributor Author

pjebs commented May 24, 2019

The function doesn't even need to be called must. It's just that Go seems to have a convention called Must in some circumstances where this function is useful.

@ianlancetaylor
Copy link
Contributor

CC @griesemer @mpvl

@ianlancetaylor ianlancetaylor changed the title proposal: builtin: must function proposal: Go 2: builtin: must function May 24, 2019
@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels May 24, 2019
@deanveloper
Copy link

I don't think that Go should promote converting errors to panics without first having better ways to handle errors. Errors in Go are well-known to be pretty cumbersome, having an easy way to convert errors into panics would just promote the idea that people shouldn't properly handle their errors.

Now, if one of the several "named handler" counterproposals to the Go 2 Error Handling draft happen, someone could easily just make a handler called must which panics if an error occurs, then call it with something like check@must errorProneFunc()

@pjebs
Copy link
Contributor Author

pjebs commented May 24, 2019

@deanveloper This proposal is not about error handling. If anything, this is about ignoring errors.

The conversion of errors to panics is a necessarily consequence of the "inside" function potentially returning an error, yet the "must" function having to return a single value.

Also the conversion to panic has an additional benefit of alerting the developer of developer mistakes during local development. That is currently how it is used in standard package's scenario's such as https://godoc.org/html/template#Must and https://golang.org/pkg/regexp/#MustCompile. That is considered idiomatic go. The proposed function just assists developers write idiomatic go - at least for the scenario the standard package uses it with the MustX pattern.

@pjebs
Copy link
Contributor Author

pjebs commented May 24, 2019

@ianlancetaylor why is this a Go2 proposal? Can't it be a Go 1 proposal?

@deanveloper
Copy link

@deanveloper This proposal is not about error handling.

Maybe not error handling in the sense of "what should I do with this error to handle it properly" but in the sense of "this function returns an error and I don't want to deal with it, how can I do this easily?"

Also the conversion to panic has an additional benefit of alerting the developer of developer mistakes during local development. That is currently how it is used in standard package's scenario's such as https://godoc.org/html/template#Must and https://golang.org/pkg/regexp/#MustCompile. That is considered idiomatic go. The proposed function just assists developers write idiomatic go - at least for the scenario the standard package uses it with the MustX pattern.

Yes it is idiomatic for that instance, where the code for writing and using templates is already very verbose, and your program would cease to function if the templates were not done properly. The issue with adding a generalized must builtin is that it encourages people to use it to handle errors in an effort to make their own code less verbose.

The issue is that without better error handling, the must builtin will be used as a substitute for lazy developers who don't want to properly handle errors.

Also the reason it's Go2 is because all language changes are being marked as such. More information here

@pjebs
Copy link
Contributor Author

pjebs commented May 24, 2019

The issue is that without better error handling, the must builtin will be used as a substitute for lazy developers who don't want to properly handle errors.

I disagree. If such a developer does such an action, then they are writing "incorrect" and bad code by ignoring the error they should not be ignoring. That is not the purpose of must. must is used because you know that no error will be returned - albeit with the additional benefit that a panic will occur during local development due to your own developer errors.

In summary, a good developer would only use the "must" function when either:

  1. They know no error is returned (such as my example above) OR
  2. The function they are dealing with SHOULD NOT return an error in production but wants additional assistance detecting it during development process (such as how it's currently used in standard package).

@deanveloper
Copy link

I disagree. If such a developer does such an action, then they are writing "incorrect" and bad code by ignoring the error they should not be ignoring. That is not the purpose of must. must is used because you know that no error will be returned - albeit with the additional benefit that a panic will occur during local development due to your own developer errors.

I don't think that "that is not the purpose of must" exactly nullifies that lazy developers will use it as a substitute for error handling.

In summary, a good developer would only use the "must" function when either:

Again, a good developer sure, but Go is a simple language with a low learning curve. While good developers may understand how to use must properly, lazy developers won't (or just don't care enough). They won't read documentation as to what may cause certain errors to occur, they write a library, don't properly test it, and get complaints from developers that they wrote lazy code.

The reasons that template and other libraries have Must forms is because they are:

  1. Extremely important to the caller that they don't return errors
  2. There are large amounts of inputs that will always work properly
  3. If the inputs are not proper, it's important that the developer knows

There's a reason that there's nothing like http.MustGet, in many instances, the developer needs to handle a network error properly rather than simply panic if one occurs.

@ianlancetaylor
Copy link
Contributor

@pjebs All language changes are considered to be Go 2 proposals. But we are doing language changes. There will probably never be a literal "Go 2". See https://github.com/golang/proposal/blob/master/design/28221-go2-transitions.md .

Separately, I agree with @deanveloper that regardless of the intent, this is in effect an error handling proposal.

@networkimprov
Copy link

@pjebs is right that there are many cases where a function that can return an error won't do so if used in certain ways. In those situations, panic on error is correct, just as it is for slice index out-of-bounds, because there is a programming mistake.

I read this as a proposal on cherry-picking a return value for use in expressions, so you can one-line things that currently require five lines:

_, v, err := someCalc(a, b)
if err != nil {
   panic(err)
}
x := y + v

Alternatives:

x := y + someCalc:2(a, b) // extracts return value; implicit panic

_ v := someCalc#panic(a, b) // specifies handler, e.g. panic/return/<user-defined>
x := y + v

x := y + someCalc#panic:2(a, b) // with handler and extract syntax

See also Requirements to Consider for Go 2 Error Handling, section E.2.

@griesemer
Copy link
Contributor

@pjebs Is your built-in must in any significant way different from the must built-in suggested in #31442?

@pjebs
Copy link
Contributor Author

pjebs commented May 29, 2019

The emphasis is different plus it has a complimentary concept of guard taken from Swift.

My emphasis is simpler. #31442 is more an alternative pattern to error handling.

His suggestion is must is a statement eg w := must os.Open("foo.txt")

Mine (due to different emphasis' such as inlining function calls that return multiple variables), must is a function. The function allows code like: os.Stdout.Write(must(json.Marshal(group)))

My suggestion also renders the current MustX( ) convention redundant.

His suggestion:

As an extra benefit, with the must keyword all functions of the MustXXX() pattern are no longer needed.

still requires it to be wrapped in a function (for the purposes of inlining). (But I could be wrong because maybe must X can still be used in inlining and not just for variable assignment => but it still looks weird like in OOP langauges with the new <Class Name> style which doesn't feel like it belongs in Go)

@pjebs
Copy link
Contributor Author

pjebs commented May 29, 2019

Also, my the intention behind my idea can be generalized as per @networkimprov as syntax sugar to cherry picking a return value. (I disagree with that generalization however because further adding the error => panic conversion seems to be clunky).

#31442 idea can't be generalised as syntax sugar for cherry picking a return value.

@networkimprov
Copy link

@griesemer re cherry-picking a return value, see #32219 (comment)

@pjebs
Copy link
Contributor Author

pjebs commented May 30, 2019

Also my suggestion can be implemented as a function in "builtin".

His suggestion as a statement is a language change.

@dpinela
Copy link
Contributor

dpinela commented Jun 5, 2019

Even if #32437 is implemented, I think this must builtin would complement it nicely, since it would serve a different use case from try, mainly initialising package-level variables.

@ianlancetaylor
Copy link
Contributor

Let's subsume this into #32437. It's a similar idea, with similar benefits and drawbacks. If we can't agree on error handling support (which might solve this issue as well), we aren't going to agree on this either.

@pjebs
Copy link
Contributor Author

pjebs commented Jul 25, 2019

I've implemented a limited version of must in my igo to go transpiler: https://github.com/rocketlaunchr/igo

You can explore its usefulness.

@bradfitz bradfitz added the error-handling Language & library change proposals that are about error handling. label Oct 29, 2019
@golang golang locked and limited conversation to collaborators Oct 28, 2020
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 v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

9 participants