-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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: spec: various changes to := #377
Comments
I noticed this situation a while ago. I argued that it conforms to the scope rules, which are usual and customary. The first err is declared under rule 4. The second err is declared under rule 5. The second declaration is the inner declaration, so the inner redeclaration rule applies, thereby hiding, within its own scope, the first err. This is the usual and customary behaviour for many languages. Some languages have a construct which allows a reference in the inner scope to the variable in the outer scope. The Go Programming Language Specification Declarations and scope The scope of a declared identifier is the extent of source text in which the identifier denotes the specified constant, type, variable, function, or package. Go is lexically scoped using blocks: 1. The scope of a predeclared identifier is the universe block. 2. The scope of an identifier denoting a constant, type, variable, or function declared at top level (outside any function) is the package block. 3. The scope of an imported package identifier is the file block of the file containing the import declaration. 4. The scope of an identifier denoting a function parameter or result variable is the function body. 5. The scope of a constant or variable identifier declared inside a function begins at the end of the ConstSpec or VarSpec and ends at the end of the innermost containing block. 6. The scope of a type identifier declared inside a function begins at the identifier in the TypeSpec and ends at the end of the innermost containing block. An identifier declared in a block may be redeclared in an inner block. While the identifier of the inner declaration is in scope, it denotes the entity declared by the inner declaration. |
Issue #514 has been merged into this issue. |
Issue #505 has been merged into this issue. |
Issue #469 has been merged into this issue. |
The go spec for short variable declaration specifically addresses redeclaration, and explicitly states that this should not happen. From the go spec: "a short variable declaration may redeclare variables provided they were originally declared in the same block with the same type" Right now, you can shadow global variables, and redeclare their type. "Redeclaration does not introduce a new variable; it just assigns a new value to the original." var someGlobal = "foo"; func someFunc() (int, os.Error) { return 1, nil } func TestThree(t *testing.T) { if someGlobal, err := someFunc(); err == nil { // rather than throwing an error, someGlobal will now silently be an int == 1 } // now it will be a string == "foo" again } |
another possibility that i think would be useful: allow non-variables on the l.h.s. of a := as long as there's one new variable there. e.g. x := new(SomeStruct) x.Field, err := os.Open(...) i actually think this is less controversial than the original rule allowing non-new variables - at least it's obvious at a glance which variables have been declared. |
Comment 11 by ravenstone13@cox.net: I think the original poster was making a case for special treatment of return parameters. In principle I agree with his argument that it would reduce the potential for a certain class of subtle errors. The question is whether this potential benefit is worth introducing a 'special case' into the spec and eventually into all go compiler implementations. Since much is being made by go promoters about it being a 'safe' language I'm leaning towards agreement with OP, ie. no shadows of return parameters. I realize this isn't a democracy, it's just my opinion FWIW :-) |
Issue #739 has been merged into this issue. |
I think the OP highlights a more general problem: redeclaring variables from outer scopes can create subtle, hard to track down errors. Possible solution: make it an error to redeclare a variable declared in the same function. Rationale for this language change: * A whole class of hard to fix errors is eliminated * Probably it won't hurt most of existing, correct Go code * Probably it will highlight bugs or hard-to-maintain spots in the existing code * Redeclaration of global names is still allowed so that a new version of `import . "Foo"` package won't hijack your code * Does not complicate specification * Does not seem to complicate implementation, at least not much |
I'd like to introduce one additional proposal for consideration, which I believe addresses the original problem brought up by the OP, and which hasn't been covered yet. What if "=" was allowed to be used to declare variables as well, but only if at least one of the variables has *been* previously declared? In other words, this would be valid: a, err := f() if err == nil { b, err = g() if err == nil { ... } } return err This would be the exact counterpart behavior to :=, which may only be used when at least one of the variables has *not* been declared previously. It feels like I'd appreciate using this in practice, and would avoid the errors I've personally found with the shadowing. How do you all feel about this? |
Another alternative based on the conversation in the mailing list would be to use a per-variable declaration syntax. For instance, this: a, b := f() Would be fully equivalent to: :a, :b = f() and a construction in an internal block such as: err = f() might be extended to the following, which is completely clear and unambiguous: :a, err = f() When one doesn't want to redefine err. One of the things which feels interesting about this proposal is that it would enable forbidding entirely partial declarations via := if that's decided to be a good option, without compromising on other aspects of the language. |
Alternative proposals in spirit similar to comment 16, based on ideas expressed in http://groups.google.com/group/golang-nuts/browse_thread/thread/5f070b3c5f60dbc1 : Ideas, Variant 1: a, (b) := f1() // redefines b, reuses a (a, b), c, (d, e) := f2() // redefines c, reuses a, b, d, e // Flaw example: redundant with "a = f3()": (a) := f3() // reuses a Ideas, Variant 2: (var a), b = f1() // redefines a, reuses b a, b, (var c), d, e = f2() // redefines c, reuses a, b, d, e // Flaw example: redundant with "var a = f4": (var a) = f4() // redefines a |
Comment 20 by james@abneptis.com: I won't re-raise this on the list, but after thinking a few more days, I think my biggest disagreement with the current implementation allowing (the above given): func TestThree(t *testing.T) { if someGlobal, err := someFunc(); err == nil { // rather than throwing an error, someGlobal will now silently be an int == 1 } // now it will be a string == "foo" again } Is that the part that is creating the issue "if someGlobal, err := someFunc(); err == nil" doesn't /really/ seem to be part of the inner block scope to the reader; Yes, it's completely expected that loop setup variables would be available within the scope of the loop, and perhaps even, by default, not available outside of the loop scope. BUT, since the "clause" is outside of the braces, I think it's reasonable for a coder to assume that it has a "middle" scope, that would by default inherit from the global scope if available, otherwise creating variable solely available to the inner loop scope. I realize that's a complex description of the change, but I think if /clauses/ are solely targeted with the change, we'd minimize the chance for both confusion and bugs unintentionally introduced. (And if unchanged, I'd love a compiler warning, but hey, I know that's not in the plans ;) ) |
James, "A block is a sequence of declarations and statements within matching brace brackets. Block = "{" { Statement ";" } "}" . In addition to explicit blocks in the source code, there are implicit blocks: 1. The universe block encompasses all Go source text. 2. Each package has a package block containing all Go source text for that package. 3. Each file has a file block containing all Go source text in that file. 4. Each if, for, and switch statement is considered to be in its own implicit block. 5. Each clause in a switch or select statement acts as an implicit block." Blocks, The Go Programming Language Specification. http://golang.org/doc/go_spec.html#Blocks "In some contexts such as the initializers for if, for, or switch statements, [short variable declarations] can be used to declare local temporary variables." Short variable declarations, The Go Programming Language Specification. http://golang.org/doc/go_spec.html#Short_variable_declarations Therefore, until you can do it automatically in your head, you can simply explicitly insert the implicit blocks. For example, var x = "unum" func implicit() { fmt.Println(x) // x = "unum" x := "one" fmt.Println(x) // x = "one" if x, err := 1, (*int)(nil); err == nil { fmt.Println(x) // x = 1 } fmt.Println(x) // x = "one" } func explicit() { fmt.Println(x) // x = "unum" { x := "one" fmt.Println(x) // x = "one" { if x, err := 1, (*int)(nil); err == nil { fmt.Println(x) // x = 1 } } fmt.Println(x) // x = "one" } } |
Comment 22 by james@abneptis.com: Thanks; It's not so much that I don't understand with it, or even disagree with it; It's that it's a frequent source of errors that are hard to physically see (differing only in colon can have a dramatically different result). (snip much longer ramble) I have no problem with var v; func(){ v := 3 } It's foo()(err os.Error){ for err := bar(); err != nil; err = bar() { } } being substantially different than foo()(err os.Error){ for err = bar(); err != nil; err = bar() { } } and both being semantically correct. Essentially, my argument is w/r/t ONLY: "In some contexts such as the initializers for if, for, or switch statements, [short variable declarations] can be used to declare local temporary variables"; I would argue that since these are special cases to begin with, that in multi-variable := usage, resolving those local temporary variables should be handled via the same scope as the containing block, but stored in the inner scope if creation is necessary; I've got no problem with how it works, just been bitten by this more times than I'd care to admit, and surprised when I'd realized how many others had been as well. |
Comment 23 by ziutek@Lnet.pl: I think that Go should be explicit language. I prefer Go: ui = uint(si) than C: ui = si if ui is unsigned and si is signed. Why do we need an implicit behavior of :=? So if := is the declaration operator it should work exactly like var for all its lhs. If some of lhs are previously declared in this scope, it should fail - I believe we should have a separate explicit construction for this case. Proposal from comment 16 is nice for me: :a, b = f() In above case it doesn't introduce any additional character. In: :a, b, :c = f() it adds only one. This notation looks good. I can easily determine what's going on. a, b, c := f() should be an abbreviation for: :a, :b, :c = f() With current := behavior I fill like this: :=? I vote for change this emoticon to: := in the future. ;) |
In fact I think there is a perfect solution in one of the proposals. So, I'll sum up what I think: 1. Allow arbitrary addressable expressions on the left side of ':='. 2. Allow no new variables on the left side of ':=' (a matter of consistency in the code, see examples). 3. Use the following rule to distinguish between a need of "declare and initialize" and "reuse": If the LHS looks like an identifier, then the meaning is: "declare and initialize". Trying to redeclare a variable in the current block that way will issue an error. Otherwise LHS must be an addressable expression and the meaning is: "reuse". Rule allows one to use paren expression to trick the compiler into thinking that an identifier is an addressable expression. Examples: a, err := A() // 'a' and 'err' are identifiers - declare and initialize b, (err) := B() // 'b' - declare and initialize, '(err)' looks like an addressable expression - reuse type MyStruct struct { a, b int } var x MyStruct x.a, err := A() // 'x.a' is an addressable expression - reuse, 'err' is an identifier - declare and initialize x.b, (err) := B() // 'x.b' and '(err)' are addressable expressions - reuse (special case without any new variables) Of course it could be: x.b, err = B() // and that's just a matter of preferrence and consistency Note: My idea is a bit different from proposal above, the following syntax is invalid: (a, b), c := Foo() The right way to do this is: (a), (b), c := Foo() Yes, it's a bit longer. But keep in mind that the alternative is typing 'var a Type', 'var b Type'. Using parens is perfectly fine to me for such a complex case. Also this approach has one very cool property - it almost doesn't alter syntax (allowing arbitrary addressable expressions on the left side of ':=' is the only change), only special semantic meaning. |
I am in favor of doing away with := entirely because of the desire to control what is done per-value on multiple returns. The :val syntax described above seems nice and short and would seem like valid syntactic sugar for a keyword driven solution: :x = f(), declare(shadow) and initialize x, infer type x = f(), assign x, infer type would be the same as auto var x = f(), declare(shadow) and initialize x, infer type auto x = f(), assign x, infer type to revisit the implicit/explicit example shown above in comment 21: var x = "unum" func implicit() { fmt.Println(x) // x = "unum" :x = "one" //<- potentially make this an error, redeclaration after use in same scope. //:x = "two" <- would not compile, can only declare once in scope fmt.Println(x) // x = "one", global x still = "unum" if :x, :err = 1, (*int)(nil); err == nil { fmt.Println(x) // x = 1 } fmt.Println(x) // x = "one" } func explicit() { fmt.Println(x) // x = "unum" { :x = "one" fmt.Println(x) // x = "one" { if :x, :err = 1, (*int)(nil); err == nil { fmt.Println(x) // x = 1 } } fmt.Println(x) // x = "one" } fmt.Println(x) // x = "unum" } to revisit the example in the original post: func f() (err os.Error) { :v, err = g(); <-- reusing err for return if err != nil { return; } if v { :v, err = h(); <-- shadowing v, but reusing err for return if err != nil { return; } } } in addition, if one wants to enforce typing per-value, specifying type removes the need for :val as you cannot re-specify a type on an existing value and thus initialisation is inferred. int :x, os.Error err = f(); initialize and assign x/error, don't compile if return value 2 is not os.Error |
Could you possibly elaborate a bit why? Especially with regards to the alternative "explicit" syntax proposals? I don't plan to argue, the right to any final decision is obviously yours as always; but I'd be highly interested to know if there are some problems expected to be introduced by those proposals, or otherwise what is the common reasoning behind this decision. Thanks. |
Agreed. Besides _changing_ :=, there are other proposals, and this problem was brought up repeatedly in the mailing list by completely different people, with this thread being referenced as the future answer (how many issues are starred by 46+ people?). It'd be nice to have some more careful consideration and feedback before dismissal. |
The decision about := is not mine, at least not mine alone. I am just trying to clean up the bug tracker, so that it reflects things we need to work on. 1. The bug entry is 1.5 years old at this point. If it were going to have an effect, it would have by now. 2. This comes up occasionally on its own. A bug entry is not necessary to remind us about it. I'll change the status back to long term but I remain skeptical that anything will change. Status changed to LongTerm. |
Thanks for the work on cleaning up, it's appreciated. It's also certainly fine for this to be closed if it reflects a decision made. The point was mostly that it'd be nice to have some feedback on the features proposed for solving the problem, given that there's so much interest on the problem and a bit of love towards a few of the proposed solutions. E.g. allowing this: :v, err = f() as equivalent to var v T v, err = f() If you have internally decided this is off the table, it'd be nice to know it, and if possible what was the reasoning. |
Another simple fix is to prefix
The benefit here is that no new expression notations are invented (same as |
Is |
Yes, it has already been a nop now. |
Could we consider a clearer (but more verbose) solution to the problem in the same spirit as Proposal
Current Go 1 variable shadowingpackage main
import (
"fmt"
)
var a int
func printGlobal() {
fmt.Printf("Print Global: %d\n", a)
}
func getNum() (int, error) {
return 4, nil
}
func main() {
a, err := getNum()
if err != nil{
fmt.Printf("error")
return
}
printGlobal()
fmt.Printf("Printing in main: %d\n", a)
} Output
With
|
@srinathh That problem with a keyword like |
@ianlancetaylor Yes you're right... but that criticism is equally applicable to any other proposal short of completely removing support for the I think the key to reducing possibility of subtle bugs is reduce cognitive load so programmers can better maintain contextual awareness. A keyword approach I think makes things a lot more obvious vs. introducing cryptic notations into the |
@srinathh Fair point. I guess what I'm mean is that while cryptic notations are definitely cryptic, and perhaps not a good idea, at least they are short. If I have to write out |
Hi, I came up with a slightly different approach for this issue. Will be glad to hear the community's thoughts on this. ProblemIn my point of view, one of the main problems with a, b := 1, 1 // depending on the context, this might mean 3 different things
// a and b are declared and initialized
// a is declared and initialized, b is only assigned a new value
// a is only assigned a new value, b is declared and initialized This might seem harmless but can be very misleading specially when it is combined with shadowed variables: a, err := foo() // a and err are declared and initialized
b, err := bar() // b is declared and initialized, err is assigned a new value
if c, err := foobar(); err != nil { // c and err are declared and initialized inside
// the scope of the if, err is shadowed
}
// now err has the value returned by bar, not foobar In this example, the programmer might have expected the same behaviour in the assignments of err from bar and foobar. Assuming that variable masking is here to stay (although that might an interesting debate), I think that the best solution is to change the ambiguity of
This "redeclaration" in the same block which "does not introduce a new variable" does not only cause the aforementioned confusions, but I also find it difficult to understand conceptually. Why is it even called redeclaration, when this is just simply an assignment? Reading this made me doubt if the address of a "redeclared" variable was preserved (it is). Proposal
a, err := foo()
b, err := foo() // compilation error because var err is already declared in this block
a:=, err:= foo() // a and err are declared and initialized (equivalent to: a, err := foo())
b:=, err= foo() // b is declared and initialized, whereas err is only assigned a new value
if true {
c:=, err= foo() // c is declared and initialized in the if block, and err is assigned a new value
}
if true {
d:=, err:= foo() // d and err are declared in the if block, err is shadowed
} Similar proposalsThis is similar to other proposals such as using Backwards compatibilityPart 1 of the proposal is not backwards compatible. Part 2 could be introduced as a Go 1 minor release. With a partial adoption, we could use automatic tools to break problematic expressions such as Pros and cons
Some remarksWith this change, single-variable short declarations ( I suggest as a default |
@a-canya, thanks for the proposal,
vs.
The
Good question. |
I haven't seen strong opposition to @nsf's suggestion. IMHO, this is the best way to solve this problem by far. It is just a little (but acceptably) verbose. |
@pam4 thanks for the answer! Very valid point indeed, and you show a good example of something that would confusing ugly if my proposal were accepted. I'd like to remark that using the // Case 1
a, b:= foo(1, "hello", x) // current ambiguous approach
:a, b = foo(1, "hello", x) // alternative 1
a:=, b= foo(1, "hello", x) // alternative 2
// Case 2. IMO that's simply bad programming. Just break it into 2 lines for God's sake
a, b := 1, 2 // current ambiguous approach
:a, b = 1, 2 // alternative 1
a:=, b= 1, 2 // alternative 2 I don't like mixing assignments and declarations, they are essentially different operations. I understand that's convenient for function returns, but other than that I would strongly discourage anyone from doing it. And looking at case 1 I don't think I'd have a problem visually distiguishing the RHS and LHS. In any case, which implementation looks better si very subjective. I hope we can find one alternative that's good enough for everyone to make a change. |
Here is my proposal. Assume a go.mod gated roll out. There are various situations in which it is annoying or impossible to specify the type for a variable. For example, a recursive closure must be written as var f func(a B, b B) c C
f = func(a B, b B) c C {
// ...
} This is because var x pkg.xtype // illegal
x := pkg.Xer() // ok
var x = pkg.Xer() // ok
var y pkg.Ytype; x, y = pkg.XAndYer() // illegal and lots of weird edge cases about when you can assign to y So, what should we do? Let's take the So, the classic nested err thing becomes
Tricky bits:
OTOH, I think this should probably be legal, so having the assignment to an auto in an inner scope should be okay:
Advantages of
|
As I noted above, nsf's suggestion is basically #30318 plus 2 points:
There is already some consensus about getting rid of redeclaration, so I think that if we can push #30318 through, we're almost there.
I can't think of any (technical) advantage over the backward compatible colon-prefix syntax, but #30318 is far more popular, with many upvotes, and already implemented!
I agree, but I think it's important to also consider edge cases.
I think of the initialization part of a declaration as being an assignment, that's why extending the
Indeed.
Hopefully the steady stream of similar proposals over many years will convince the core team that there is a need for such a mechanism. |
Almost, but not for the declaration in
What's the backward compatibility problem here? Comparing to the "colon-prefix syntax" one, nsf's solution don't need to invent a syntax. |
I don't understand.
Without redeclaration, a bare identifier on the LHS of a // this code currently compiles, but it should fail per nsf's proposal
a, ok := f()
if !ok {
//...
}
b, ok := g() // ok is redeclared
// it should be written like this instead:
a, ok := f()
if !ok {
//...
}
b, (ok) := g()
// under #30318 alone, both are supposed to work
Just to be sure there's no misunderstanding, the "colon-prefix syntax" works with the That said, the actual choice of symbol is not that important to me. |
@pam4 The intention of the other issue (#30318) is to allow non-pure-identifier items on the left of There are some possible ways to go through:
The way |
Sorry, you lost me again. The colon-prefix syntax doesn't use the
What you describe as
If we go for #30318, dropping redeclarations is the obvious next step ( |
Do you mean, if we go for the colon-prefix syntax there, the #30318 cases can be solved as
? If this is true, I think this is not an elegant design, for code readability and easy-explanation reasons, and it needs to invent a new expression syntax.
So do you mean the new syntax will coexist with the old redeclartion syntax, to keep back compatibility? If this is true, it doesn't remove the confusion the current issue tries to remove. IMHO, |
@go101, ok, it seems that we understood each other now. oldV1.field, oldV2, :newV = f() // colon-prefix syntax
oldV1.field, (oldV2), newV := f() // #30318
No, the intent is for the
The other solution can also be done in 2 steps (#30318 and then dropping redeclarations), but before the second step there is always the possibility of forgetting a Anyway, regardless of my preference I'm just arguing that the colon-prefix syntax is a viable alternative. I think your preference is still good and more likely to happen (because of the popularity of #30318). So it's not me that you have to convince, I would gladly introduce #30318 today and drop redeclarations in Go2. |
The text was updated successfully, but these errors were encountered: