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: revisit explicit annotations in := #21303

Closed
jimmyfrasche opened this Issue Aug 4, 2017 · 24 comments

Comments

Projects
None yet
@jimmyfrasche
Member

jimmyfrasche commented Aug 4, 2017

Before Go 1 := would only work if every name on the lhs had not been declared in the current scope. This made it hard to write common code like

x, err := f()
//...
y, err := g()

Either y would require an explicit declaration with var or there would need to be an ugly workaround such as

y, err2 := g()
err = err2

Among other peccadilloes this meant copying a bit of valid code from one place to another could make it invalid and it would need to be rewritten.

This made := much less useful than it could be so the current behavior of implicitly reusing existing names defined in the current scope was introduced and erroring out if none of the names were new.

This made := much more useful, but has made it too easy to shadow variables in outer scopes.

When it was introduced, there was much discussion in the mailing list thread announcing the change (apologies but I cannot find a link) for a way to explicitly annotate the names to reuse such as

y, ^err := g() // mnemonic for "reusing variable defined above"

This was dismissed—if I recall correctly (again apologizes for not being able to track down the original thread)—because it would mean that copying a bit of valid code from one place to another could make it invalid, as annotations would need to be added or removed depending on the new context.

However, this is subtly true with the current behavior := depending on the names involved and what is or is not in the current scope, leading to bugs caused by accidental shadowing that are hard to track down due to the implicit nature of what is shadowed when. This is particularly vexing with named returns.

With explicit annotation it would still be possible to accidentally shadow a variable, but it would be easier to see what was happening at a glance and possibly easier to detect mechanically.

I think the idea of explicit annotation should be revisited for Go 2. I propose the following semantics

  1. x, y, z := f() follows the pre-Go 1 semantics—all names must be unbound in the current scope
  2. x, ^y, ^z := f() reuses y and z as defined in the current scope or an outer but non-global scope†
  3. annotating a name that does not exist is an error
  4. at least one name on the lhs must not be annotated

† making it incompatible with reusing variables in package global scope is a bit uneven but seems a wise safety precaution

This should be entirely go fix-able as the correct annotations to mirror the Go 1 semantics could be applied mechanically.

@gopherbot gopherbot added this to the Proposal milestone Aug 4, 2017

@gopherbot gopherbot added the Proposal label Aug 4, 2017

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Aug 4, 2017

Member

Dup of #377?

Member

bradfitz commented Aug 4, 2017

Dup of #377?

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche Aug 4, 2017

Member

That thread covers a lot of potential changes to :=. It looks like the explicit annotation was brought up after a dozen or so posts and eventually dominates the conversation (I only skimmed). I don't mind this being closed as a dup but #377 is closed due to age.

Member

jimmyfrasche commented Aug 4, 2017

That thread covers a lot of potential changes to :=. It looks like the explicit annotation was brought up after a dozen or so posts and eventually dominates the conversation (I only skimmed). I don't mind this being closed as a dup but #377 is closed due to age.

@davecheney

This comment has been minimized.

Show comment
Hide comment
@davecheney

davecheney Aug 4, 2017

Contributor
Contributor

davecheney commented Aug 4, 2017

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche Aug 4, 2017

Member

@davecheney that's Go 1 code.

Member

jimmyfrasche commented Aug 4, 2017

@davecheney that's Go 1 code.

@Azareal

This comment has been minimized.

Show comment
Hide comment
@Azareal

Azareal Aug 4, 2017

^ is the bitwise xor operator.

I'm not really sure this is that useful to be honest.
If I think something's going to be shadowed, I usually explicitly declare the variables.

Azareal commented Aug 4, 2017

^ is the bitwise xor operator.

I'm not really sure this is that useful to be honest.
If I think something's going to be shadowed, I usually explicitly declare the variables.

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Aug 4, 2017

Member

Another alternative would be to annotate the new names instead of the old: that would make a ShortVarDecl equivalent to an Assignment.

For example, we could shift the colons from the = symbol to the new names:

x:, err: = f()
//
y:, err = g()
Member

bcmills commented Aug 4, 2017

Another alternative would be to annotate the new names instead of the old: that would make a ShortVarDecl equivalent to an Assignment.

For example, we could shift the colons from the = symbol to the new names:

x:, err: = f()
//
y:, err = g()
@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche Aug 4, 2017

Member

I don't really care about the syntax. There are other suggestions in #377 and the mailing list thread linked in there. I'm sure the Go team will pick something reasonable and pleasant in the face of all other constraints, if they decide to implement.

I'll use ^ in any examples that I write for consistency with the first post. I know it's an operator already but I think the polysemy should be fine in this case since a ^ would already be illegal in any place where this would come up, but, if it's not, that's fine.

I just want to be able to explicitly and concisely reuse variables from outer scopes without having to entirely and awkwardly rewrite the line with multiple var declarations. I want it to be easier to spot and avoid shadowing.

Annotating new names rather than old names would make it easier to avoid accidental shadowing in the first place.

I was thinking that another option would be to keep the default rules of := as it is now, but allow annotations to mean "use the one in an outer scope". That way Go 1 and Go 2 code would be source compatible (wrt this feature, at least) without needing a go fix. I'm not sure how far the plan is to go with that since that would mean no new keywords. It would mean you'd only be able to annotate old names, though.

I'd prefer "you always need to annotate" as it would mean not seeing an annotation had as much meaning as seeing an annotation, even though that would mean having to annotate more often.

Member

jimmyfrasche commented Aug 4, 2017

I don't really care about the syntax. There are other suggestions in #377 and the mailing list thread linked in there. I'm sure the Go team will pick something reasonable and pleasant in the face of all other constraints, if they decide to implement.

I'll use ^ in any examples that I write for consistency with the first post. I know it's an operator already but I think the polysemy should be fine in this case since a ^ would already be illegal in any place where this would come up, but, if it's not, that's fine.

I just want to be able to explicitly and concisely reuse variables from outer scopes without having to entirely and awkwardly rewrite the line with multiple var declarations. I want it to be easier to spot and avoid shadowing.

Annotating new names rather than old names would make it easier to avoid accidental shadowing in the first place.

I was thinking that another option would be to keep the default rules of := as it is now, but allow annotations to mean "use the one in an outer scope". That way Go 1 and Go 2 code would be source compatible (wrt this feature, at least) without needing a go fix. I'm not sure how far the plan is to go with that since that would mean no new keywords. It would mean you'd only be able to annotate old names, though.

I'd prefer "you always need to annotate" as it would mean not seeing an annotation had as much meaning as seeing an annotation, even though that would mean having to annotate more often.

@as

This comment has been minimized.

Show comment
Hide comment
@as

as Aug 7, 2017

Contributor

I don't really care about the syntax.

Then why not just use

x, err := f()
y, e2 := g(); err=e2
Contributor

as commented Aug 7, 2017

I don't really care about the syntax.

Then why not just use

x, err := f()
y, e2 := g(); err=e2
@rogpeppe

This comment has been minimized.

Show comment
Hide comment
@rogpeppe

rogpeppe Aug 7, 2017

Contributor

Before Go 1 := would only work if every name on the lhs had not been declared in the current scope.

I don't believe this is true, although I remember the discussion on changing the semantics of :=.
See my earlier comment: #21161 (comment)

From that previous discussion, ISTR favouring a syntax like this:

:a, :err = foo()     // equivalent to a, err := foo()
:b, err = foo()      // b is newly declared; err is not.

This doesn't avoid accidental shadowing though - I'm pretty sure you
don't want to ban all shadowed variable names.

Contributor

rogpeppe commented Aug 7, 2017

Before Go 1 := would only work if every name on the lhs had not been declared in the current scope.

I don't believe this is true, although I remember the discussion on changing the semantics of :=.
See my earlier comment: #21161 (comment)

From that previous discussion, ISTR favouring a syntax like this:

:a, :err = foo()     // equivalent to a, err := foo()
:b, err = foo()      // b is newly declared; err is not.

This doesn't avoid accidental shadowing though - I'm pretty sure you
don't want to ban all shadowed variable names.

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche Aug 7, 2017

Member

@as I don't care about the specific syntax used as long as there is some syntax with the desired semantics

@rogpeppe I could easily be misremembering or confusing it with a comment from the Go team that "it worked like that but then we changed it". Maybe even another language entirely. I tried to find the thread to check before posting but could not so I crossed my fingers and posted. Oh well.

At any rate, no I don't want to ban shadowing.

Shadowing is good. But it can also be awkward and easy to shadow too much.

The proposal is for some way to keep the nice bits of := but add a way to explicitly allow reuse of a variable defined in an outer scope. The current behavior can cause subtle problems especially with named returns and common names like err so use of := requires a dose of paranoia.

If that's the :a, err = foo() syntax, great.

Member

jimmyfrasche commented Aug 7, 2017

@as I don't care about the specific syntax used as long as there is some syntax with the desired semantics

@rogpeppe I could easily be misremembering or confusing it with a comment from the Go team that "it worked like that but then we changed it". Maybe even another language entirely. I tried to find the thread to check before posting but could not so I crossed my fingers and posted. Oh well.

At any rate, no I don't want to ban shadowing.

Shadowing is good. But it can also be awkward and easy to shadow too much.

The proposal is for some way to keep the nice bits of := but add a way to explicitly allow reuse of a variable defined in an outer scope. The current behavior can cause subtle problems especially with named returns and common names like err so use of := requires a dose of paranoia.

If that's the :a, err = foo() syntax, great.

@carlmjohnson

This comment has been minimized.

Show comment
Hide comment
@carlmjohnson

carlmjohnson Aug 7, 2017

Contributor

I would like it if shadowing an outer scope in the short declaration were banned.

So, for example, this would be legal:

func Example() (int, error) {
    x, err := thingOne()
    if err != nil {
        return 0, err
    }
    y, err := thingTwo()
    if err != nil {
        return 0, err
    }
    return x+y, nil
}

Because while err is reused, it does not shadow an outer err.

But this would be illegal:

func Example() (int, error) {
    x, err := thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        y, err := thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

Because the inner err shadows an outer one.

I think something like a shadow ban is necessary to prevent the footgun of accidental shadowing.

Contributor

carlmjohnson commented Aug 7, 2017

I would like it if shadowing an outer scope in the short declaration were banned.

So, for example, this would be legal:

func Example() (int, error) {
    x, err := thingOne()
    if err != nil {
        return 0, err
    }
    y, err := thingTwo()
    if err != nil {
        return 0, err
    }
    return x+y, nil
}

Because while err is reused, it does not shadow an outer err.

But this would be illegal:

func Example() (int, error) {
    x, err := thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        y, err := thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

Because the inner err shadows an outer one.

I think something like a shadow ban is necessary to prevent the footgun of accidental shadowing.

@rogpeppe

This comment has been minimized.

Show comment
Hide comment
@rogpeppe

rogpeppe Aug 7, 2017

Contributor

@carlmjohnson
What about this example?

func Example() (int, error) {
	x, err := thingOne()
	if err != nil {
		return 0, err
	}
	f := func() int {
		y, err := thingTwo()
		if err != nil {
			log.Println(err)
		}
		return y
	}
	return f(), nil
}
Contributor

rogpeppe commented Aug 7, 2017

@carlmjohnson
What about this example?

func Example() (int, error) {
	x, err := thingOne()
	if err != nil {
		return 0, err
	}
	f := func() int {
		y, err := thingTwo()
		if err != nil {
			log.Println(err)
		}
		return y
	}
	return f(), nil
}
@carlmjohnson

This comment has been minimized.

Show comment
Hide comment
@carlmjohnson

carlmjohnson Aug 7, 2017

Contributor

I think that having to call the error err2 is a small price to pay for making closures close over what you expect in other cases.

Contributor

carlmjohnson commented Aug 7, 2017

I think that having to call the error err2 is a small price to pay for making closures close over what you expect in other cases.

@dotaheor

This comment has been minimized.

Show comment
Hide comment
@dotaheor

dotaheor Sep 15, 2017

How about ban short declarations except the ones in the first parts of control flow blocks?

Short declarations bring too many confusions than their convenience. And they are much less readable than the long declarations.

dotaheor commented Sep 15, 2017

How about ban short declarations except the ones in the first parts of control flow blocks?

Short declarations bring too many confusions than their convenience. And they are much less readable than the long declarations.

@lpar

This comment has been minimized.

Show comment
Hide comment
@lpar

lpar Jan 29, 2018

How about ban short declarations except the ones in the first parts of control flow blocks?

I'll go further: How about we have just one assignment syntax? Allow var syntax in control flow initializers, and use it everywhere. Or allow := syntax to specify types and use that everywhere instead (assigning the zero value where applicable).

I don't care too much what the syntax is, but it bothers me that there are multiple syntaxes and I have to remember which one to use when.

lpar commented Jan 29, 2018

How about ban short declarations except the ones in the first parts of control flow blocks?

I'll go further: How about we have just one assignment syntax? Allow var syntax in control flow initializers, and use it everywhere. Or allow := syntax to specify types and use that everywhere instead (assigning the zero value where applicable).

I don't care too much what the syntax is, but it bothers me that there are multiple syntaxes and I have to remember which one to use when.

@object88

This comment has been minimized.

Show comment
Hide comment
@object88

object88 Jan 30, 2018

I confess that something about having two different declarations makes me itch just a little bit. If the short declaration is removed, the previous example becomes...

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        var y int
        y, err = thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

Or, to shadow by way of scoping,

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        var y int
        var err error
        y, err = thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

However, effective redeclaration via shadowing would be disallowed:

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }

    var y int
    var err error // ERROR
    y, err = thingTwo()
    if err != nil {
       return 0, err
    }
    x += y

    return x, nil
}

I don't care for the added verbosity, and I'm rather fond of the := operator. But like this it is extremely explicit, which improves readability, both of which are nice.

object88 commented Jan 30, 2018

I confess that something about having two different declarations makes me itch just a little bit. If the short declaration is removed, the previous example becomes...

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        var y int
        y, err = thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

Or, to shadow by way of scoping,

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }
    if x > 1 {
        var y int
        var err error
        y, err = thingTwo()
        if err != nil {
            return 0, err
        }
        x += y
    }
    return x, nil
}

However, effective redeclaration via shadowing would be disallowed:

func Example() (int, error) {
    var x int
    var err error
    x, err = thingOne()
    if err != nil {
        return 0, err
    }

    var y int
    var err error // ERROR
    y, err = thingTwo()
    if err != nil {
       return 0, err
    }
    x += y

    return x, nil
}

I don't care for the added verbosity, and I'm rather fond of the := operator. But like this it is extremely explicit, which improves readability, both of which are nice.

@object88

This comment has been minimized.

Show comment
Hide comment
@object88

object88 Jan 30, 2018

Going off on a slight tangent, the verbosity above could be improved if we could use var to declare multiple variables, using the same syntax as function parameters:

var x, y int, err error

object88 commented Jan 30, 2018

Going off on a slight tangent, the verbosity above could be improved if we could use var to declare multiple variables, using the same syntax as function parameters:

var x, y int, err error
@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Jan 30, 2018

Contributor

I really can't imagine that Go 2 would remove either := or var. Yes, they do similar things, but they are not the same. We can usefully discuss tweaks to them, such as this proposal, but I don't see any point to discussing actually removing one or the other.

Contributor

ianlancetaylor commented Jan 30, 2018

I really can't imagine that Go 2 would remove either := or var. Yes, they do similar things, but they are not the same. We can usefully discuss tweaks to them, such as this proposal, but I don't see any point to discussing actually removing one or the other.

@carlmjohnson

This comment has been minimized.

Show comment
Hide comment
@carlmjohnson

carlmjohnson Feb 28, 2018

Contributor

Commenting to add a cross-reference to #21114.

Contributor

carlmjohnson commented Feb 28, 2018

Commenting to add a cross-reference to #21114.

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Mar 7, 2018

Contributor

Closing as dup of #377.

Contributor

ianlancetaylor commented Mar 7, 2018

Closing as dup of #377.

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche May 15, 2018

Member

@ianlancetaylor #377 is "locked and limited to collaborators."

Member

jimmyfrasche commented May 15, 2018

@ianlancetaylor #377 is "locked and limited to collaborators."

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor May 15, 2018

Contributor

@jimmyfrasche Are you disagreeing that this should be closed as a dup?

Still, I'll unlock #377 for now.

Contributor

ianlancetaylor commented May 15, 2018

@jimmyfrasche Are you disagreeing that this should be closed as a dup?

Still, I'll unlock #377 for now.

@jimmyfrasche

This comment has been minimized.

Show comment
Hide comment
@jimmyfrasche

jimmyfrasche May 15, 2018

Member

No, it is a dup. I had assumed that by posting a summary in #377 that you were attempting to consolidate the discussion there which wouldn't work very well if no one could discuss. I figured you either forgot to unlock it or didn't notice it was locked and was trying to be helpful.

Member

jimmyfrasche commented May 15, 2018

No, it is a dup. I had assumed that by posting a summary in #377 that you were attempting to consolidate the discussion there which wouldn't work very well if no one could discuss. I figured you either forgot to unlock it or didn't notice it was locked and was trying to be helpful.

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor May 15, 2018

Contributor

@jimmyfrasche Thanks. I sincerely hope that #377 doesn't turn into a long-running discursive discussion. I don't think that is helpful.

Contributor

ianlancetaylor commented May 15, 2018

@jimmyfrasche Thanks. I sincerely hope that #377 doesn't turn into a long-running discursive discussion. I don't think that is helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment