-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: add collect statements to the language #25626
Comments
How would this work with nesting? |
I imagine nesting wouldn't be an issue. If you imagine the code as written in current go (using block scopes and with
|
@mccolljr I really like the idea of a collect statement. It's almost what I was trying to convey in my gist (https://gist.github.com/KernelDeimos/384aabd36e1789efe8cbce3c17ffa390), with the only difference (non-syntactically, that I can see) being there's no option to continue control flow from the same place after checking the nil-able type (and control flow returns to the parent block, whereas my idea requires the not-nil-handling-code to be in a sub-scope is it may be called more than once like in a loop structure). I assume that's a worthy sacrifice for how much simpler what you propose would be to implement, and how useful it would be in a wide variety of cases. I think what you propose could also work with more than one variable at a time, provided the compiler checks that any assignment with a _! is consistent with types specified after collect func GetSomething() (s *Something) {
var err error
collect (s, err) {
_!, _! = fetchOrNil1()
_!, _! = fetchOrNil2()
// The below line would cause a compiler error
_! = errorOrNil()
// The behaviour for the below line should
// probably also be a compiler error
_!, _ = new(Something)
}
return s
} |
@KernelDeimos Interesting suggestion for collecting more than one variable. If that were the approach, though, I'd want to not use the same However, I'm not really sure what the example above would be doing. When you have |
Is there any purpose in the Also, is there any expected behavior for non-pointer types? Like what would happen with Edit - Not sure if it's coming off as me being super critical, mostly just curious haha |
I feel like this would just bring about a |
@luke-park Perhaps it could be convention to have |
I'm happy people are engaging with the idea! I chose As for collecting to values of non-pointer types, I'm on the fence. On the one hand, I can see the usefulness if a developer treats zero values as invalid, but on the other hand, value types often have useful/meaningful zero values. I'm not sure if it makes sense to include non-pointer types, but I'd be interested to hear others thoughts as to why or why not. |
@luke-park I can understand that concern, but I also think you're overestimating the useful scope of a collect statement. For small functions I could see that being the case, certainly, but I also think it can make those functions much cleaner, ala the I feel like it wouldn't be any more or less bad than switch statements, which also get used to keep a bunch of if statements from muddying the flow of the code. You trade an extra level of indentation for clarity elsewhere. I personally like the indentation because it helps me visually parse the code. I think the vast majority of the code inside collect statements would be a series of function calls, and I think being able to read through that series of calls linearly without having to skip over interspersed nil checks etc. makes it much easier to follow the intended path of the code, despite the extra indentation. |
I get the overall idea but this proposal as is introduces 2 new elements with a special handling flow not obvious at first sight, it seems a lot to reduce the burden of error handling to me. |
I don't think I agree with this assessment. Go has unique keywords that aren't "obvious at first sight" to someone who has yet to learn the language, but that become de rigueur very quickly. The choice of keyword
It's meant to address far more than error handling. In the original proposal I put up the idea that the underlying problem that people want addressed when they complain about error handling is really the handling of I would be interested in your thoughts after going through some of your code and re-writing to use That would help me understand how others would think of and use this construct. I'm admittedly close to it, as I've spent some time whittling down versions of this idea until I came to this version. Outside perspective is always helpful. |
switch statement exists in many languages so I don't think it's a good counterexample, most of the people know this statement and the way it works. Concerning select and go they handle asynchronous code, so how they are working is related to the purpose they are intended for, the flow is special but to handle a non orthogonal thing which is concurrency.
I missed this point from your original proposition, thanks to enlighten me.
Sure, it saves lines and the code is more concise, I have nothing to say against that. What I wanted to say with my comment is, changes this proposal want to introduce, both in language and the way the code flow works, seems big to me regarding what they solve. In my opinion, it doesn't seem to fit what I understand from the go philosophy. |
I get your point, but don't forget that go's switch statement behaves quite different than other languages. For one, it defaults to breaking after each case rather than falling through, it allows type switch syntax, and some other niceties that are irregular. There's definitely some stuff that needs to be learned to really understand what is going on with a switch statement in go. I'd even say go's
That's good stuff (not being sarcastic - I really do appreciate your explanation). I personally feel the opposite. Solutions such as adding |
Regarding the If you did this, you could also (and I'm deviating a bit here from the original proposal) use labels instead of defining which variables are being var err error
var data []byte
collect MyCoolLabel {
data, !err = somefunction()
}
MyCoolLabel:
return err Perhaps there could be different prefixes for different purposes:
@mccolljr in response to your question earlier about the use for multiple not-nil checks ( |
FWIW, I've been working today to hack this feature in to the compiler. It's been pretty dang easy so far, and I feel like it's because I chose a single identifier and placed the otherwise-illegal character at the end of it. It wouldn't necessarily have to be
var err error
var data []byte
collect MyCoolLabel {
data, !err = somefunction()
}
MyCoolLabel:
return err I actually don't hate that. I had to look at it for a bit, but it's pretty clever. I'll probably try to hack that into the compiler after I finish my version. Or, perhaps, a hybrid version where you can specify a label to jump to rather than just the end of the block (under the hood I'm just implementing it with syntax rewriting using I'd probably put the |
At first I thought Also, in terms of Also, labels are a bad idea. They make messy code imo, I really like the original syntax with I would like to point out that this try {
// several io operations
} catch (Exception err) {
// handle error
} versus var err error
collect err {
// several io operations
}
if err != nil {
// handle error
} BUT the redeeming thing in Go is that both options are available. Using The problem with |
I think the similarity with |
It's hard for me to evaluate your proposal because your examples that use the current Go language are written in a style that I find unusual and unnecessarily complicated. The second code listing in the initial post could be rewritten to be something like this:
And this is the same for the second example in the third post. I don't see a benefit to adding To make a convincing case fo |
The point I was making is that I really don't like preambles, I should have been more clear about that. I want to think about handling an error after (or when) it happens, not before. I hate typing out a function, then when I realize I need to error check, need to go above (for the |
@deanveloper If I understand what you're saying, I'd argue that still counts as being "visual". If you're using Of course, a language shouldn't require you to have a fancy editor, but my point is that I don't think a language should really care about stuff like that. Another example that I think will make my argument more concrete is that it's relatively easy to preempt when you need some kind of error handler. The same thought process is necessary when writing loops - rarely do you write the code to operate on an array element before realizing that you need a loop structure. |
I have a very rudimentary implementation of collect statements: https://github.com/mccolljr/go/tree/collect The error messages are not great, although better than I expected before doing anything to correct. If you want to test it out, clone the repo, switch to the Notes
Here's a sample program I've been using to test: package main
import "io/ioutil"
import "flag"
import "fmt"
import "encoding/json"
import "os"
import _ "github.com/mattn/go-sqlite3"
import "database/sql"
type Conf struct {
Server struct {
Host string
Port string
}
Database struct {
Name string
}
}
var (
CONF Conf
DB *sql.DB
)
func LoadConf() error {
var (
err error
data []byte
)
__collect__ err {
data, _! = ioutil.ReadFile("conf.json")
_! = json.Unmarshal(data, &CONF)
}
return err
}
func ParseFlags() error {
flag.StringVar(&CONF.Server.Host, "sh", CONF.Server.Host, "-sh=127.0.0.1")
flag.StringVar(&CONF.Server.Port, "sp", CONF.Server.Port, "-sp=8080")
flag.StringVar(&CONF.Database.Name, "db", CONF.Database.Name, "-db=\":memory:\"")
flag.Parse()
return nil
}
func LaunchDB() (err error) {
__collect__ err {
DB, _! = sql.Open("sqlite3", CONF.Database.Name)
_! = DB.Ping()
}
return
}
func main() {
var err error
// perform initialization
__collect__ err {
_! = LoadConf()
_! = ParseFlags()
_! = LaunchDB()
}
// exit on error
if err != nil {
fmt.Println("fatal:", err)
os.Exit(1)
}
fmt.Printf("%+v\n", CONF)
} |
@adg You're definitely right about the current examples being poor. I've got a fork with this implemented enough to test, and I'm going to convert some of my existing code to get better example material. If you want, you can install my fork and play with it as well. |
Even if switch can have capabilities that you need to know, the overall behaviour is mostly what you could expect from a switch. The same for the if and for statement, there are things particular you need to learn yes, but mostly they behave like in other languages, no surprise about it. When you say
I feel the same when I look at the example you provided, the flow is not as straightforward as a list of if check on error. You need to switch your mind to understand the way things behave in a collect block, "only" (I'm not saracastic as well) to handle nil value where plain if check are really expressive and without any ambiguity. I can't say more regarding to my original comment. |
From the initial post, it looks like a One way to accomplish this in Go now is with function literals: func TryComplexOperation() (*Result, error) {
var err error
var r1, r2, r3 *Result
collect := func(f func()) {
if err == nil {
f()
}
}
collect(func() { r1, err = Step1() })
collect(func() { r2, err = Step2(r1, "something") })
collect(func() { r3, err = Step3(r2, 12) })
return r3, err
} Here the guard is a function whose logic can be arbitrary, and not restricted to a predicate like in your example. |
@smasher164 A general note - I've been reworking my fork into a much cleaner and more general state. I will update here when I have it building and passing all tests, stay tuned. |
When reading the proposal, I keep thinking it would be useful to check for more things than just the func ComplexOperation() (*Result, error) {
var err error
var r *Reslult
collect err == nil && !r.Satisfactory() {
// keep going until the expression is false.
r, err = Step1()
r, err = Step2(r)
r, err = Step3(r)
}
return r, err
} Then the Assuming func ComplexOperation() (*Result, error) {
var err error
var r *Reslult
r, err = Step1() // avoids r == nil case.
collect err == nil && !r.Satisfactory() {
// never enter if err != nil or r.Satisfactory() == true after Step1()
r, err = Step2(r)
r, err = Step3(r)
}
return r, err
} UPDATE: This is really just a possible syntax for @smasher164 guard example If you won't to avoid the closures. |
In concept I love the idea, but I'd like to propose an alternate that would not introduce any new keywords nor any new special syntax, just an extension of
|
Did you consider This feature tries things until the var gets a non-zero value. Also I imagine that Edit: this is directed to the OP. |
@mikeschinkel and @networkimprov, A problem with your example @mikeschinkel is the A problem with both mine and your example though, is that it's not really clear when the break/collect/try check should be re-calculated. For every line? For every semicolon? Every time either of the variables in the check are updated? What if you send a pointer to the variable to a sub-scope or function? What is a line in the eyes of the compiler? In contrast, this is clear with the original I tried for myself to update my proposal with explicit markers, but then I feel it probably doesn't add enough benefit over relying on what's possible with todays Go features? I.e.
Although now that I read it with "break", it looks a lot nicer than it did with "collect", I feel this probably isn't a sginificant improvment over this:
UPDATE: Although on the same note, perhaps it could be argued that |
It may be helpful to extend the proposed construct to support two kinds of errors:
Many APIs (e.g. os.File) can report catastrophic errors, which should terminate the program. It would be nice to avoid this:
|
@mccolljr, @mikeschinkel for clarity I formalized my syntax suggestion and split it out as a new proposal. I suppose it's different enough that it should not be discussed in detail here. Sorry for the spam, and thank you for the inspiration! |
Good point. But it was not shadowed, it was just a typo. Fixed. BTW, I can get on board with @networkimprov's keyword
Yes, I can see that. I made an assumption that it would be obvious when it would be recalculated, but obviously I was wrong in thinking it would be obvious. You are correct that lines are not appropriate, which is why I was envisioning it would break from the scope when This could be done by the compiler implementing assignment to As for the explicit You say its not clear what Further, use of BTW, I have been using the following construct for a while to address the use-case that @mccolljr's proposal appears to be trying to address, and it works extremely well if not rather verbose. So my inline proposal was simply using syntax sugar (and I am a big fan of syntax sugar) to do what I have already proven works extremely well in practice:
|
@mikeschinkel, that's some nice points. I didn't see this reply before after creating #27075. Feel free to continue the discussion there. |
@adg, the most benefit is in situations where you handle the error in the same function:
versus:
And it's better not to read three lines of ceremonial boilerplate for each function call when returning errors:
|
@networkimprov I really like your syntax, but not the adoption of the
|
@networkimprov The problem with the suggestion of using a triggering expression to implicitly define possible checking points is that it's hard in general to detect all possible such points statically. For example:
Once you take the address of |
@randall77 the triggering expression was suggested by @smyrman, and is currently under discussion in #27075 (which has explicit checkpoints). My suggestions were keyword |
I haven't been here in a while, but I'd like to give an update with my experience. Work has gotten busy again so I had to put work on this on hold for a while. That being said, I found as I was writing test cases that I use this feature far less than I expected I might. As mentioned by someone else (I apologize, not sure who at the moment), this does tend to add an extra level of indentation to many functions... something I'm not too jazzed about in practice. I see a lot of suggestions in this thread and I will read through them when I have a little more time, but as it stands I actually feel my proposal, having tried it in the wild, is not as helpful I once thought it to be. |
A better solution, perhaps, would be to allow a block of go code to break with a value. Maybe (100% spitballing): result, err := {
e, temp1 := trySomething()
break (nil, e) if e // implicitly means "if e is non-zero"
e, temp2 := tryOther(temp1)
break (nil, e) if e
break (alwaysOk(temp2), nil) // no if here, always break
// if no break were to be called, the values assigned from the block would contain their zero values
} Still, though, I don't think it's as helpful of a construct as I had originally thought |
FYI: There are some pretty similar semantics to collect in @rsc's draft proposal here with a
One interested detail seams to be a suggestion that this could work: func printSum(a, b string) error {
handle err { return err }
fmt.Println("result:", check strconv.Atoi(x) + check strconv.Atoi(y))
return nil
}
|
The |
#21161 has seen a lot of proposals regarding error handling. Based on the discussions there and my experience writing go, I'd like to propose what I call a
collect
statement. The root of this idea is that the complaints leveled against Go's error handling being repetitive/verbose are really complaints that checking fornil
in go code is repetitive/verbose.I could see it being beneficial to have a way to "collect" to a variable - that is, try a series of statements until a non-
nil
value is encountered. I've proposed syntax and control flow here, but I'm not particularly married to either. My proposed style isn't without reason, though. I feel that it fits with go's philisophy of being able to read code linearly, and having to explicitly announce intent in code.A
collect
statement would be of the formcollect [IDENT] [BLOCK STMT]
, where ident must me an in-scope variable of anil
-able type. Within acollect
statement, a special variable_!
is available as an alias for the variable being collected to._!
cannot be used anywhere but as an assignment, same as_
. Whenever_!
is assigned to, an implicitnil
check is performed, and if_!
is not nil, the block ceases execution.Theoretically, this would look something like this:
which is equivalent to this code, which will compile (provided all necessary functions etc. are defined, of course)
Aside from error handling, I think a
collect
statement has some other value additions.For example, imagine you'd like to try to retrieve a value in a series of ways
With
collect
you could write that asAlso consider the scenario where you have several subsets of a larger operation, which can be annotated separately to give better error contxt
New syntax features required:
collect
(I'd need to see how often this ident occurs in wild go code, I haven't run a scan on my GOROOT or GOPATH yet)_!
(I've implemented this in the parser to see if it would be possible, and it's not terribly difficult to make this match as an ident without allowing users to declare it as a name for a variable, etc)I think something like this would provide developers with a clean and concise way to express code that may encounter nil values, while keeping the control flow nice and linear.
I'd really like to see this discussed, even if it ends up not being adopted, because as I've developed the concept I've increasingly fallen in love with the idea. Because conceptually equivalent code can be created today (as demonstrated above), I think the impact on the language would be minimal except for at the syntactic level.
The text was updated successfully, but these errors were encountered: