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: Assigning to fields with short declaration notation #6842

Closed
niemeyer opened this issue Nov 28, 2013 · 15 comments
Closed

Proposal: Assigning to fields with short declaration notation #6842

niemeyer opened this issue Nov 28, 2013 · 15 comments
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@niemeyer
Copy link
Contributor

Is there any good reason for this to be unsupported:

    x.f, err := f()

This would be unambiguous, as it would only work if x was previously declared and err
was not.

The fact it doesn't work forces the use of:

    var err error
    x.f, err = f()

Which while okay, doesn't feel like an improvement over the former version.

As is known, there's also precedence for the use of := on reassignments, given that the
statement:

    x, err := f()

may reassign x if it was already declared in the current scope.
@robpike
Copy link
Contributor

robpike commented Nov 28, 2013

Comment 1:

The := notation is a shorthand for common cases. It's not meant to cover every possible
declaration one may write. I'd prefer to leave as is, but won't close this until others
have weighed in.

Labels changed: added priority-someday, languagechange, removed priority-triage.

Status changed to Thinking.

@adg
Copy link
Contributor

adg commented Nov 28, 2013

Comment 2:

I was squinting at something like Gustavo's first example in a readability review
recently, actually wondering if it should work. I think it should work and can't think
of any drawbacks to the change, apart from complicating the definition of :=.

@cznic
Copy link
Contributor

cznic commented Nov 29, 2013

Comment 3:

In the first level, := was a syntactic sugar for a handy case
        var v T           // (1)
        v = exprOfTypeT
so it can be written as just
        v := exprOfTypeT  // (2)
(1) and (2) relate in a simple way. Per this proposal, it would do also allow
        var v t // (3)
        w.x, v = exprOfTypeOfFieldXFromStructW, exprOfTypeT
Shortening the explicit form (3) to
        w.x, v := exprOfTypeOfFieldXFromStructW, exprOfTypeT // (4)
means that (3) and (4) are no more related in such a simple way, IMO rendering the
original sweetness toxic. Consider (by extension)
        *p, v := f()
        s[x], x := g() 
        h(i(j())).z[a]().b, y := k(), x // etc.
I don't think this improves readability. It's much easier to miss the fact that {v,x,y}
are being declared because of the distraction of the now possibly unbound complexity of
the LHSs. In contrast to the clear meaning of the status quo dead simple LHSs legal with
:=
        a, b, c := f()
        d, e, f := expr1, expr2, expr3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-main.

@griesemer
Copy link
Contributor

Comment 6:

Owner changed to @griesemer.

@ascheglov
Copy link

A slightly different case is when it happens in the if statement:
if x.f, ok := f(); !ok {
You usually want that ok variable visible only inside that if statement, and you don't want to declare it in outer scope.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@jamesgarfield
Copy link

It seems like the drawback to this change is in the error, or not ok, code path.

When a function returns a value or an error, sometimes that value is nil, other times it is just the zero value.

When you are just assigning to variables, this is fine. However, once you start allowing struct properties or array indices to also be on the LHS, you open yourself to the (likely) possibility of assigning a new value into your struct/array during an error case, which is suboptimal at best.

@gkop
Copy link

gkop commented Jun 17, 2016

(I'm a newb who found this behavior surprising) Is the current best practice to forward-declare err and use =, or use a temporary local variable for the struct field? I suppose "it depends". Is there a good article on the subject?

@griesemer
Copy link
Contributor

@gkop It's common to write code such as:

res1, err := f1(...)
if err != nil ...
res2, err := f2(...)
if err != nil ...

and re-use the same error variable. Sometimes the results can be local, so one might write

if res, err := f(...); err == nil {
   // use res
} else {
   // handle err, etc
}

It's rarely necessary to "forward-declare" an error variable (usually only if the other variables are not new and need to be used and there's no error variable yet - again, this is very rare).

@gkop
Copy link

gkop commented Jun 17, 2016

@griesemer thanks! I'm particularly interested, what about when one of the variables we're assigning is a struct field and err isn't yet declared?

@griesemer
Copy link
Contributor

@gkop You will have to declare that error variable somewhere; e.g.:

var err error
s.f, err = f()
if err ...

@griesemer griesemer changed the title spec: Assigning to fields with short declaration notation Proposal: Assigning to fields with short declaration notation Nov 5, 2016
@griesemer griesemer modified the milestones: Proposal, Unplanned Nov 5, 2016
@griesemer
Copy link
Contributor

Changed this into a proposal so we can collect feedback and eventually make a decision.

@rsc
Copy link
Contributor

rsc commented Nov 21, 2016

Let's leave this with #377 for now and revisit if and when we get to Go 2.

@rsc rsc closed this as completed Nov 21, 2016
@rsc rsc added v2 A language change or incompatible library change and removed Proposal labels Nov 21, 2016
@golang golang locked and limited conversation to collaborators Nov 21, 2017
@bradfitz
Copy link
Contributor

This bug has reappeared as #30318.

vdot0x23 referenced this issue in vdot0x23/authelia Sep 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests