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: add collect statements to the language #25626

Closed
mccolljr opened this issue May 29, 2018 · 41 comments
Closed

proposal: Go 2: add collect statements to the language #25626

mccolljr opened this issue May 29, 2018 · 41 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

@mccolljr
Copy link

#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 for nil 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 form collect [IDENT] [BLOCK STMT], where ident must me an in-scope variable of a nil-able type. Within a collect 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 implicit nil check is performed, and if _! is not nil, the block ceases execution.

Theoretically, this would look something like this:

func TryComplexOperation() (*Result, error) {
    var result *Result
    var err error
    
    collect err {
        intermediate1, _! := Step1()
        intermediate2, _! := Step2(intermediate1, "something")
        // assign to result from the outer scope
        result, _! = Step3(intermediate2, 12)
    }
    return result, err
}

which is equivalent to this code, which will compile (provided all necessary functions etc. are defined, of course)

func TryComplexOperation() (*Result, error) {
    var result *Result
    var err error

    {
        var intermediate1 SomeType
        intermediate1, err = Step1()
        if err != nil { goto collectEnd }
        
        var intermediate2 SomeOtherType
        intermediate2, err = Step2(intermediate1, "something")
        if err != nil { goto collectEnd }

        result, err = Step3(intermediate2, 12)
        // if err != nil { goto collectEnd }, but since we're at the end already we can omit this
    }

collectEnd:
    return result, err
}

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

  1. retrieve from a local shared cache
  2. retrieve from a global shared cache
  3. instantiate a new object

With collect you could write that as

// try several approaches for acquiring a value
func GetSomething() (s *Something) {
    collect s {
        _! = getLocalCache()
        _! = getGlobalCache()
        _! = new(Something)
    }
    return s
}

Also consider the scenario where you have several subsets of a larger operation, which can be annotated separately to give better error contxt

func TryComplexOperation() (*Result, error) {
    var (
        ioErr error
        bytes1, bytes2 []byte
    )

    collect ioErr {
        bytes1, _! = GetBytes()
        bytes2, _! = GetOtherBytes()
    }

    if ioErr != nil {
        return nil, fmt.Errorf("read error: %s", ioErr)
    }

    var (
        parseErr error
        target1 SomeStruct
        target2 SomeStruct2
    )

    collect parseErr {
        _! = json.Unmarshal(bytes1, &target1)
        _! = json.Unmarshal(bytes2, &target2)
    }

    if parseErr != nil {
        return nil, fmt.Errorf("parse error: %s", err)
    }

    return &Result{target1, target2}, nil
}

New syntax features required:

  1. keyword 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)
  2. special ident _! (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.

@gopherbot gopherbot added this to the Proposal milestone May 29, 2018
@mccolljr mccolljr changed the title proposal: go2: add collect statements to the language proposal: Go 2: add collect statements to the language May 29, 2018
@ianlancetaylor ianlancetaylor added LanguageChange v2 A language change or incompatible library change labels May 29, 2018
@deanveloper
Copy link

How would this work with nesting?

@mccolljr
Copy link
Author

mccolljr commented May 29, 2018

I imagine nesting wouldn't be an issue. If you imagine the code as written in current go (using block scopes and goto), it would look like this:

with collect
func TryComplexOperation() (*Result, error) {
	var result *Result
	var err error
    
	collect err {
		var (
			subError error
			intermediate1 SomeType
			intermediate2 SomeType2
		)

		collect subError {
			intermediate1, _! = trySomething()
			intermediate2, _! = trySomething2()
		}
		_! = subError
		result, _! = trySomething3(intermediate1, intermediate2)
	}
	
	return result, err
}
with goto using current go syntax
func TryComplexOperation() (*Result, error) {
	var result *Result
	var err error

	{
		var (
			subError      error
			intermediate1 SomeType
			intermediate2 SomeType2
		)

		{
			intermediate1, subErr = trySomething()
			if subErr != nil {
				goto endInnerCollect
			}

			intermediate2, subErr = trySomething2()
			if subErr != nil {
				goto endInnerCollect
			}
		}
	endInnerCollect:

		err = subError
		if err != nil {
			goto endOuterCollect
		}
		result, err = trySomething3(intermediate1, intermediate2)
		// if err != nil { goto endOuterCollect }, omitted because we're at the end anyway
	}
endOuterCollect:
	return result, err
}

I think it wouldn't make much sense to allow using _! as a collection target. I think the proposal forbids that, anyway, as it wouldn't technically count as an "assignment". Each collect block has its own _! mapping. I'm having trouble coming up with a valid reason to assign to the collector of the outer collect block from an inner collect block.

@KernelDeimos
Copy link

KernelDeimos commented May 29, 2018

@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
}

@mccolljr
Copy link
Author

@KernelDeimos Interesting suggestion for collecting more than one variable. If that were the approach, though, I'd want to not use the same _! identifier for each collected variable. I'd prefer something like _1, _2 etc.

However, I'm not really sure what the example above would be doing. When you have _!, _! = fetchOrNil1(), you'd exit the block if the function returned an error OR returned a valid *Something. I'm not sure that's what you'd want in that scenario. I realize that's just some pseudocode that I put together and that you modified, though. Would you be able to provide an example from some real world code where collecting on 2 variables would be helpful? I'm not saying it can't be, I'm just having trouble thinking of what that situation would be.

@deanveloper
Copy link

deanveloper commented May 30, 2018

Is there any purpose in the _!? Why not continue using err (or err!?) (or whatever variable you are collecting?) It may be more clear as to what's going on.

Also, is there any expected behavior for non-pointer types? Like what would happen with var n int; collect n { ... }?

Edit - Not sure if it's coming off as me being super critical, mostly just curious haha

@luke-park
Copy link

I feel like this would just bring about a collect statement for every function. I'm not necessarily opposed to the idea, but it feels unclean to think that a vast majority of function bodies will be indented twice because they all start with collect statement.

@deanveloper
Copy link

@luke-park Perhaps it could be convention to have collect be on the same indent-level, similar to switch or select? Although switch and select only share the indent with case statements, so I'm not sure as to how it would look for collect...

@mccolljr
Copy link
Author

I'm happy people are engaging with the idea! I chose _! for a couple of reasons - 1 being that ! is used in other languages to denote something that warrants extra attention / consideration. Ruby has an idiom where methods that modify their receivers end with !. Kotlin uses ! to assert a nillable type is non-nil. Rust uses ! to call a macro to make clear it's not just a function call. It could also be _? for similar reasons. Why it should be any of these at all is reason 2 - it makes it clear that something more than just an assignment is happening. Along with these, there's the added benefits of being easy to visually scan and see where in a collect statement an assignment to the collection target is happening, is familiar to go developers since _ is something we're used to seeing when we want to ignore an assignment. It also seemed reasonable to keep the _ and this new symbol similar, since they both have similar restrictions on when they could be used. You wouldn't be able to use _! as a value, same as _.

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.

@mccolljr
Copy link
Author

@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 GetSomething() example.

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.

@antham
Copy link

antham commented May 30, 2018

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.

@mccolljr
Copy link
Author

@antham

this proposal as is introduces 2 new elements with a special handling flow not obvious at first sight

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. go, for example, suggests that it may "go" do something, but what that actually is isn't obvious until you read about it. It does introduce one new construct with a handling flow that differs from the rest of the code... like switch statements, or select statements. I actually think if you'd never read a collect statement before, or this proposal, you might still have a decent idea what was going on.

The choice of keyword collect is certainly up for discussion. There may be a more descriptive keyword that could be chosen. I can see the term collect being unclear. Perhaps collapse or until would be more clear.

it seems a lot to reduce the burden of error handling to me

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 nil values in go. It's a new control flow statement that is meant to allow go developers to write more concise and linearly readable code when dealing with potentially nil values. I believe the savings in lines of code more than justifies the addition of a single keyword. _! is not a breaking change, because it's not currently a valid identifier, and it would even be possible to introduce a keyword including an illegal character like ! so that the addition of the keyword isn't a breaking change.

I would be interested in your thoughts after going through some of your code and re-writing to use collect statements. What are the savings in lines of code? Is it more / less than I expect? Does a side-by-side comparison of the old version demonstrate that the collect version is easier or harder to reason about (assuming the reader knows what collect does)?

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.

@antham
Copy link

antham commented May 30, 2018

@mccolljr

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.

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 nil values in go.

I missed this point from your original proposition, thanks to enlighten me.

I would be interested in your thoughts after going through some of your code and re-writing to use collect statements. What are the savings in lines of code? Is it more / less than I expect? Does a side-by-side comparison of the old version demonstrate that the collect version is easier or harder to reason about (assuming the reader knows what collect does)?

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.

@mccolljr
Copy link
Author

@antham

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.

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 for and if statements have some capabilities that are unique to go. range clauses in for statements, initializer statements for if statements, etc.

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.

That's good stuff (not being sarcastic - I really do appreciate your explanation). I personally feel the opposite. Solutions such as adding ?: or trap are things I consider not to fit with the go philosophy, in the sense that they either introduce syntax meant to move statements critical to control flow out of the place where they matter, i.e. where they'd be "executed", or they provide shorthand syntax for very specific cases, or introduce a lot of new visual complexity to the language. I feel collect, as a concept but also as proposed, provides a more streamlined way to express something that is very frequently expressed in go code anyway. I'd like to hear more about why you disagree with that, or where you feel it goes against go's philosophy. You're not the first to voice this concern, and it's something I'd like to better understand.

@KernelDeimos
Copy link

KernelDeimos commented May 31, 2018

Regarding the !_ of which @deanveloper suggested could be identifiers instead, what if identifiers were prefixed with ! to indicate a != nil check? For instance, data, !err := somefunction() or !err1, !err2 := somefunction()

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 collected

  var err error
  var data []byte
  collect MyCoolLabel {
    data, !err = somefunction()
  }
MyCoolLabel:
  return err

Perhaps there could be different prefixes for different purposes:

  • ! goto label if not nil (must be nil-able type)
  • ? goto label if false (must be bool type)

@mccolljr in response to your question earlier about the use for multiple not-nil checks (_!, _! := fetchOrNil1()), I hadn't actually thought of one before I wrote the example. However, if would enable checking multiple error objects if that was useful for any reason.

@mccolljr
Copy link
Author

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 _!, it could just as easily be boo? or _1_^.

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 collected

  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 goto and autogenerated labels anyway).

I'd probably put the ! at the end of the identifier since it's much easier to parse that way.

@deanveloper
Copy link

@mccolljr

The choice of keyword collect is certainly up for discussion. There may be a more descriptive keyword that could be chosen. I can see the term collect being unclear. Perhaps collapse or until would be more clear.

At first I thought until was really good, but then I had realized that until is used by several other languages as a type of loop. May seem confusing. Also, collapse just feels unintuitive.

Also, in terms of !err... definitely not. It should be err!, as "!" Expression is already used for inverting bools and would definitely be confusing.

Also, labels are a bad idea. They make messy code imo, I really like the original syntax with collect Variable and _! much better.

I would like to point out that this collect pattern is very similar to try/catch, which I didn't like at first...

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 collect is extremely redeeming for if you're handling several errors at once, just like try/catch is extremely useful for large blocks of code.

The problem with try/catch is for pesky one-liners which have a lot of boilerplate if you use try/catch but in Go we would still have if err != nil to handle one-liners, which is much more useful.

@mccolljr
Copy link
Author

I think the similarity with try/catch is purely visual. panic/defer, functionally speaking, is Go's closest equivalent to try/catch. collect lacks any and all ability to affect the flow of the program outside of the function in which it is defined, which I think is a very key difference.

@adg
Copy link
Contributor

adg commented May 31, 2018

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:

func TryComplexOperation() (*Result, error) {
	result1, err := Step1()
	if err != nil {
		return nil, err
	}
	result2, err := Step2(result1, "something")
	if err != nil {
		return nil, err
	}
	return Step3(result2, 12)
}

And this is the same for the second example in the third post.

I don't see a benefit to adding collect based on this example. It doesn't show off the power of collect. Rather, it shows how awkward Go code can be made better. (The obvious response to that is "Don't write awkward Go code in the first place.")

To make a convincing case fo collect (to me, at least) you'll need to show a problematic function (or ideally many such functions) written in today's idiomatic Go, and how that function would be improved with the use of collect.

@deanveloper
Copy link

@mccolljr

I think the similarity with try/catch is purely visual.

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 try), below (for the catch), and in front of (to indent it) just to check for errors. Again, because we still have if err != nil, which is purely after the error-prone function, this isn't a problem.

@KernelDeimos
Copy link

@deanveloper If I understand what you're saying, I'd argue that still counts as being "visual". If you're using vim, for example, you can type Otry to put a "try" above the line, a plugin indents the rest for you, 2ja}catch from normal mode and you're golden.

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.

@mccolljr
Copy link
Author

mccolljr commented May 31, 2018

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 collect branch, and build as you would normally install build go from source. Make sure you set the GOBINDIR environment variable to the directory you want these new go binaries to end up in.

Notes

  1. The build will succeed, but there are some test that fail towards the very end of all.bash. I'll get to those ASAP, but for now the go binaries will still have been built, and you can still use them to build & run code using collect statements. The failed tests don't appear significant at first glance.
  2. Keyword collect is implemented as __collect__ in this implementation, for testing & to avoid collisions.
  3. You can't currently use _! on the left-hand side of a := statement, only =. I made this decision because it made some problem easier to solve, I'm just drawing a blank as to what it was. It's not a permanent solution, it just helped in getting a reasonably working implementation faster.
  4. Some tools will not work with this fork (as expected). guru still works in most cases, autocomplete via gocode still seems mostly there, but gofmt and goimports fail miserably.
  5. I've only tested this on a Mac

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)
}

@mccolljr
Copy link
Author

@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.

@antham
Copy link

antham commented May 31, 2018

@mccolljr

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

they provide shorthand syntax for very specific cases, or introduce a lot of new visual complexity to the language

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.

@smasher164
Copy link
Member

smasher164 commented Jun 4, 2018

From the initial post, it looks like a collect statement is a guard against a scope, where we exit the scope if some value is not valid, or conversely stay in the scope if some value is not invalid. See https://en.wikipedia.org/wiki/Guarded_Command_Language.

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.

@mccolljr
Copy link
Author

mccolljr commented Jun 7, 2018

@smasher164
I like that in concept but you'd have to define more explicitly what the collect function does. I don't like the idea of adding the overhead of closures and function calls just for some visual clarity.

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.

@smyrman
Copy link

smyrman commented Aug 16, 2018

When reading the proposal, I keep thinking it would be useful to check for more things than just the != nil case when going to the step of expanding the syntax. Why not make the statement more like a for-loop or if statement with a check clause, so you could express more things...

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 ! or _! syntax is not needed, and the whole things becomes a bit more readable (in my opinion), and more powerful.

Assuming func (*Result) Satisfactory() bool that returns false when r == nil, otherwise, the check could also be written as:

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.

@mikeschinkel
Copy link

mikeschinkel commented Aug 18, 2018

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 break and the use of a break triggering expression to evaluate after each statement inside the break's { ... } scope:

break err!=nil {
    err = foo()
    num,err := bar()
    num,str,err := baz()
    if something_else_is_wrong() {
       err= errors.New("Something else is wrong!")
       break
    }
    fmt.Printf("Num: %v and Str: %v", num, str )
}
if err != nil {
    fmt.Printf("Oh, the HORROR!" )
}

@networkimprov
Copy link

networkimprov commented Aug 18, 2018

Did you consider try as the keyword instead of collect? If so, why the latter?

This feature tries things until the var gets a non-zero value. Also I imagine that try is clearer to those with C++, Java, or Javascript chops.

Edit: this is directed to the OP.

@smyrman
Copy link

smyrman commented Aug 18, 2018

@mikeschinkel and @networkimprov, break and try seams like a nice word to use, but I am no longer sure if my own suggestion to this post of letting collect do more is a good idea.

A problem with your example @mikeschinkel is the :=, as it looks like err inside the break scope is shadowed, and would therefore be ignored by the break check.

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 collect proposal from @mccolljr because the explicit _! syntax marks where the != nil (or other zero-value?) check should be re-calculated.

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.

func ComplexOperation() (*Result, error) {
	var err error
	var r *Reslult

	break err == nil && !r.Satisfactory() {
	step:
		r, err = Step1()
	step:
		r, err = Step2(r)
	step:
		r, err = Step3(r) // no recalculation done here
		r, err = Step4(r)
	}
        return r, err 
}

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:

func ComplexOperation() (*Result, error) {
	var err error
	var r *Reslult

	if err == nil && !r.Satisfactory() {
		r, err = Step1()
	if err == nil && !r.Satisfactory() {
		r, err = Step2(r)
	}
	if err == nil && !r.Satisfactory() {
		r, err = Step3(r) // no recalculation done here
		r, err = Step4(r)
	}
        return r, err 
}

UPDATE: Although on the same note, perhaps it could be argued that select / case doesn't offer a significant value over if / else neither? Yet it's really convenient to use and sometimes easier to read.

@networkimprov
Copy link

It may be helpful to extend the proposed construct to support two kinds of errors:

  • expected/manageable: as proposed
  • unexpected/fatal: execute log.Fatal(), panic(), or user-defined func
try err, log.Fatal { // optional 2nd arg is a func taking argument of type error
   _! = f1() // any error lands in err
   _# = f2() // any error is passed to log.Fatal
}

Many APIs (e.g. os.File) can report catastrophic errors, which should terminate the program. It would be nice to avoid this:

try err {
   _! = f1()
   ebad := f2()
   if ebad != nil {
      log.Fatal(ebad)
   }
}

@smyrman
Copy link

smyrman commented Aug 18, 2018

@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!

@mikeschinkel
Copy link

mikeschinkel commented Aug 18, 2018

@smyrman

" A problem with your example @mikeschinkel is the :=, as it looks like err inside the break scope is shadowed, and would therefore be ignored by the break check."

Good point. But it was not shadowed, it was just a typo. Fixed.

BTW, I can get on board with @networkimprov's keyword try instead of break as it would be just as good a word to use IMO.

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?"

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 err was assigned a not-nil value.

This could be done by the compiler implementing assignment to err as a closure with a guard condition and any assignment would thus trigger a break out of the scope.

As for the explicit _! syntax that is what I object to most. _ is basically /dev/null and so what does /dev/null! mean? How does an ! modify a special variable implemented to allow values assigned to it disappear?

You say its not clear what break err!=nil means? I'd say it would be a lot easier to learn and remember that break err!=nil breaks on change in err!=nil rather than to grok something that fundamentally changes the meaning of the special _ variable name.

Further, use of _! adds yet another "magic symbol" to the language and squats on a syntax that could not reasonably be in any other scope in the future in the case that a better, more obvious use for that syntax were to emerge whereas a break err !=nil { ... } would be explicitly limited to the scope it was used with.

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:

var err error
for {
    err = foo()
    if err!=nil {
        break
    }
    num,err := bar()
    if err!=nil {
        break
    }
    num,str,err := baz()
    if err!=nil {
        break
    }
    if something_else_is_wrong() {
        err= errors.New("Something else is wrong!")
        break
    }
    fmt.Printf("Num: %v and Str: %v", num, str )
    break
}
if err != nil {
    fmt.Printf("Oh, the HORROR!" )
}

@smyrman
Copy link

smyrman commented Aug 18, 2018

@mikeschinkel, that's some nice points. I didn't see this reply before after creating #27075. Feel free to continue the discussion there.

@networkimprov
Copy link

networkimprov commented Aug 18, 2018

@adg, the most benefit is in situations where you handle the error in the same function:

func F(stream) {
   var a, b, c T
   var err error

   a, err = f1()
   if err == nil {
      b, err = f2()
      if err == nil {
         c, err = f3()
         // this pattern is hard to read and may continue to great depth
         // using if err != nil { goto HandleError } may be preferable to indenting
      }
   }
   if err != nil {
      fmt.Fprintf(stream, "error %s", err.Error())
   } else {
      fmt.Fprintf(stream, "result %d", calc(a,b,c))
   }
}

versus:

try err {
   a, _! = f1()
   b, _! = f2()
   c, _! = f3()
}
if err != nil {
   ...
}

And it's better not to read three lines of ceremonial boilerplate for each function call when returning errors:

a, err = f1()
if err != nil {
   return err
}
b, err = f2()
if err != nil {
   ...

@mikeschinkel
Copy link

@networkimprov I really like your syntax, but not the adoption of the _! construct. Better to keep it simple like so:

try err {
   a, err := f1()
   b, err := f2()
   c, err := f3()
}
if err != nil {
   ...
}

@randall77
Copy link
Contributor

randall77 commented Aug 20, 2018

@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:

try err != nil {
    err = foo()  // this is one
    p = &err
    q := &otherErr
    if someComplicatedCondition() {
        q = p
    }
    *q = foo() // is this one?
}

Once you take the address of err, all of the assignments to it become hard to track. They can happen in other functions, even.
You either have to give up and say assignments by pointer don't count, or enforce that you can't take the address of any variable in a condition (and also err can't be a global variable). Either seems unsatisfactory.

@networkimprov
Copy link

networkimprov commented Aug 20, 2018

@randall77 the triggering expression was suggested by @smyrman, and is currently under discussion in #27075 (which has explicit checkpoints).

My suggestions were keyword try, and flagging fatal errors with _#.

@mccolljr
Copy link
Author

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.

@mccolljr
Copy link
Author

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

@smyrman
Copy link

smyrman commented Aug 30, 2018

FYI: There are some pretty similar semantics to collect in @rsc's draft proposal here with a handle keyword (similar to defer for registering error handler) and a corresponding explicit check keyword:

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
}

A check applies to an expression of type error or a function call returning a list of values ending in a value of type error.

@networkimprov
Copy link

networkimprov commented Aug 30, 2018

The collect block is essentially try, and the new check/handle proposal is essentially catch, so they're similar ideas at root. Meanwhile I'm working on a revision of that: #27519

@golang golang locked and limited conversation to collaborators Sep 15, 2019
@seankhliao seankhliao added the error-handling Language & library change proposals that are about error handling. label Jan 16, 2021
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