diff --git a/doc/spec.md b/doc/spec.md
index c9cb1d49..dd4f9f22 100644
--- a/doc/spec.md
+++ b/doc/spec.md
@@ -1090,7 +1090,7 @@ Four Starlark constructs bind names, as illustrated in the example below:
`load` statements (`a` and `b`),
`def` statements (`c`),
function parameters (`d`),
-and assignments (`e`, `h`, including the augmented assignment `e += h`).
+and assignments (`e`, `h`, including the augmented assignment `e += 1`).
Variables may be assigned or re-assigned explicitly (`e`, `h`), or implicitly, as
in a `for`-loop (`f`) or comprehension (`g`, `i`).
@@ -1182,7 +1182,7 @@ x = 2 # static error: cannot reassign global x declared on lin
```
@@ -1190,14 +1190,13 @@ x = 2 # static error: cannot reassign global x declared on lin
If a name was pre-bound by the application, the Starlark program may
explicitly bind it, but only once.
+An augmented assignment statement such as `x += y` is considered both a
+reference to `x` and a binding use of `x`, so it may not be used at
+top level.
+
Implementation note:
-An augmented assignment statement such as `x += 1` is considered a
-binding of `x`.
-However, because of the special behavior of `+=` for lists, which acts
-like a non-binding reference, the Go implementation suppresses the
-"cannot reassign" error for all augmented assigments at toplevel,
-whereas the Java implementation reports the error even when the
-statement would apply `+=` to a list.
+The Go implementation of Starlark permits augmented assignments to appear
+at top level if the `-globalreassign` flag is enabled.
A function may refer to variables defined in an enclosing function.
In this example, the inner function `f` refers to a variable `x`
@@ -2640,7 +2639,7 @@ An `if` statement is permitted only within a function definition.
An `if` statement at top level results in a static error.
Implementation note:
-The Go implementation of Starlark permits `if`-statements to appear at top-level
+The Go implementation of Starlark permits `if`-statements to appear at top level
if the `-globalreassign` flag is enabled.
@@ -2667,7 +2666,7 @@ A `while` statement at top level results in a static error.
Implementation note:
The Go implementation of Starlark permits `while` loops only if the `-recursion` flag is enabled.
-A `while` statement is permitted at top-level if the `-globalreassign` flag is enabled.
+A `while` statement is permitted at top level if the `-globalreassign` flag is enabled.
### For loops
@@ -2709,7 +2708,7 @@ In Starlark, a `for` loop is permitted only within a function definition.
A `for` loop at top level results in a static error.
Implementation note:
-The Go implementation of Starlark permits loops to appear at top-level
+The Go implementation of Starlark permits loops to appear at top level
if the `-globalreassign` flag is enabled.
@@ -4033,7 +4032,6 @@ See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
* The `chr` and `ord` built-in functions are supported.
* The `set` built-in function is provided (option: `-set`).
* `set & set` and `set | set` compute set intersection and union, respectively.
-* `x += y` rebindings are permitted at top level.
* `assert` is a valid identifier.
* The parser accepts unary `+` expressions.
* A method call `x.f()` may be separated into two steps: `y = x.f; y()`.
@@ -4041,5 +4039,5 @@ See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
* `hash` accepts operands besides strings.
* `sorted` accepts the additional parameters `key` and `reverse`.
* `type(x)` returns `"builtin_function_or_method"` for built-in functions.
-* `if`, `for`, and `while` are permitted at toplevel (option: `-globalreassign`).
+* `if`, `for`, and `while` are permitted at top level (option: `-globalreassign`).
* top-level rebindings are permitted (option: `-globalreassign`).
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 9c5a0b80..feb8f0e9 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -293,19 +293,15 @@ type use struct {
// bind creates a binding for id in the current block,
// if there is not one already, and reports an error if
-// a global was re-bound and allowRebind is false.
+// a global was re-bound.
// It returns whether a binding already existed.
-func (r *resolver) bind(id *syntax.Ident, allowRebind bool) bool {
+func (r *resolver) bind(id *syntax.Ident) bool {
// Binding outside any local (comprehension/function) block?
if r.env.isModule() {
id.Scope = uint8(Global)
prev, ok := r.globals[id.Name]
if ok {
- // Global reassignments are permitted only if
- // they are of the form x += y. We can't tell
- // statically whether it's a reassignment
- // (e.g. int += int) or a mutation (list += list).
- if !allowRebind && !AllowGlobalReassign {
+ if !AllowGlobalReassign {
r.errorf(id.NamePos, "cannot reassign global %s declared at %s", id.Name, prev.NamePos)
}
id.Index = prev.Index
@@ -446,11 +442,6 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
}
}
r.expr(stmt.RHS)
- // x += y may be a re-binding of a global variable,
- // but we cannot tell without knowing the type of x.
- // (If x is a list it's equivalent to x.extend(y).)
- // The use is conservatively treated as binding,
- // but we suppress the error if it's an already-bound global.
isAugmented := stmt.Op != syntax.EQ
r.assign(stmt.LHS, isAugmented)
@@ -458,8 +449,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
if !AllowNestedDef && r.container().function != nil {
r.errorf(stmt.Def, doesnt+"support nested def")
}
- const allowRebind = false
- r.bind(stmt.Name, allowRebind)
+ r.bind(stmt.Name)
r.function(stmt.Def, stmt.Name.Name, &stmt.Function)
case *syntax.ForStmt:
@@ -467,8 +457,8 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
r.errorf(stmt.For, "for loop not within a function")
}
r.expr(stmt.X)
- const allowRebind = false
- r.assign(stmt.Vars, allowRebind)
+ const isAugmented = false
+ r.assign(stmt.Vars, isAugmented)
r.loops++
r.stmts(stmt.Body)
r.loops--
@@ -498,7 +488,6 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
r.errorf(stmt.Load, "load statement within a function")
}
- const allowRebind = false
for i, from := range stmt.From {
if from.Name == "" {
r.errorf(from.NamePos, "load: empty identifier")
@@ -507,7 +496,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
if from.Name[0] == '_' {
r.errorf(from.NamePos, "load: names with leading underscores are not exported: %s", from.Name)
}
- r.bind(stmt.To[i], allowRebind)
+ r.bind(stmt.To[i])
}
default:
@@ -519,8 +508,7 @@ func (r *resolver) assign(lhs syntax.Expr, isAugmented bool) {
switch lhs := lhs.(type) {
case *syntax.Ident:
// x = ...
- allowRebind := isAugmented
- r.bind(lhs, allowRebind)
+ r.bind(lhs)
case *syntax.IndexExpr:
// x[i] = ...
@@ -616,15 +604,15 @@ func (r *resolver) expr(e syntax.Expr) {
// enclosing container (function/module) block.
r.push(&block{comp: e})
- const allowRebind = false
- r.assign(clause.Vars, allowRebind)
+ const isAugmented = false
+ r.assign(clause.Vars, isAugmented)
for _, clause := range e.Clauses[1:] {
switch clause := clause.(type) {
case *syntax.IfClause:
r.expr(clause.Cond)
case *syntax.ForClause:
- r.assign(clause.Vars, allowRebind)
+ r.assign(clause.Vars, isAugmented)
r.expr(clause.X)
}
}
@@ -755,7 +743,6 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
b := &block{function: function}
r.push(b)
- const allowRebind = false
var seenOptional bool
var star, starStar *syntax.Ident // *args, **kwargs
for _, param := range function.Params {
@@ -769,7 +756,7 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
} else if seenOptional {
r.errorf(param.NamePos, "required parameter may not follow optional")
}
- if r.bind(param, allowRebind) {
+ if r.bind(param) {
r.errorf(param.NamePos, "duplicate parameter: %s", param.Name)
}
@@ -780,7 +767,7 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
} else if star != nil {
r.errorf(param.OpPos, "optional parameter may not follow *%s", star.Name)
}
- if id := param.X.(*syntax.Ident); r.bind(id, allowRebind) {
+ if id := param.X.(*syntax.Ident); r.bind(id) {
r.errorf(param.OpPos, "duplicate parameter: %s", id.Name)
}
seenOptional = true
@@ -803,7 +790,7 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
starStar = id
}
}
- if r.bind(id, allowRebind) {
+ if r.bind(id) {
r.errorf(id.NamePos, "duplicate parameter: %s", id.Name)
}
}
diff --git a/resolve/testdata/resolve.star b/resolve/testdata/resolve.star
index bd038ce6..1baad688 100644
--- a/resolve/testdata/resolve.star
+++ b/resolve/testdata/resolve.star
@@ -91,18 +91,17 @@ def g():
f()
---
-# It's permitted to rebind a global using a += assignment.
+# It is not permitted to rebind a global using a += assignment.
x = [1]
x.extend([2]) # ok
-x += [3] # ok (a list mutation, not a global rebinding)
+x += [3] ### `cannot reassign global x`
def f():
x += [4] # x is local to f
y = 1
-y += 2 # ok (even though it is in fact a global rebinding)
-
+y += 2 ### `cannot reassign global y`
z += 3 # ok (but fails dynamically because z is undefined)
---
@@ -193,15 +192,15 @@ while U: # ok
---
# The parser allows any expression on the LHS of an assignment.
-1 = 2 ### "can't assign to literal"
-1+2 = 3 ### "can't assign to binaryexpr"
-f() = 4 ### "can't assign to callexpr"
+1 = 0 ### "can't assign to literal"
+1+2 = 0 ### "can't assign to binaryexpr"
+f() = 0 ### "can't assign to callexpr"
-[a, b] = [1, 2]
-[a, b] += [3, 4] ### "can't use list expression in augmented assignment"
-(a, b) += [3, 4] ### "can't use tuple expression in augmented assignment"
-[] = [] ### "can't assign to \\[\\]"
-() = () ### "can't assign to ()"
+[a, b] = 0
+[c, d] += 0 ### "can't use list expression in augmented assignment"
+(e, f) += 0 ### "can't use tuple expression in augmented assignment"
+[] = 0 ### "can't assign to \\[\\]"
+() = 0 ### "can't assign to ()"
---
# break and continue statements must appear within a loop
diff --git a/starlark/testdata/assign.star b/starlark/testdata/assign.star
index ae97bb2c..c3fb0910 100644
--- a/starlark/testdata/assign.star
+++ b/starlark/testdata/assign.star
@@ -288,6 +288,7 @@ assert.eq(g(), {"one": 1, "two": 2})
---
# parenthesized LHS in augmented assignment (success)
+# option:globalreassign
load("assert.star", "assert")
a = 5